Skip to content

Commit aab66c6

Browse files
Michael Norrisfacebook-github-bot
authored andcommitted
relax codes%8 condition on Hamming distance
Summary: `faiss::hammings()` used to `FAISS_THROW_IF_NOT(ncodes % 8 == 0)`, even though the other Hamming entry points (`hammings_knn_hc`, `hamming_range_search`) have always accepted code sizes that are not a whole number of 64-bit words. This lifts that restriction so `hammings()` matches the rest of the API. Ragged sizes are routed to a new out-of-line helper, `hammings_ragged()`, built on the existing `HammingComputer` family (which already carries a byte tail). Multiples of 8 keep taking the word-level kernels exactly as before, so there is no behavior change for existing callers. Also adds a `FAISS_NOINLINE` macro to `platform_macros.h` (MSVC `__declspec(noinline)` / GCC-Clang `__attribute__((noinline))`), used to keep `hammings_ragged()` out of line. ## Why FAISS_NOINLINE Letting the compiler inline `hammings_ragged()` into `hammings_fixSL()` instantiates the whole `HammingComputer` family into that function and roughly doubles its size (3,597 -> 7,565 bytes at the AVX2 level). That measurably slows the word-level code sizes that share the runtime-nwords loop. Benchmarked on a Xeon 8339HC (Cooper Lake, AVX-512), `mode/opt`, na=256 x nb=4096 = 1M pairs per call, one code size per process, A/B interleaved with order flipping, 300-400 samples per cell, replicated on two cores. Positive = inlining is slower, i.e. `noinline` wins. | ncodes | static AVX2 | dynamic dispatch (AVX512) | | --- | --- | --- | | 8, 16, 32, 64, 128 | ~0% | ~0% | | 24 | +3.4% | +53% | | 40 | +5.0% | +22% | | 48 | ~0% | +28% | | 56 | +4.4% | +35% | | 12, 20, 33 (ragged) | -5% | -5% to -12% | Two distinct mechanisms, both confirmed in the disassembly: - **Static AVX2** - the inlined version has higher register pressure and spills, adding exactly one reload per pair in the shared outer loop. Retired instruction counts (noise-free) show +1.00 instr/pair at ncodes 24/40/48/56 and 0.00 at 8/16/32/64/128. - **Dynamic dispatch** - the popcount inner loop is a byte-identical 24-byte sequence in both builds, but out of line it starts at `%32 == 0` (one 32-byte fetch window) and inlined at `%32 == 16` (straddles two). IPC drops 3.02 -> 2.36 at identical clock. This one is a code-placement effect: it is real today but is not something `noinline` reliably controls, and it could invert on a compiler upgrade. Note that the dedicated `ncodes` 8/16/32/64 kernels are unaffected either way - they have their own specialized `hammings_impl<nbits>` blocks that the ragged inline never touches. The trade: the ragged path itself is ~5% slower for being out of line. That is accepted here because the word-level sizes are the common case and are the ones that were already supported. Differential Revision: D115982541
1 parent 80a1656 commit aab66c6

4 files changed

Lines changed: 76 additions & 2 deletions

File tree

faiss/impl/platform_macros.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ inline int __builtin_clzll(uint64_t x) {
104104
#endif // _MSC_VER
105105

106106
#define FAISS_ALWAYS_INLINE __forceinline
107+
#define FAISS_NOINLINE __declspec(noinline)
107108

108109
// MSVC uses pragma pack instead of __attribute__((packed))
109110
// Use FAISS_PACK_STRUCTS_BEGIN/END to wrap packed structure definitions
@@ -137,6 +138,7 @@ inline int __builtin_clzll(uint64_t x) {
137138
#define FAISS_PACK_STRUCTS_END
138139

139140
#define FAISS_ALWAYS_INLINE __attribute__((always_inline)) inline
141+
#define FAISS_NOINLINE __attribute__((noinline))
140142

141143
#endif
142144

faiss/utils/hamming.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ FAISS_API extern size_t hamming_batch_size;
111111
*
112112
* @param a size na * nbytespercode
113113
* @param b size nb * nbytespercode
114-
* @param nbytespercode should be multiple of 8
114+
* @param nbytespercode any size; multiples of 8 take a faster kernel
115115
* @param dis output distances, size na * nb
116116
*/
117117
void hammings(

faiss/utils/hamming_distance/hamming_impl.h

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,25 @@ void generalized_hammings_knn_hc_impl(
298298
}
299299
}
300300

301+
FAISS_NOINLINE void hammings_ragged(
302+
const uint8_t* __restrict a,
303+
const uint8_t* __restrict b,
304+
size_t na,
305+
size_t nb,
306+
size_t ncodes,
307+
hamdis_t* __restrict dis) {
308+
with_HammingComputer<THE_SIMD_LEVEL>(ncodes, [&]<class HammingComputer>() {
309+
for (size_t i = 0; i < na; i++) {
310+
HammingComputer hc(a + i * ncodes, ncodes);
311+
const uint8_t* bi = b;
312+
for (size_t j = 0; j < nb; j++) {
313+
dis[i * nb + j] = hc.hamming(bi);
314+
bi += ncodes;
315+
}
316+
}
317+
});
318+
}
319+
301320
} // anonymous namespace
302321

303322
/******************************************************************
@@ -364,7 +383,19 @@ void hammings_fixSL<THE_SIMD_LEVEL>(
364383
size_t nb,
365384
size_t ncodes,
366385
hamdis_t* dis) {
367-
FAISS_THROW_IF_NOT(ncodes % 8 == 0);
386+
// Ragged sizes take the out-of-line HammingComputer path. Keeping it out
387+
// of line matters: inlining it here instantiates the whole computer family
388+
// into this function (roughly doubling its size) and slows the word-level
389+
// sizes that share the runtime-nwords loop below. Measured at ncodes
390+
// 24/40/48/56: +3-5% under static AVX2, where the inlined version spills
391+
// and reloads once more per pair, and +22-56% under dynamic dispatch, where
392+
// the (byte-identical) hot loop stops landing on a 32-byte boundary. The
393+
// dedicated 8/16/32/64 kernels are unaffected either way. The ragged path
394+
// itself pays ~5% for being out of line, which is the trade made here.
395+
if (ncodes % 8 != 0) {
396+
hammings_ragged(a, b, na, nb, ncodes, dis);
397+
return;
398+
}
368399
switch (ncodes) {
369400
case 8:
370401
hammings_impl<64>(C64(a), C64(b), na, nb, dis);

tests/test_hamming.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,3 +333,44 @@ TEST(TestHamming, test_hamming_knn) {
333333
EXPECT_EQ(dist_gen, *true_bit_distances) << assert_str.str();
334334
}
335335
}
336+
337+
// Code sizes that are not a whole number of 64-bit words take the byte-tail
338+
// kernel rather than the word-level one. The knn entry points have always
339+
// accepted them; hammings() used to throw instead.
340+
TEST(TestHamming, test_hammings_ragged_code_size) {
341+
std::default_random_engine rng(123);
342+
std::uniform_int_distribution<int32_t> uniform(0, 255);
343+
344+
const size_t na = 3;
345+
const size_t nb = 7;
346+
347+
for (auto code_size : {1, 2, 5, 12, 20, 33}) {
348+
std::vector<uint8_t> a(na * code_size);
349+
std::vector<uint8_t> b(nb * code_size);
350+
for (auto& v : a) {
351+
v = uniform(rng);
352+
}
353+
for (auto& v : b) {
354+
v = uniform(rng);
355+
}
356+
357+
// Reference: byte-wise popcount of the XOR, independent of faiss.
358+
std::vector<hamdis_t> expected(na * nb);
359+
for (size_t i = 0; i < na; ++i) {
360+
for (size_t j = 0; j < nb; ++j) {
361+
int d = 0;
362+
for (int c = 0; c < code_size; ++c) {
363+
d += __builtin_popcount(
364+
static_cast<unsigned>(
365+
a[i * code_size + c] ^
366+
b[j * code_size + c]));
367+
}
368+
expected[i * nb + j] = d;
369+
}
370+
}
371+
372+
std::vector<hamdis_t> dis(na * nb);
373+
faiss::hammings(a.data(), b.data(), na, nb, code_size, dis.data());
374+
EXPECT_EQ(dis, expected) << "code_size = " << code_size;
375+
}
376+
}

0 commit comments

Comments
 (0)