perf(kernel): disable AVX-512 VBMI2 tier by default; defer dict-frame table clear

[polaz]

Summary

Two perf changes, the first is the substantive one:

Disable the AVX-512 VBMI2 decode tier by default. The dashboard's
low-entropy-1m decompress outsiders at level_3_dfast / level_4_dfast
(reported ~6x slower than the C reference, -83%) are an AVX-512 artifact, not
a real regression. Verified ours-vs-c_ffi on three hosts, interleaved,
back-to-back so contamination hits both arms equally:

host	tier	rust	c_ffi	result
i9	AVX2	33.9 µs	45.5 µs	rust 1.34x faster
M1	NEON	24.9 µs	28.1 µs	rust 1.13x faster
ula	SSE4.2 (scalar)	87.0 µs	97.1 µs	rust 1.12x faster

Rust beats the C reference on every tier we can test — including the worst-case
pure-scalar path. The dashboard runner is AVX-512-capable, so the runtime
dispatch selected the VBMI2 tier there; AVX-512 license-based frequency
downclocking stalls the whole (bursty, memory-bound) decode and the heavier
kernel never amortizes, so the VBMI2 tier runs far slower than AVX2 on that
silicon. With kernel_vbmi2 removed from the default feature set, AVX-512 hosts
fall back to the AVX2 tier (faster there); the kernel is kept and opt-in via
--features kernel_vbmi2 for a sustained-AVX-512 workload that benefits.

Defer the dict-frame table clear to prime. The dict-active matcher reset
zero-filled the hash/chain tables, but the reuse hot path immediately restores a
clean primed snapshot over them via clone_from, so that fill was thrown away.
Deferred to prime_with_dictionary (mirrors the Fast matcher's existing
table_overwritten_by_restore skip that the HC path lacked). Byte-identical
output; small win on tiny dict frames.

Verification

The VBMI2 change cannot be measured on the available hosts (none have AVX-512) —
the dashboard CI runner is the AVX-512 test machine. The dashboard decode delta
on low-entropy-1m level_3_dfast / level_4_dfast should move from the -83%
band back toward parity once the runner stops selecting VBMI2.

Testing

• cargo nextest run -p structured-zstd — 789 pass
• cargo clippy --all-targets — clean
• ours-vs-c_ffi interleaved on i9 / M1 / ula (table above)

Summary by CodeRabbit

• Chores
  • Disabled the AVX-512 (kernel_vbmi2) tier from the default feature set; it can still be enabled explicitly via --features kernel_vbmi2.
• Documentation
  • Updated “Quick start” notes to clarify default AVX-512 behavior and dispatch caps on AVX-512 hosts.
• Performance
  • Reduced unnecessary matcher/table clearing work during dictionary reuse.
  • Streamlined FSE encoder state-table initialization.
• Bug Fixes
  • Improved correctness when clearing a dictionary and then encoding a subsequent no-dictionary frame.
• Tests
  • Added a regression test for dictionary-clear → no-dictionary round-trips.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 6f4f9696-2cc3-42de-a3c9-958097dfd79d

📥 Commits

Reviewing files that changed from the base of the PR and between 94bac83 and ee45c19.

📒 Files selected for processing (1)

• zstd/src/fse/fse_encoder.rs

---

📝 Walkthrough

Walkthrough

The PR refactors HashChain dictionary priming by deferring hash-table clearing from MatchTable::reset into a new clear_chain_hash_tables helper called by prime_with_dictionary, extends invalidate_primed_dictionary to reset dictionary-active state for proper frame transitions, adds a regression test for dictionary invalidation, removes kernel_vbmi2 from the default feature set with updated documentation, and optimizes FSE table construction with uninitialized vector allocation.

Changes

HashChain dict prime: deferred hash-table clearing

Layer / File(s)	Summary
Deferred clear in MatchTable::reset and new helper zstd/src/encoding/match_table/storage.rs	The dict-active reuse branch of MatchTable::reset drops its immediate fill(HC_EMPTY) calls on hash/chain tables, retaining only position-bookkeeping rewinds. A new pub(crate) clear_chain_hash_tables helper fills hash_table, hash3_table, and chain_table to HC_EMPTY.
Call site in prime_with_dictionary zstd/src/encoding/match_generator.rs	Inserts a call to matcher.table.clear_chain_hash_tables() in the HashChain branch of prime_with_dictionary, before offset_hist is written and mark_dictionary_primed() is reached, with comments explaining the deferred-clear contract.
Dictionary invalidation state tracking zstd/src/encoding/match_generator.rs	In invalidate_primed_dictionary for HashChain, extends the invalidation by setting table.dictionary_active = false after table.dms.invalidate(), ensuring the next reset does not defer table clearing when the subsequent frame has no dictionary.
Regression test for dictionary invalidation zstd/src/encoding/frame_compressor.rs	Adds clear_dictionary_then_nodict_frame_roundtrips test to verify that after clearing the dictionary on a reused compressor, a subsequent no-dictionary frame with embedded dictionary content still round-trips correctly without stale dict-region matches.

AVX-512 default feature

Layer / File(s)	Summary
Remove kernel_vbmi2 from defaults and update docs zstd/Cargo.toml, README.md	kernel_vbmi2 is removed from the default feature list and replaced with a comment block explaining the performance rationale. README is updated to document that AVX-512 is off by default, dispatch caps at AVX2 on AVX-512 hosts, and kernel_vbmi2 can be enabled explicitly via --features kernel_vbmi2.

FSE encoder optimization

Layer / File(s)	Summary
Uninitialized vector allocation zstd/src/fse/fse_encoder.rs	The state_table_flat allocation in build_table_from_probabilities switches from zero-initialized vec![0u16; table_size] to an unsafe uninitialized Vec created with set_len, relying on the subsequent construction loop to fully overwrite the buffer before use. Safety invariants (spread-phase cycling and cumul partitioning) are enforced with always-on asserts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• structured-world/structured-zstd#337: The PR implements dictionary-aware match table reset optimization and deferred hash-table clearing that directly addresses the dictionary-per-frame prime and per-frame table-zeroing levers in the optimal parser performance analysis.

Possibly related PRs

• structured-world/structured-zstd#307: Both PRs directly modify zstd/Cargo.toml's kernel_vbmi2 decode-kernel feature behavior (default enablement vs explicit opt-in).
• structured-world/structured-zstd#427: Both PRs modify the HashChain dictionary priming and state lifecycle in match_generator.rs and match_table/storage.rs.
• structured-world/structured-zstd#166: Both PRs modify build_table_from_probabilities in zstd/src/fse/fse_encoder.rs with optimizations to state_table_flat construction and safety invariants.

Poem

🐇 No more AVX haste by default,
The chain tables clear when the dict is exalted.
Reset rewinds the floor,
But the fill waits one step more—
A deferred memset, clean and unhalted! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the two main performance optimizations: disabling AVX-512 VBMI2 by default and deferring dictionary-frame table clearing.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/vbmi2-off-and-scratch-reuse

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[greptile-apps]

Greptile Summary

• Disables the AVX-512 VBMI2 decode kernel from the default feature set while keeping it available behind kernel_vbmi2.
• Updates README and feature documentation to clarify default AVX-512 fallback and runtime dispatch behavior.
• Defers HashChain dictionary table clearing to dictionary priming and adds invalidation coverage when dictionaries are removed or replaced.
• Optimizes FSE encoder state-table initialization and adds regression coverage for dictionary clearing followed by no-dictionary compression.

Confidence Score: 5/5

The changes are narrowly scoped performance and feature-default updates with regression coverage for dictionary reuse behavior.

No correctness, safety, or security issues were identified in the changed code, and the described test coverage exercises the affected compression paths.

T-Rex Logs

What T-Rex did

• Observed the base VBMI2 default features include kernel_vbmi2.
• Observed the head VBMI2 default features do not include kernel_vbmi2, while kernel_vbmi2 remains declared and implies kernel_avx2.
• Manifest checks completed with exit code 0, but cargo-based checks could not run because cargo was not found in the execution environment.
• Compared no-dictionary-after-clear round-trip tests between the base and head; the head passed the same test as the base.
• Saved a temporary base test patch at trex-artifacts/dict-clear-deferred-base-test.patch and did not add table-clear instrumentation.
• Before: base checkout and focused FSE test attempt failed to start because cargo was missing (Exit code 127).
• After: head checkout and focused FSE test attempt again failed to start because cargo was missing (Exit code 127).
• Head safety check: attempted Miri command confirms cargo, rustc, and rustup are missing in the environment.

_{Ran code and verified through T-Rex}

_{Reviews (3): Last reviewed commit: "fix(fse): enforce table-init invariant i..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@zstd/src/encoding/match_table/storage.rs`:
- Around line 1351-1365: The HashChain dictionary deactivation is incomplete
because when `invalidate_primed_dictionary()` is called, it only invalidates the
cached dms tree but fails to set `dictionary_active = false`. This causes
`reset()` to defer table clears based on the stale `dictionary_active = true`
flag, but since the dictionary is gone, the expected `prime_with_dictionary()`
or `restore_primed_dictionary()` calls never execute, leaving deferred clears
unprocessed and stale table entries with old absolute positions. To fix this,
modify the `invalidate_primed_dictionary()` method to explicitly set
`dictionary_active = false` for HashChain, ensuring that `reset()` will not
defer the table clear when no dictionary is available for re-priming on the next
frame.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: f964e277-2e80-4847-b8b4-2deef34d68b8

📥 Commits

Reviewing files that changed from the base of the PR and between 74416f2 and 87e7fa7.

📒 Files selected for processing (3)

• zstd/Cargo.toml
• zstd/src/encoding/match_generator.rs
• zstd/src/encoding/match_table/storage.rs

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@zstd/src/fse/fse_encoder.rs`:
- Around line 808-825: The unsafe code using `set_len` on `state_table_flat`
currently depends on invariants that are only verified in debug builds via
debug_assert statements (located around the phase-2 verification logic before
this block). Convert these debug_assert calls to regular assert or assert_eq
calls to ensure the safety contract for full table initialization is enforced in
all build profiles, preventing potential undefined behavior if probability
normalization regresses in release builds.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: b6b98277-7b6e-49a0-be79-0768594c2499

📥 Commits

Reviewing files that changed from the base of the PR and between 87e7fa7 and 94bac83.

📒 Files selected for processing (5)

• README.md
• zstd/src/encoding/frame_compressor.rs
• zstd/src/encoding/match_generator.rs
• zstd/src/encoding/match_table/storage.rs
• zstd/src/fse/fse_encoder.rs