Speed up the chunker hot loop and fix a false-boundary edge case#367
Draft
folbricht wants to merge 1 commit into
Draft
Speed up the chunker hot loop and fix a false-boundary edge case#367folbricht wants to merge 1 commit into
folbricht wants to merge 1 commit into
Conversation
Restructure the buzhash rolling-hash loop in Chunker.Next: - Drop the ring-buffer window. The window bytes always live in the read buffer, so the byte leaving the window at position pos is simply buf[pos-ChunkerWindowSize]. This removes a store, an index increment and a wrap branch per byte, along with the hValue/hWindow/ hIdx fields whose state never survived across chunks. - Replace the two-part divisibility test with a branch-free multiply-and-rotate form (Hacker's Delight 10-17): hValue % d == d-1 is evaluated as "hValue+1 divisible by d" via rotr32((hValue+1)*inv(odd), k) - qBias <= qMax. This avoids Go's variable-shift overflow guard and reduces register pressure that previously spilled two constants to the stack in the hot loop. - Unroll the loop 2-way using h2 = rol2(h) ^ rol1(a0) ^ a1, halving the loop-carried rotate/xor dependency chain. The new boundary test also fixes a subtle bug introduced in #307: the old test computed hValue - (d-1), which wraps around for hValue < d-1, and since 2^32 is not divisible by d, exactly one hash value per discriminator (14413 for the default 64KB average chunk size) declared a false chunk boundary compared to the plain modulo. The new form was verified exhaustively against "hValue % d == d-1" over all 2^32 hash values for a range of discriminators, including powers of two, and TestChunkerBoundaryTest pins the equivalence for the wrap-around edge cases. Benchmarks (i7-7600U, interleaved runs against master): BenchmarkChunker ~369 MB/s -> ~499 MB/s (+35%) BenchmarkChunkNull50M ~255 MB/s -> ~433 MB/s (+70%) desync make -n 1 512MB 2.74s -> 2.27s Index output is byte-identical to master on a 512MB random test file.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restructures the buzhash rolling-hash loop in
Chunker.Nextfor a ~35% throughput gain on real data (~70% on boundary-free data), and fixes a subtle boundary-detection bug along the way.Changes
Drop the ring-buffer window. The window bytes always live in the read buffer, so the byte leaving the window at position
posis simplybuf[pos-ChunkerWindowSize]. This removes a store, an index increment and a wrap branch per byte, along with thehValue/hWindow/hIdxfields whose state never survived across chunks anyway.Branch-free boundary test.
hValue % d == d-1is now evaluated as "hValue+1divisible byd" using the multiply-and-rotate divisibility test (Hacker's Delight §10-17):rotr32((hValue+1)*inv(odd), k) - qBias <= qMax. The previous two-part test compiled to a variable shift with Go's shift-overflow guard (4 instructions) and kept 5 constants live, which spilled two of them to the stack inside the hot loop.2-way unroll. Using
h2 = rol2(h) ^ rol1(a0) ^ a1(rotation distributes over xor) halves the loop-carried rotate/xor dependency chain; the table loads and boundary tests run off the critical path. A 4-way unroll measured within noise, so the simpler form is kept.Bug fix
The Lemire-style test introduced in #307 computed
hValue - (d-1), which wraps around forhValue < d-1. Since 2^32 is not divisible byd, exactly one hash value per discriminator — 14413 for the default 64KB average chunk size — declared a false chunk boundary compared to the plain modulo. It's a one-in-2^32-per-byte event, so data integrity was never affected, but boundaries could deviate from casync/pre-#307 desync on large inputs, reducing cross-version dedup.The new form was verified exhaustively against
hValue % d == d-1over all 2^32 hash values for a range of discriminators (including powers of two, which need theqBiasterm).TestChunkerBoundaryTestpins the equivalence for the wrap-around edge cases.Benchmarks
i7-7600U, interleaved A/B runs against master under identical thermal conditions:
BenchmarkChunker(real data)BenchmarkChunkNull50M(no boundaries)desync make -n 1on 512MB, end-to-enddesync makeproduces byte-identical indexes to master on a 512MB random test file.