[lizthegrey]

Two independent changes to the per-block table setup, one commit each. Profiling against libzstd (dev d9c0c7e) on the real corpora from https://klauspost.com/files/compress/ showed per-block table setup costing Go almost twice what it costs C on arm64. On Graviton5, apache.log re-encoded with zstd -3 spent 15.1% of Go's DecodeAll time building tables versus 9.6% of libzstd's, about 8 of the 13 points Go trailed by on that file.

Independent of #1212, #1230, #1232 and #1233: it touches only table construction, is based on master, and applies cleanly on top of all four.

(1) Fold the FSE symbol transform into the table build (fe03c18)

Each sequence table was built in two passes:

• buildDtable spread the symbols and set nbBits/newState.
• transform then rewrote every state to replace the symbol with its addBits and baseline, checking the symbol bound on each entry. That check was also the only thing rejecting a table that gives a count to a symbol past its range; (3) moves it into readNCount.

Each table's per-symbol addBits/baseline is now precomputed once in initPredefined, and the build loop merges them into the entry it stores, as the reference decoder does in ZSTD_buildFSETable. The asm build loop (_generate/gen_fse.go, regenerated for amd64 and arm64) now does one 8-byte store per state instead of a byte store and a word store.

maxBits is taken from the last symbol's addBits, because addBits never decreases with the symbol in any of the three tables; initPredefined panics if that ever stops holding. For every table readNCount accepts (with (3), no symbol past the table's range), the value is exact, since the last symbol always has a nonzero count. In any other case it can only be too large, which costs an extra bitreader fill but never loses bits.

transform was 3.2–3.4% of DecodeAll time on Graviton5 (1.2% on Zen 3) and is gone. The merged store costs nothing measurable: buildDtable_asm's share is unchanged, and it is now at about the same share as ZSTD_buildFSETable (6.7% vs 6.35%).

BenchmarkBuildDtable now includes the work transform did, so benchstat against master reads +21–43%. Compared like for like (master's buildDtable + transform vs this buildDtable, Zen 3, n=10): litLength −19.8%, offset −16.0%, matchLength −19.0%, all p=0.000.

(2) huff0: faster ReadTable weight stats and table fill (f20c7fd)

• Weight stats. Real weight lists repeat values, so counting into one histogram made each increment wait on the previous store to the same counter; that was ~10% of ReadTable. Weights are now counted into four histograms and merged. The weight total and the "weight too large" check are taken from the merged counts, with the same errors as before.
• Table fill. Runs of 4 or more table entries are filled 4 at a time.

Test: TestReadTableWeights in decompress_test.go covers the weight checks inside and after the four-at-a-time loop, plus a valid table.

The remaining piece of ReadTable is decoding the weights through the generic fse.Decompress, which is not changed here.

(3) Reject FSE counts for symbols past the table's range (d1dbb1a)

readNCount's main loop stops at the table's last symbol, but a run of zero counts only checks against 255. A table description could therefore skip past the last valid symbol and give the next one a count, and the check after the loop allowed up to 256 symbols. On master transform rejected such tables; with (1) alone they would get states with zero addBits and baseline, and a maxBits too low for the refill paths. readNCount now checks the symbol count against the table's own maximum. Test: TestReadNCountSymbolPastTable in seqdec_test.go, an offsets table with a count for symbol 40 after a zero run; it fails without the fix.

Results

Both commits together, measured single-threaded with checksums ignored. For each A/B, every asm TEXT symbol was pinned to 64 bytes in both builds (measurement only), and runs alternated between the two builds. Baseline A is master plus #1212, #1230, #1232 and #1233; B adds this PR. Measured against master alone, the percentages will be slightly larger.

Corpora:

• DecodeAll: 200 MB slices of apache/gob/json/nyc-taxi, each re-encoded with zstd -3, zstd -19 and this package at SpeedDefault, plus the benchdecoder.zip files.
• Streaming: apache.log and cockroach.node1.log as provided, Reader with concurrency 1.

Change in time, B vs A (negative is faster):

	Graviton5 (c9g)	Sapphire Rapids (c7i)
DecodeAll, -3 slices	−3.1%	−2.0%
DecodeAll, -19 slices	−1.7%	−1.4%
DecodeAll, SpeedDefault slices	~0 (n.s.)	−0.5%
DecodeAll, small files	−2.9%	−2.8%
Streaming, apache + cockroach	−1.8%	−2.2%
huff0 BenchmarkDecompress4XTable	−7.8%	−7.6%
Cells regressed by more than 1%	0	0

• Graviton5 per file: apache -3 −4.1%, gob -3 −3.6%, json -3 −3.5%, comp-data.bin −7.1%, geo.protodata −4.7%, html −4.5% (all significant).
• Sapphire Rapids: all 23 non-jpeg DecodeAll cells are faster and significant, from −0.35% to −6.2%.
• BenchmarkDecoder_DecodeAll: 12 of 12 files faster on Graviton5, and 11 of 12 on Sapphire Rapids (fireworks.jpeg unchanged).

As % of libzstd, A → B:

	Graviton5	Sapphire Rapids
apache -3	88.9 → 91.9%	87.9 → 90.0%
apache -19	89.1 → 91.5%	89.6 → 91.0%
comp-data.bin	79.8 → 85.3%	76.5 → 81.2%

Validation

• Tests: go test ./zstd ./huff0 ./fse pass on amd64 (Zen 3) and arm64 (Neoverse-N1). With -tags noasm, zstd passes on both and huff0 on amd64. TestBuildDtableARM64MatchesReference checks the arm64 asm against the Go build.
• Asm: the committed .s files regenerate byte-identically from gen_fse.go.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved validation of Huffman weights and FSE symbol ranges, helping the decoder reject malformed compressed data.
  • Updated decoding-table construction while preserving validation and error handling, and streamlined how table entries are populated.
• Tests
  • Added coverage for invalid and zero-weight Huffman tables, as well as FSE descriptions containing symbols outside the supported range.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 08e47ef4-41b2-4d76-9b9f-abc067de07b5

📥 Commits

Reviewing files that changed from the base of the PR and between bcc082b and 59de0e2.

📒 Files selected for processing (2)

• zstd/blockdec.go
• zstd/seqdec_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The changes update Huffman weight parsing and decoder-table filling. They also change Zstandard FSE decoding to construct table entries from predefined symbol-extension data across generic and assembly implementations.

Changes

Huffman table construction

Layer / File(s)	Summary
Huffman weight parsing and table filling huff0/decompress.go, huff0/decompress_test.go	ReadTable combines weight counts from four histograms and checks for oversized and zero weights. Decoder-table filling handles runs of at least four entries in groups of four. The new test covers valid weights, oversized weights in both scan paths, and all-zero weights.

Zstandard FSE decoder tables

Layer / File(s)	Summary
Predefined symbol-extension data zstd/fse_predefined.go	Initialization builds extended decoder symbols from addBits and baseLine values, and checks that addBits does not decrease across consecutive symbols.
FSE table construction in sequence and dictionary decoding zstd/fse_decoder.go, zstd/blockdec.go, zstd/dict.go, zstd/seqdec_test.go	readNCount takes a table index and uses the corresponding maximum symbol and extension data to build the decoding table. Sequence and dictionary decoding pass the table index and no longer call transform. A regression test checks rejection of an out-of-range symbol.
Combined FSE decoder entries across implementations zstd/fse_decoder_generic.go, zstd/fse_decoder_asm.go, zstd/fse_decoder_amd64.s, zstd/fse_decoder_arm64.s, zstd/_generate/gen_fse.go, zstd/fse_decoder_arm64_test.go, zstd/fse_decoder_bench_test.go	Generic, assembly, and generated builders combine symbol-extension data, bit counts, and next states in decoder-table entries. The assembly reference test and benchmark pass the matching extension data.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~30 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 59de0

No actionable merge-blocking risk is established; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to bcc08

The changed decoder appears to reject out-of-range table symbols before using them, and no introduced security issue was established. Some error-recovery behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — Malformed compressed blocks and dictionary tables are the relevant input surfaces for this validation change; the observed effects stay within decoding and its callers.

Security Findings and Attack Paths

• inferred — No introduced attack path was established: the reviewed malformed-symbol case is rejected before table use, and a block parsing error stops the observed synchronous decode paths. Complete asynchronous recovery coverage remains uncertain.

Trust Boundaries and Controls

• observed — The block caller identifies the sequence-table domain; readNCount enforces that domain’s symbol bound, and buildDtable checks extension length and propagates fill errors.

Resilience and Maintainability Implications

• inferred — Partial table writes on failure make error propagation essential: repeat mode can retain a decoder, but the observed frame and synchronous streaming paths stop on the error rather than decoding a subsequent block with it.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 12 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main changes: improving per-block table setup in both zstd and huff0.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[lizthegrey]

Nice find!