[Waterpine]

Summary

BloomFilter computed each bit index as hash % capacity, a 64-bit hardware division per hash function, and hashed u64 items through an out-of-line generic function. This PR removes both costs:

• The shifted hash is always below 2^63, so the remainder is computed by division by an invariant integer (Granlund and Montgomery, 1994): one high multiply and a shift give the quotient, and one low multiply gives the remainder. The multiplier is derived from the bit array length when a filter is built or deserialized, and the result equals hash % capacity for every supported capacity.
• XxHash64 overrides Hasher::write_u64 with a small inlined body that produces the same hash as the default, and compute_hash is #[inline], so hashing a u64 item no longer goes through a 343-instruction generic function.

The reduction and the hashing change are by @pmcgleenon.

On a CPU with a slow 64-bit divider (Kaby Lake), the three bloom::update scenarios take 0.30 to 0.49 of the time on main (51% to 70% less time). On Zen 3, which has a fast divider, they take 0.59 to 0.94 (6% to 41% less time). See "Scope of the results".

Serialization and bit placement are unchanged on every target. BloomFilter grows from 40 to 64 bytes; heap use is unchanged.

Results

The ratio is the time with this PR divided by the time on main; below 1 is faster.

scenario	Zen 3 (EPYC 7R13)	Kaby Lake (i5-7500)
bloom::update::u64	0.68	0.31
bloom::update::bytes_32	0.94	0.49
bloom::update::contains_and_insert_present_u64	0.59	0.30

Each ratio is the product of two measurements with the repository harness (bloom --repeat 10, medians of three alternating runs): the first version of this PR against main (86eb143), and the final version against the first version. Zen 3: 0.96 × 0.71, 1.03 × 0.91, 0.94 × 0.63. Kaby Lake: 0.44 × 0.70, 0.54 × 0.90, 0.46 × 0.65.

Scope of the results

• The gain from removing the division depends on the CPU's divider. With the first version of this PR (a 128-bit multiplier), Zen 3 gained little and bytes_32 was 3% slower; the single high multiply fixes that, and every measured scenario is now faster than main on both CPUs. CPUs with even faster dividers, such as Apple M-series or newer AMD and Intel cores, were not measured.
• The final version was not measured directly against main; the ratios above multiply two measurements. On Haswell (Xeon E5-2660 v3), the first version measured 0.50, 0.60, and 0.54 against main (6856e45, 10 paired runs), and query-only contains and inserts into a 12 MB filter measured 0.41 to 0.81. The final version was not measured on Haswell.
• The hashing change applies to u64 items only; byte slices and strings hash as before. Count-Min and XOR filters also hash through XxHash64; their hash values are unchanged, and their performance was not measured.
• 32-bit targets were not measured.

Compatibility

Bit placement is unchanged on every target. compute_bit_index still converts the double hash to usize before the reduction, which truncates it on 32-bit targets such as wasm32; filters serialized there depend on that layout. The resulting difference from 64-bit builds and from the other DataSketches implementations predates this PR and needs a migration plan, so it is left for a separate change.

Checks

cargo x check, cargo x lint, cargo x test, and cargo +1.86.0 x test pass. main at c344dad is merged into the branch.

[pmcgleenon]

Hi @Waterpine @tisonkun

I re-measured the change with the repository harness on two CPUs, with the PR cherry-picked onto main (86eb143) so both sides share the same harness. Medians of three alternating bloom --repeat 10 runs; the ratio is PR time / main time, below 1 is faster.

scenario	Zen 3 (EPYC 7R13)	Kaby Lake (i5-7500)
bloom::update::u64	0.96	0.44
bloom::update::bytes_32	1.03	0.54
bloom::update::contains_and_insert_present_u64	0.94	0.46

So the Haswell result reproduces (and then some) on Skylake-family cores, but on CPUs with a fast 64-bit divider the five multiplies cost about what the division did, and the 32-byte case is slightly slower because hashing already saturates the multiplier port.

One more thing from profiling on the Zen 3 box: for u64 items, compute_hash is a non-inlined 343-instruction function (about 40% of insert time) because the default Hasher::write_u64 routes through the fully inlined streaming write and LLVM refuses to inline the hash. Overriding write_u64 with a small body, marking compute_hash #[inline], and reducing the shifted hash (always below 2^63) with a 64-bit multiplier and one shift instead of the 128-bit multiplier gives, relative to this PR:

scenario	Zen 3	Kaby Lake
bloom::update::u64	0.71	0.70
bloom::update::bytes_32	0.91	0.90
bloom::update::contains_and_insert_present_u64	0.63	0.65

Bit placement is unchanged and the placement digests pass.

I made this change on a branch from this PR
https://github.com/pmcgleenon/datasketches-rust/tree/perf/bloom-hash-and-two-multiply

[Waterpine]

@pmcgleenon Thanks for measuring this on Zen 3 and Kaby Lake, and for the follow-up branch.

The Zen 3 numbers show the case I could not measure: the gain comes from removing the 64-bit division, so on a CPU with a fast divider the 128-bit reduction costs about as much as the division it replaces, and bytes_32 ends up slightly slower. I would rather not merge a change that makes a measured case slower, so I'd like to fold your commit into this PR and keep you as its author. With the single-multiply reduction and the inlined hash, your numbers put every scenario below main on both CPUs.

@tisonkun the write_u64 override changes the shared XxHash64, which Count-Min and XOR also use. Should it stay in this PR, or go in a separate one? Also, could you approve the CI run when you have a moment?

[tisonkun]

@Waterpine I reproduced a silent false negative when an existing wasm32 filter is read after upgrading to this PR. Please keep the old mapping here and handle the compatibility correction separately with a migration plan.

This was introduced in 7615853: compute_bit_index, line 650 removes the as usize before the right shift. The earlier optimization (4f5253c) preserved it. On 32-bit targets, this changes the input from a truncated hash to the full u64.

Both the new reduction algorithm and the write_u64 optimization can stay. Restore the original truncation before passing the hash to the reducer:

let hash = h0.wrapping_add(u64::from(i).wrapping_mul(h1)) as usize;
self.reducer.remainder((hash >> 1) as u64) as usize

Minimal reproduction: the bytes below were generated by 4ba130e on wasm32 using BloomFilterBuilder::with_size(192, 3).seed(9001).build(), then inserting 0u64. In actual wasm32 builds run in Node, the base reads this item successfully; the PR head fails the assertion. Enable the bloom feature:

use datasketches::bloom::BloomFilter;

fn main() {
    let hex = "0401150003000000292300000000000003000000000000000300000000000000000808040000000000000000000000000000000000000000";
    let bytes: Vec<u8> = (0..hex.len())
        .step_by(2)
        .map(|i| u8::from_str_radix(&hex[i..i + 2], 16).unwrap())
        .collect();

    let filter = BloomFilter::deserialize(&bytes).unwrap();
    assert!(filter.contains(&0u64)); // Fails on the PR head.
}

The reverse direction also fails on rollback. Old and new filters still pass is_compatible, so mixing them also silently breaks set operations. Re-serialization does not repair the mapping.

About splitting the hashing change

Splitting the hashing optimization into a separate PR is fine and would make its validation easier to review, but I do not require a split. Keeping it here is also fine.

[Waterpine]

@tisonkun Thanks for catching this and for the reproduction. I've restored the usize conversion as you suggested (40c9066), so bit placement is unchanged on every target, and the unit test compares against the original expression again. I have no wasm32 runtime here, so I couldn't rerun your reproduction; the code is the form you suggested.

The description no longer claims the 32-bit change; the 32-bit correction is left for a separate change with a migration plan. I'll keep the hashing change in this PR, since the measurements cover the combination. main is merged in to resolve the conflict with #340.