Repository navigation
Conversation
FSE_buildCTable_wksp() and ZSTD_buildFSETable_body() spread symbols
into the table in one pass, then walk the table in position order to
build the state transitions, reading each symbol back.
The spread step, tableSize/2 + tableSize/8 + 3, is odd for every valid
tableLog, so the spread is an invertible permutation: position u holds
the symbol at sorted index (u * inv) & (tableSize-1), where inv is the
inverse of step modulo tableSize. Walking positions in order and
reading symbols straight from the sorted layout fuses the two passes,
removing the scattered writes into the table and the read-back.
Only the fast path (no low-probability symbols) changes. The
low-probability spread skips positions and is left as is. Tables are
bit-identical, so compressed output is unchanged.
Fast-path table build time, ns/table, mixed tableLog 5-9:
FSE_buildCTable_wksp ZSTD_buildFSETable
Apple M1, clang 21 449 -> 365 509 -> 389
Xeon 8481C, gcc 13 677 -> 512 600 -> 524
EPYC 9B14, gcc 13 547 -> 460 628 -> 511
End-to-end decompression of independent 4 KB to 32 KB chunks improves
by 1-2% on all three machines; 64 KB and larger blocks are unchanged.
This branch has not been deployed
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
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.
What and why
FSE_buildCTable_wksp()andZSTD_buildFSETable()build their tables in two passes: spread the symbols into the table, then walk it in position order and read each symbol back. This PR fuses the two passes on the fast path (no low-probability symbols).FSE_TABLESTEP()is odd for every validtableLog, so the spread is an invertible permutation: positionuholds the symbol at sorted index(u * inv) & (tableSize-1), withinvthe inverse of the step modulotableSize. Walking positions in order and reading symbols from the sorted layout removes the scattered writes and the read-back.FSE_invTableStep()(new, infse.h) computesinvand asserts it.Tables are bit-identical, so the format and the compressed output are unchanged. The low-probability path is left as is. No API change.
Performance
Default
makeflags. Apple M1 (clang 21), Intel Xeon 8481C and AMD EPYC 9B14 (gcc 13.3, Ubuntu 24.04).Table build, ns per table, 256 random distributions with tableLog 5 to 9, min of 7 rounds, base and new interleaved:
FSE_buildCTable_wkspZSTD_buildFSETableDecompression speed,
zstd -b1 -B<chunk> -i3on 3.4 MB of C source, two interleaved runs:Compression speed within ±1%, compressed sizes identical. Not measured: other data sets, levels above 1, MSVC, 32-bit.
Verification
tests/fuzzer.ccompares both builders against a plain spread-then-read-back reference for every tableLog, with and without low-probability symbols. It fails on three mutants, including a wrong but shared inverse that keeps encoder and decoder consistent.fuzzer,zstreamtestanddecodecorpus -tpass with asserts enabled.make staticAnalyzereports the same findings ondevand on this branch.FSE_buildDTable_internal()regressed 5 to 23% on Sapphire Rapids with gcc, so it is left out.Checklist