From e4e2ffb8333855c4a4fad6b060165ce1d229cb37 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 15:02:36 +0300 Subject: [PATCH 1/8] perf(opt): walk the repeat offsets as indices, not as a list of options Upstream's optimal finder takes the three repeat offsets by index, with the litLength-0 rotation applied to the INDEX rather than to a materialised list (zstd_opt.c:646-649): repCode runs from ll0 to ZSTD_REP_NUM + ll0, and reads rep[repCode] except for the last slot, which is rep[0] - 1. The zero that slot can hold is discarded by the same bound that discards an out-of-window offset, through an intentional unsigned underflow. Ours built the same three candidates as an [Option; 3] and flattened it. The parser visits nearly every position on input it finds no matches in, so the option machinery ran 9,135 times a frame on the measured fixture and stood in the profile as its own lines (the flatten and its discriminant checks, about 6% of the encode between them). The rotation is now an index and the zero is discarded by the gate that was already there. 209,035 fewer retired instructions a frame, 4,950,812 -> 4,741,777 (-4.2%), on a 10 KiB random payload at level 13 with a 1,280-byte dictionary, musl, i9, three rounds, the count identical to the digit every run. That is 22.9 instructions a position, which is the shape of what was removed. NOT a speed claim: cycles read +1.85%, but the control arm for that pair -- level 1, whose Fast backend never enters this finder, and whose instruction count is bit-identical between the two binaries -- moved -10.8% on its own, so code layout swamps anything the clock could say here. Kept for the operations that are provably gone. Output is byte-identical over 30 fixture-and-level rows, and the dictionary path emits the same 9,179 bytes a frame as before and as the reference. The note above the dictionary descent now states what the code does: it spends what the live walk left of the compare budget, as upstream does (zstd_opt.c:724 spends it, :777 admits the dictionary walk only on the remainder, :782 keeps spending the same counter). Part of #493. --- zstd/src/encoding/hc/generator.rs | 37 ++++++++++++++++++------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/zstd/src/encoding/hc/generator.rs b/zstd/src/encoding/hc/generator.rs index 10acb4715..d8936288f 100644 --- a/zstd/src/encoding/hc/generator.rs +++ b/zstd/src/encoding/hc/generator.rs @@ -509,22 +509,26 @@ macro_rules! bt_insert_and_collect_matches_body { let mut skip_further_match_search = false; let mut rep_len_candidate_found = false; if idx + 4 <= concat.len() { - let rep_offsets: [Option; 3] = if $lit_len == 0 { - [ - Some($reps[1] as usize), - Some($reps[2] as usize), - ($reps[0] > 1).then_some(($reps[0] - 1) as usize), - ] - } else { - [ - Some($reps[0] as usize), - Some($reps[1] as usize), - Some($reps[2] as usize), - ] - }; let rbase = concat.as_ptr(); let rlen = concat.len(); - for rep in rep_offsets.into_iter().flatten() { + // Upstream walks the three repeat offsets as plain indices with the + // litLength-0 rotation applied to the INDEX, not to a materialised + // list (zstd_opt.c:646-649): `repCode` runs from `ll0` to + // `ZSTD_REP_NUM + ll0`, taking `rep[repCode]` except for the last + // slot, which is `rep[0] - 1`. Building the same three candidates as + // `[Option<_>; 3]` and flattening them paid the option machinery on + // every position, and this parser visits nearly every position, so + // the flatten and its discriminant checks stood in the profile as + // their own lines. The zero that the `then_some` used to filter is + // discarded by the `rep == 0` gate below, which is where upstream + // discards it too (its `repOffset-1` underflows past the bound). + let ll0 = usize::from($lit_len == 0); + for rep_code in ll0..3 + ll0 { + let rep = if rep_code == 3 { + ($reps[0] as usize).wrapping_sub(1) + } else { + $reps[rep_code] as usize + }; if rep == 0 || rep > $abs_pos { continue; } @@ -896,8 +900,9 @@ macro_rules! bt_insert_and_collect_matches_body { // Dict dual-probe (upstream zstd `ZSTD_dictMatchState`, zstd_opt.c:777-813): // after the live tree, descend the immutable dictionary BINARY TREE - // (built in `prime_dms_bt`) with its OWN compare budget and push any - // dict match longer than the live best into the ladder. The DUBT + // (built in `prime_dms_bt`) on what the live walk left of the compare + // budget, and push any dict match longer than the live best into the + // ladder. The DUBT // descent reaches the longest dict match efficiently (a hash-chain // surfaced only the few same-bucket candidates and left most of the // dict savings unrealised at btlazy2 / btopt). Dict positions are From 04d332f64675f5c4ea30dbf6a726745678a4d317 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 15:02:59 +0300 Subject: [PATCH 2/8] test(bench): let the cparams probe take a dictionary size The probe hardcoded dictSize 0, so it could not be asked what the reference selects for a dictionary-primed case. Upstream folds the dictionary into the size hint, which can widen the window and with it the chain and hash logs, so 0 is the one case that cannot stand in for the others. It is now the third argument and still defaults to 0. --- zstd/examples/cparams_check.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/zstd/examples/cparams_check.rs b/zstd/examples/cparams_check.rs index 31fb8bbb5..030c7ba31 100644 --- a/zstd/examples/cparams_check.rs +++ b/zstd/examples/cparams_check.rs @@ -1,13 +1,16 @@ //! One-shot diagnostic: ask upstream zstd which cParams it selects for a -//! given (level, srcSize, dictSize=0) tuple via `ZSTD_getCParams`. Useful +//! given (level, srcSize, dictSize) tuple via `ZSTD_getCParams`. Useful //! for checking our per-level table widths (windowLog / hashLog / chainLog) //! against upstream's source-size-adjusted values. //! //! Build: cargo build --release -p ffi-bench --example cparams_check -//! Run: ./target/release/examples/cparams_check [level] [src_size] -//! level compression level (default 1) -//! src_size source size in bytes for the size hint (default 1022035, -//! the decodecorpus-z000033 fixture; 0 = unknown/unbounded) +//! Run: ./target/release/examples/cparams_check [level] [src_size] [dict_size] +//! level compression level (default 1) +//! src_size source size in bytes for the size hint (default 1022035, +//! the decodecorpus-z000033 fixture; 0 = unknown/unbounded) +//! dict_size dictionary size in bytes (default 0). Upstream folds it +//! into the size hint, so a dictionary can widen the window +//! and with it the chain and hash logs. use zstd::zstd_safe::zstd_sys; @@ -15,7 +18,7 @@ fn main() { let args: Vec = std::env::args().collect(); let level: i32 = args.get(1).and_then(|s| s.parse().ok()).unwrap_or(1); let src_size: u64 = args.get(2).and_then(|s| s.parse().ok()).unwrap_or(1022035); - let dict_size = 0usize; + let dict_size: usize = args.get(3).and_then(|s| s.parse().ok()).unwrap_or(0); // SAFETY: standard libzstd query. let cp = unsafe { zstd_sys::ZSTD_getCParams(level, src_size, dict_size) }; From 567bea7f26bfec3d690ce7e0d9940280a73c29af Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 15:21:53 +0300 Subject: [PATCH 3/8] perf(opt): take the rep probe's shared inputs once, not once per offset The three repeat-offset probes share three things and were recomputing all of them on every one of the three: the current position's four-byte gate word (read and masked again each time), the history origin (read off the match table through the same `&mut` the tree walk below writes through, so the optimizer had to reload it rather than keep it), and the tail length from the current position. Upstream reads its own `ip` word through a plain local pointer that nothing aliases, which is what taking these into locals above the loop amounts to. 58,050 fewer retired instructions a frame, 4,741,777 -> 4,683,727 (-1.22%), on 10 KiB random at level 13 with a 1,280-byte dictionary, musl, i9. The count is identical to the digit across runs. Output is byte-identical over 30 fixture-and-level rows and the dictionary path emits the same 9,179 bytes a frame. No speed claim: the session that measured cycles was not quiet enough to carry one, the reference arm alone spreading 9.6% across its own readings. Kept for the operations that are provably gone. Where the rest of this finder's work sits, measured by ablation on the same fixture (retired instructions are deterministic, so one run an arm is exact): the rep probe is 859,410 a frame, 18.1% of the encode, and on this input it changes the output by not one byte; the hash3 probe is 795,248, 16.8%; the tree walk and its insert are about 1,190,547. That is 94 instructions a position for the rep probe against roughly 35 for upstream's, so most of that gap is still there. Part of #493. --- zstd/src/encoding/hc/generator.rs | 44 +++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/zstd/src/encoding/hc/generator.rs b/zstd/src/encoding/hc/generator.rs index d8936288f..3c240da75 100644 --- a/zstd/src/encoding/hc/generator.rs +++ b/zstd/src/encoding/hc/generator.rs @@ -522,6 +522,24 @@ macro_rules! bt_insert_and_collect_matches_body { // their own lines. The zero that the `then_some` used to filter is // discarded by the `rep == 0` gate below, which is where upstream // discards it too (its `repOffset-1` underflows past the bound). + // Everything the three probes share is taken once. The current + // position's gate word does not depend on which repeat offset is + // being tried, and neither does the history origin or the tail + // length, but all three sat inside the loop: the word was re-read + // and re-masked on every repeat, and the origin came off the table + // through the `&mut` that the walk below writes through, so the + // optimizer had to reload it. Upstream reads its own `ip` word + // through a plain local pointer nothing aliases, which is what + // hoisting these amounts to. + let hist_start = $table.history_abs_start; + let cur_tail = rlen - idx; + // SAFETY: `idx + 4 <= rlen` from the guard above. + let cur_word = unsafe { rbase.add(idx).cast::().read_unaligned().to_le() }; + let cur_gate = if $min_match_len == 3 { + cur_word & 0x00FF_FFFF + } else { + cur_word + }; let ll0 = usize::from($lit_len == 0); for rep_code in ll0..3 + ll0 { let rep = if rep_code == 3 { @@ -533,25 +551,23 @@ macro_rules! bt_insert_and_collect_matches_body { continue; } let candidate_pos = $abs_pos - rep; - if candidate_pos < $table.history_abs_start { + if candidate_pos < hist_start { continue; } - let candidate_idx = candidate_pos - $table.history_abs_start; - // SAFETY: `idx + 4 <= rlen` (guard above) and `candidate_idx < idx` - // (rep >= 1), so both 4-byte reads stay inside `concat`. - let gate_matches = unsafe { - let cand = rbase.add(candidate_idx).cast::().read_unaligned(); - let cur = rbase.add(idx).cast::().read_unaligned(); - if $min_match_len == 3 { - (cand.to_le() & 0x00FF_FFFF) == (cur.to_le() & 0x00FF_FFFF) - } else { - cand == cur - } + let candidate_idx = candidate_pos - hist_start; + // SAFETY: `candidate_idx < idx` (rep >= 1) and `idx + 4 <= rlen`, + // so the 4-byte read stays inside `concat`. + let cand_word = + unsafe { rbase.add(candidate_idx).cast::().read_unaligned().to_le() }; + let cand_gate = if $min_match_len == 3 { + cand_word & 0x00FF_FFFF + } else { + cand_word }; - if !gate_matches { + if cand_gate != cur_gate { continue; } - let rmax = (rlen - candidate_idx).min(rlen - idx).min(tail_limit); + let rmax = (rlen - candidate_idx).min(cur_tail).min(tail_limit); // SAFETY: same umbrella; both pointers + `rmax` stay in `concat`. let match_len = unsafe { $cpl(rbase.add(candidate_idx), rbase.add(idx), rmax) }; if match_len < $min_match_len { From 543bc2ce7cbdc0a153bfec7f0de18b27f7b03f8d Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 17:09:31 +0300 Subject: [PATCH 4/8] perf(opt): gate the hash3 probe before the vector compare, and share the table reads The short-match probe entered the vector prefix compare on every position it had a bucket hit for. The bucket is keyed on three bytes and the shortest match this parser accepts is three, so three bytes that differ cannot produce a candidate: they are now checked with one four-byte load and a masked xor before the compare runs. Upstream reaches the same early exit through the first word its ZSTD_count loads, which is why its probe costs a fraction of what ours did. Two smaller things go with it. The bucket read was a bounds-checked slice get with an empty-slot fallback, for a slot the hash cannot put out of range (it is masked to hash3_log bits and the table is 1 << hash3_log wide); it is now a direct read under a debug assertion, as upstream indexes hashTable3[hash3]. And the history origin and the live-history pointer and length, which both probes and the walk all wanted, came off the match table each time through the same &mut the walk writes through; they are taken once at the top of the body. Per frame on 10 KiB random with a 1,280-byte dictionary, musl, i9, arms alternating in one session, three rounds, ranges not overlapping: level 13 2,104,123 -> 1,984,634 cycles -5.68% 4,683,578 -> 4,607,622 insn -1.62% level 19 2,218,754 -> 2,096,793 cycles -5.50% 4,931,907 -> 4,853,487 insn -1.59% Against libzstd on the same runs, level 13 goes from 1.358x to 1.281x of its cycles. Cycles fall three times faster than the instruction count, which is the point: what the gate removes is the vector compare's setup on input that mismatches immediately, not a couple of scalar operations. Output byte-identical over 30 fixture-and-level rows, and the dictionary path emits the same 9,179 bytes a frame. Costs 2.43% at level 1 on an 8 MiB access log (131,756,579 -> 134,957,977 cycles), where retired instructions are bit-identical between the two binaries and the Fast backend never enters this finder at all. It is code layout: the level-1 hot function moves from a 32-byte boundary to 48 mod 64. Stable across three sessions, so it is real time and it is reported, but it is not work this change added. Part of #493. --- zstd/src/encoding/hc/generator.rs | 72 ++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 25 deletions(-) diff --git a/zstd/src/encoding/hc/generator.rs b/zstd/src/encoding/hc/generator.rs index 3c240da75..5caa6b0bf 100644 --- a/zstd/src/encoding/hc/generator.rs +++ b/zstd/src/encoding/hc/generator.rs @@ -508,9 +508,16 @@ macro_rules! bt_insert_and_collect_matches_body { // this frame (inlining the same probes into the DP caller regressed). ===== let mut skip_further_match_search = false; let mut rep_len_candidate_found = false; - if idx + 4 <= concat.len() { - let rbase = concat.as_ptr(); - let rlen = concat.len(); + // Shared by both probes below. The history origin and the live-history + // pointer/length are fixed for this position but live on the match + // table, which the tree walk further down writes through, so each + // reader was fetching them again. + let hist_start = $table.history_abs_start; + let cbase = concat.as_ptr(); + let clen = concat.len(); + if idx + 4 <= clen { + let rbase = cbase; + let rlen = clen; // Upstream walks the three repeat offsets as plain indices with the // litLength-0 rotation applied to the INDEX, not to a materialised // list (zstd_opt.c:646-649): `repCode` runs from `ll0` to @@ -524,14 +531,10 @@ macro_rules! bt_insert_and_collect_matches_body { // discards it too (its `repOffset-1` underflows past the bound). // Everything the three probes share is taken once. The current // position's gate word does not depend on which repeat offset is - // being tried, and neither does the history origin or the tail - // length, but all three sat inside the loop: the word was re-read - // and re-masked on every repeat, and the origin came off the table - // through the `&mut` that the walk below writes through, so the - // optimizer had to reload it. Upstream reads its own `ip` word - // through a plain local pointer nothing aliases, which is what - // hoisting these amounts to. - let hist_start = $table.history_abs_start; + // being tried, and neither does the tail length, but both sat + // inside the loop: the word was re-read and re-masked on every + // repeat. Upstream reads its own `ip` word through a plain local + // pointer nothing aliases, which is what hoisting these amounts to. let cur_tail = rlen - idx; // SAFETY: `idx + 4 <= rlen` from the guard above. let cur_word = unsafe { rbase.add(idx).cast::().read_unaligned().to_le() }; @@ -609,23 +612,25 @@ macro_rules! bt_insert_and_collect_matches_body { // call): table lookup + one common-prefix scan via `$cpl`, reusing // the BT collect's `concat` / `idx` / `tail_limit`. Labeled block so // the probe's early-outs yield None without returning from the walk. + let h3_log = $table.hash3_log; let h3_candidate: Option<$crate::encoding::opt::types::MatchCandidate> = - if $table.hash3_log == 0 || idx + 4 > concat.len() { + if h3_log == 0 || idx + 4 > clen { None } else { 'h3: { let hh = $crate::encoding::match_table::storage::MatchTable::hash_position_at( - concat, - idx, - $table.hash3_log, - 3, + concat, idx, h3_log, 3, ); - let entry = $table - .hash3_table() - .get(hh) - .copied() - .unwrap_or($crate::encoding::match_table::storage::HC_EMPTY); + // The hash is masked to `h3_log` bits and the table is + // `1 << h3_log` slots wide, so the slot is in range by + // construction and the bounds-checked slice read plus + // its empty-slot fallback were paying for a case that + // cannot arise. Upstream indexes `hashTable3[hash3]` + // directly for the same reason. + debug_assert_eq!($table.hash3_table().len(), 1usize << h3_log); + // SAFETY: `hh < 1 << h3_log == hash3_table().len()`. + let entry = unsafe { *$table.hash3_table().get_unchecked(hh) }; let Some(cand_abs) = $crate::encoding::match_table::storage::MatchTable::stored_abs_position_fast( entry, @@ -635,17 +640,34 @@ macro_rules! bt_insert_and_collect_matches_body { else { break 'h3 None; }; - if cand_abs < $table.history_abs_start || cand_abs >= $abs_pos { + if cand_abs < hist_start || cand_abs >= $abs_pos { break 'h3 None; } let off = $abs_pos - cand_abs; if off >= $crate::encoding::bt::HC3_MAX_OFFSET { break 'h3 None; } - let cand_idx = cand_abs - $table.history_abs_start; - let hbase = concat.as_ptr(); + let cand_idx = cand_abs - hist_start; + // The bucket is keyed on three bytes and the shortest + // match this parser accepts is three, so three bytes + // that differ cannot produce a candidate: check them + // before entering the vector compare, which on input + // that mismatches immediately costs more than the + // answer. Upstream reaches the same early exit through + // the first word its `ZSTD_count` loads. + // SAFETY: `cand_idx < idx` and `idx + 4 <= clen`, so + // both four-byte reads stay inside the live history. + let (cand_head, cur_head) = unsafe { + ( + cbase.add(cand_idx).cast::().read_unaligned().to_le(), + cbase.add(idx).cast::().read_unaligned().to_le(), + ) + }; + if (cand_head ^ cur_head) & 0x00FF_FFFF != 0 { + break 'h3 None; + } // SAFETY: cand_idx/idx within history; tail_limit bounds the scan. - let ml = unsafe { $cpl(hbase.add(cand_idx), hbase.add(idx), tail_limit) }; + let ml = unsafe { $cpl(cbase.add(cand_idx), cbase.add(idx), tail_limit) }; (ml >= $min_match_len).then_some( $crate::encoding::opt::types::MatchCandidate { start: $abs_pos, From 8fa04751af5a783683254e6f8046086b7ce7869c Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 18:16:56 +0300 Subject: [PATCH 5/8] perf(opt): stop marshalling the cost profile into the finder every position The per-position finder took a 24-byte cost profile by value. System V passes a struct that size in memory, so its prologue copied it into the frame with a vector move plus a third load before any work started, on every one of the 9,135 positions a frame this fixture searches. It read two of the four fields. One of those two is an associated const of the strategy the finder is already monomorphized for, so it needs no argument at all and now arrives as a literal. The other is not a const despite the profile's own docs saying every field is: the pass rewrites "sufficient_match_len" before the parse ("sufficient_match_len_for_pass", so btultra2's seed pass runs a different length from its main pass), and it now crosses as one scalar in a register. The profile itself no longer crosses at all. Per frame on 10 KiB random at level 13 with a 1,280-byte dictionary, musl, i9, three runs at +-0.05%: 1,984,634 -> 1,962,640 cycles (-1.11%) and 4,607,622 -> 4,552,000 retired instructions (-1.21%). Against libzstd on the same fixture that is 1.281x -> 1.267x of its cycles. Output byte-identical over 30 fixture-and-level rows. Reading BOTH values off the strategy's consts measured better still, -3.06%, and was wrong: it replaced the per-pass length with the raw const and moved the output on four of those thirty rows. The byte check is what caught it, which is the whole reason it runs before the timer. Two tests set the chain depth through the profile they handed in. Production never varied that value, so they now set "table.search_depth", which is the knob the walk actually reads and the one production does vary. Part of #493. --- zstd/src/encoding/hc/generator.rs | 17 +++-- zstd/src/encoding/hc/optimal.rs | 63 ++++++++--------- zstd/src/encoding/match_generator/tests.rs | 78 ++++++---------------- zstd/src/encoding/match_table/storage.rs | 15 +++-- 4 files changed, 76 insertions(+), 97 deletions(-) diff --git a/zstd/src/encoding/hc/generator.rs b/zstd/src/encoding/hc/generator.rs index 5caa6b0bf..276217c01 100644 --- a/zstd/src/encoding/hc/generator.rs +++ b/zstd/src/encoding/hc/generator.rs @@ -475,7 +475,8 @@ macro_rules! bt_insert_and_collect_matches_body { $search_depth:expr, $abs_pos:ident, $current_abs_end:ident, - $profile:ident, + $sufficient_len:expr, + $max_chain_depth:expr, $min_match_len:ident, $best_len_for_skip:ident, $out:ident, @@ -587,7 +588,7 @@ macro_rules! bt_insert_and_collect_matches_body { }, $min_match_len, ); - if match_len > $profile.sufficient_match_len + if match_len > $sufficient_len || $abs_pos + match_len >= $current_abs_end { skip_further_match_search = true; @@ -685,7 +686,7 @@ macro_rules! bt_insert_and_collect_matches_body { $min_match_len, ); if !rep_len_candidate_found - && (h3.match_len > $profile.sufficient_match_len + && (h3.match_len > $sufficient_len || $abs_pos + h3.match_len >= $current_abs_end) { $table.skip_insert_until_abs = $abs_pos + 1; @@ -809,7 +810,15 @@ macro_rules! bt_insert_and_collect_matches_body { // for the full discussion of the upstream `STREAM_ABS_HEADROOM` // cap in `MatchTable::add_data`. let mut match_end_abs = $abs_pos + 9; - let mut compares_left = $profile.max_chain_depth.min($search_depth); + // Both of these are associated consts of the strategy the caller is + // monomorphized for, so they arrive as literals rather than as fields + // of a 24-byte profile the caller had to marshal through memory on + // every position: System V passes a struct that size in memory, and the + // prologue was copying it into the frame with a vector move before any + // work started. Upstream reads the same values off `cParams` through a + // pointer it already holds, and its finder is inlined into the parser + // loop, so it never marshals anything per position either. + let mut compares_left = ($max_chain_depth).min($search_depth); let mut common_length_smaller = 0usize; let mut common_length_larger = 0usize; let pair_idx = $table.bt_pair_index_for_abs($abs_pos); diff --git a/zstd/src/encoding/hc/optimal.rs b/zstd/src/encoding/hc/optimal.rs index b9b0870bd..ed4aa05f5 100644 --- a/zstd/src/encoding/hc/optimal.rs +++ b/zstd/src/encoding/hc/optimal.rs @@ -185,7 +185,7 @@ macro_rules! build_optimal_plan_impl_body { $self.$collect::<$strategy_ty>( $current_abs_start + skipped_literals, current_abs_end, - profile, + profile.sufficient_match_len, HcCandidateQuery { reps: initial_reps, lit_len: initial_litlen + skipped_literals, @@ -293,7 +293,7 @@ macro_rules! build_optimal_plan_impl_body { $self.$collect::<$strategy_ty>( $current_abs_start, current_abs_end, - profile, + profile.sufficient_match_len, HcCandidateQuery { reps: initial_reps, lit_len: initial_litlen, @@ -651,7 +651,7 @@ macro_rules! build_optimal_plan_impl_body { $self.$collect::<$strategy_ty>( abs_pos, current_abs_end, - profile, + profile.sufficient_match_len, HcCandidateQuery { reps: nodes.get_unchecked(pos).reps, lit_len: nodes.get_unchecked(pos).litlen as usize, @@ -976,7 +976,7 @@ macro_rules! collect_optimal_candidates_initialized_body { $strategy_ty:ty, $abs_pos:ident, $current_abs_end:ident, - $profile:ident, + $sufficient_match_len:ident, $query:ident, $out:ident, $bt_insert_step:ident, @@ -1072,7 +1072,12 @@ macro_rules! collect_optimal_candidates_initialized_body { bt_search_depth, $abs_pos, $current_abs_end, - $profile, + // Not a strategy const: the pass adjusts it (btultra2's seed + // pass runs a different sufficient length from its main pass), + // so it arrives as a scalar. The chain depth beside it IS the + // strategy's own const and needs no argument at all. + $sufficient_match_len, + <$strategy_ty as crate::encoding::strategy::Strategy>::MAX_CHAIN_DEPTH, min_match_len, best_len_ref, $out, @@ -1884,7 +1889,7 @@ impl HcMatchGenerator { &mut self, abs_pos: usize, current_abs_end: usize, - profile: HcOptimalCostProfile, + sufficient_match_len: usize, query: HcCandidateQuery, out: &mut Vec, ) { @@ -1900,7 +1905,7 @@ impl HcMatchGenerator { .collect_optimal_candidates_initialized::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ), @@ -1908,7 +1913,7 @@ impl HcMatchGenerator { .collect_optimal_candidates_initialized::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ), @@ -1916,14 +1921,14 @@ impl HcMatchGenerator { .collect_optimal_candidates_initialized::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ), StrategyTag::BtOpt => self.collect_optimal_candidates_initialized::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ), @@ -1957,7 +1962,7 @@ impl HcMatchGenerator { &mut self, abs_pos: usize, current_abs_end: usize, - profile: HcOptimalCostProfile, + sufficient_match_len: usize, query: HcCandidateQuery, out: &mut Vec, ) { @@ -1970,7 +1975,7 @@ impl HcMatchGenerator { self.collect_optimal_candidates_initialized_neon::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ) @@ -1984,7 +1989,7 @@ impl HcMatchGenerator { self.collect_optimal_candidates_initialized_avx2_bmi2::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ) @@ -1994,7 +1999,7 @@ impl HcMatchGenerator { self.collect_optimal_candidates_initialized_sse2::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ) @@ -2004,7 +2009,7 @@ impl HcMatchGenerator { self.collect_optimal_candidates_initialized_sse42::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ) @@ -2012,7 +2017,7 @@ impl HcMatchGenerator { FastpathKernel::Scalar => self.collect_optimal_candidates_initialized_scalar::( abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, ), @@ -2031,7 +2036,6 @@ impl HcMatchGenerator { self.collect_optimal_candidates_initialized_simd128::( abs_pos, current_abs_end, - profile, query, out, ) @@ -2054,7 +2058,6 @@ impl HcMatchGenerator { self.collect_optimal_candidates_initialized_scalar::( abs_pos, current_abs_end, - profile, query, out, ) @@ -2078,7 +2081,7 @@ impl HcMatchGenerator { &mut self, abs_pos: usize, current_abs_end: usize, - profile: HcOptimalCostProfile, + sufficient_match_len: usize, query: HcCandidateQuery, out: &mut Vec, ) { @@ -2087,7 +2090,7 @@ impl HcMatchGenerator { S, abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, bt_insert_step_no_rebase_neon, @@ -2107,7 +2110,7 @@ impl HcMatchGenerator { &mut self, abs_pos: usize, current_abs_end: usize, - profile: HcOptimalCostProfile, + sufficient_match_len: usize, query: HcCandidateQuery, out: &mut Vec, ) { @@ -2116,7 +2119,7 @@ impl HcMatchGenerator { S, abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, bt_insert_step_no_rebase_sse2, @@ -2142,7 +2145,7 @@ impl HcMatchGenerator { &mut self, abs_pos: usize, current_abs_end: usize, - profile: HcOptimalCostProfile, + sufficient_match_len: usize, query: HcCandidateQuery, out: &mut Vec, ) { @@ -2151,7 +2154,7 @@ impl HcMatchGenerator { S, abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, bt_insert_step_no_rebase_sse2, @@ -2171,7 +2174,7 @@ impl HcMatchGenerator { &mut self, abs_pos: usize, current_abs_end: usize, - profile: HcOptimalCostProfile, + sufficient_match_len: usize, query: HcCandidateQuery, out: &mut Vec, ) { @@ -2180,7 +2183,7 @@ impl HcMatchGenerator { S, abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, bt_insert_step_no_rebase_avx2_bmi2, @@ -2207,7 +2210,7 @@ impl HcMatchGenerator { &mut self, abs_pos: usize, current_abs_end: usize, - profile: HcOptimalCostProfile, + sufficient_match_len: usize, query: HcCandidateQuery, out: &mut Vec, ) { @@ -2216,7 +2219,7 @@ impl HcMatchGenerator { S, abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, bt_insert_step_no_rebase_simd128, @@ -2239,7 +2242,7 @@ impl HcMatchGenerator { &mut self, abs_pos: usize, current_abs_end: usize, - profile: HcOptimalCostProfile, + sufficient_match_len: usize, query: HcCandidateQuery, out: &mut Vec, ) { @@ -2248,7 +2251,7 @@ impl HcMatchGenerator { S, abs_pos, current_abs_end, - profile, + sufficient_match_len, query, out, bt_insert_step_no_rebase_scalar, diff --git a/zstd/src/encoding/match_generator/tests.rs b/zstd/src/encoding/match_generator/tests.rs index ee4f89f67..15eead3af 100644 --- a/zstd/src/encoding/match_generator/tests.rs +++ b/zstd/src/encoding/match_generator/tests.rs @@ -1403,23 +1403,23 @@ fn hc_collect_optimal_candidates_keeps_reps_when_chain_depth_zero() { // BT strategy (BtOpt shares Lazy's OPT_LEVEL=0 / USE_HASH3=false consts). hc.strategy_tag = crate::encoding::strategy::StrategyTag::BtOpt; hc.hc.search_depth = 0; + // The finder caps its walk at `table.search_depth`, and takes the other + // half of that bound from the strategy's associated const rather than from + // a value a caller hands it. So zero depth has to be set where the walk + // reads it; `hc.search_depth` above is the configure-time source and does + // not reach the walk on its own. + hc.table.search_depth = 0; hc.table.history = b"xyzxyzxyzxyz".to_vec(); hc.table.history_start = 0; hc.table.history_abs_start = 0; let abs_pos = 6usize; let current_abs_end = hc.table.history.len(); - let profile = HcOptimalCostProfile { - max_chain_depth: 0, - sufficient_match_len: usize::MAX / 2, - accurate: false, - favor_small_offsets: false, - }; let mut out = Vec::new(); hc.collect_optimal_candidates( abs_pos, current_abs_end, - profile, + usize::MAX / 2, HcCandidateQuery { reps: [3, 6, 9], lit_len: 1, @@ -1449,17 +1449,11 @@ fn hc_collect_optimal_candidates_panics_for_non_bt_strategy() { hc.table.history_start = 0; hc.table.history_abs_start = 0; hc.table.ensure_tables(); - let profile = HcOptimalCostProfile { - max_chain_depth: 0, - sufficient_match_len: usize::MAX / 2, - accurate: false, - favor_small_offsets: false, - }; let mut out = Vec::new(); hc.collect_optimal_candidates( 6, hc.table.history.len(), - profile, + usize::MAX / 2, HcCandidateQuery { reps: [1, 2, 3], lit_len: 1, @@ -1496,18 +1490,13 @@ fn hc_collect_optimal_candidates_dispatches_every_bt_strategy() { hc.table.chain_log = 8; hc.table.hash3_log = 8; hc.table.ensure_tables(); + hc.table.search_depth = 8; let abs_pos = 12usize; - let profile = HcOptimalCostProfile { - max_chain_depth: 8, - sufficient_match_len: usize::MAX / 2, - accurate: false, - favor_small_offsets: false, - }; let mut out = Vec::new(); hc.collect_optimal_candidates( abs_pos, hc.table.history.len(), - profile, + usize::MAX / 2, HcCandidateQuery { // Reps past abs_pos are skipped, so the only candidate source is // the (hash3 / BT) match finder — keeping the observable clean. @@ -1540,17 +1529,12 @@ fn hc_collect_optimal_candidates_rep_tail_match_skips_chain_probe() { hc.table.ensure_tables(); hc.table.insert_positions(0, abs_pos); - let profile = HcOptimalCostProfile { - max_chain_depth: 32, - sufficient_match_len: usize::MAX / 2, - accurate: true, - favor_small_offsets: false, - }; + hc.table.search_depth = 32; let mut out = Vec::new(); hc.collect_optimal_candidates( abs_pos, hc.table.history.len(), - profile, + usize::MAX / 2, HcCandidateQuery { reps: [1, 4, 8], lit_len: 1, @@ -1580,17 +1564,12 @@ fn hc_collect_optimal_candidates_long_chain_match_advances_skip_window() { hc.table.insert_positions(0, abs_pos); hc.table.skip_insert_until_abs = 0; - let profile = HcOptimalCostProfile { - max_chain_depth: 32, - sufficient_match_len: usize::MAX / 2, - accurate: true, - favor_small_offsets: false, - }; + hc.table.search_depth = 32; let mut out = Vec::new(); hc.collect_optimal_candidates( abs_pos, hc.table.history.len(), - profile, + usize::MAX / 2, HcCandidateQuery { reps: [1, 4, 8], lit_len: 1, @@ -1618,18 +1597,12 @@ fn hc_collect_optimal_candidates_advances_skip_window_on_plain_bt_path() { let abs_pos = 8usize; hc.table.skip_insert_until_abs = 0; - - let profile = HcOptimalCostProfile { - max_chain_depth: 0, - sufficient_match_len: usize::MAX / 2, - accurate: true, - favor_small_offsets: false, - }; + hc.table.search_depth = 0; let mut out = Vec::new(); hc.collect_optimal_candidates( abs_pos, hc.table.history.len(), - profile, + usize::MAX / 2, HcCandidateQuery { reps: [1, 4, 8], lit_len: 1, @@ -1673,17 +1646,12 @@ fn hc_ldm_candidates_are_merged_into_optimal_candidates() { match_len: 40, }; - let profile = HcOptimalCostProfile { - max_chain_depth: 0, - sufficient_match_len: usize::MAX / 2, - accurate: true, - favor_small_offsets: false, - }; + hc.table.search_depth = 0; let mut out = Vec::new(); hc.collect_optimal_candidates( abs_pos, current_abs_end, - profile, + usize::MAX / 2, HcCandidateQuery { reps: [1, 4, 8], lit_len: 1, @@ -1739,12 +1707,6 @@ fn btultra_and_btultra2_both_keep_dictionary_candidates() { hc.table.skip_insert_until_abs = 0; }; - let profile = HcOptimalCostProfile { - max_chain_depth: 32, - sufficient_match_len: usize::MAX / 2, - accurate: true, - favor_small_offsets: false, - }; let abs_pos = 96usize; let mut out = Vec::new(); @@ -1754,7 +1716,7 @@ fn btultra_and_btultra2_both_keep_dictionary_candidates() { hc.collect_optimal_candidates( abs_pos, 160, - profile, + usize::MAX / 2, HcCandidateQuery { reps: [1, 4, 8], lit_len: 1, @@ -1773,7 +1735,7 @@ fn btultra_and_btultra2_both_keep_dictionary_candidates() { hc.collect_optimal_candidates( abs_pos, 160, - profile, + usize::MAX / 2, HcCandidateQuery { reps: [1, 4, 8], lit_len: 1, diff --git a/zstd/src/encoding/match_table/storage.rs b/zstd/src/encoding/match_table/storage.rs index 9fdbb4b4d..0627b4efc 100644 --- a/zstd/src/encoding/match_table/storage.rs +++ b/zstd/src/encoding/match_table/storage.rs @@ -1990,7 +1990,8 @@ impl MatchTable { search_depth, abs_pos, current_abs_end, - profile, + profile.sufficient_match_len, + profile.max_chain_depth, min_match_len, best_len_for_skip, out, @@ -2030,7 +2031,8 @@ impl MatchTable { search_depth, abs_pos, current_abs_end, - profile, + profile.sufficient_match_len, + profile.max_chain_depth, min_match_len, best_len_for_skip, out, @@ -2070,7 +2072,8 @@ impl MatchTable { search_depth, abs_pos, current_abs_end, - profile, + profile.sufficient_match_len, + profile.max_chain_depth, min_match_len, best_len_for_skip, out, @@ -2111,7 +2114,8 @@ impl MatchTable { search_depth, abs_pos, current_abs_end, - profile, + profile.sufficient_match_len, + profile.max_chain_depth, min_match_len, best_len_for_skip, out, @@ -2149,7 +2153,8 @@ impl MatchTable { search_depth, abs_pos, current_abs_end, - profile, + profile.sufficient_match_len, + profile.max_chain_depth, min_match_len, best_len_for_skip, out, From d2b906c9e8e4294ae756d974ac71c6fdfb3e9da2 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 19:34:05 +0300 Subject: [PATCH 6/8] docs(build): record that pinning function alignment is slower on the grid An edit to code one compression level never executes can still move that level's timing by a few percent with the retired instruction count bit-identical, because the encoder's hot functions are large enough that anything added or removed shifts the ones after it across cache lines. Forcing every function onto a 64-byte boundary looks like the cure. It is not. The same commit built with and without "-C llvm-args=-align-all-functions=6", i9, three rounds each, per frame: decodecorpus z000033, level 5 62.41 M -> 67.68 M cycles (+8.4%) 8 MiB access log, level 9 679.19 M -> 710.21 M (+4.6%) 8 MiB access log, level 1 131.85 M -> 134.53 M (+2.0%) z000033 at levels 1 / 13 / 19 (+0.6..1.3%) 2 MiB incompressible, level 1 (-0.1%) Slower on everything that moves at all. Two hand-picked cases had said the opposite, and the grid is what corrected them. The note also records the second half of it: the layout shift is a property of the binary, not of the change. The same two encoders that differ by up to 10% at level 5 in the small loop example differ by 0.02-0.8% in the CLI. So a cycle delta with identical instruction counts belongs to the binary it was measured in, and the instruction count is what says whether a change altered the work. Part of #493. --- Cargo.toml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 93e0769b6..dd66e22e0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,3 +28,30 @@ debug = "line-tables-only" inherits = "bench" debug = "full" strip = "none" + +# Do NOT pin function alignment here, and do not add a workspace +# `.cargo/config.toml` that does it either. +# +# The encoder's hot functions are big enough that an edit anywhere moves the +# ones after it across cache lines, so a change to code one level never +# executes can still shift that level's timing by a few percent with the +# retired instruction count bit-identical. Forcing every function onto a +# 64-byte boundary (`-C llvm-args=-align-all-functions=6`) looks like the cure +# and is not: measured on the i9, the SAME commit built with and without the +# flag, three rounds each, per frame — +# +# decodecorpus z000033, level 5 62.41 M -> 67.68 M cycles (+8.4%) +# 8 MiB access log, level 9 679.19 M -> 710.21 M (+4.6%) +# 8 MiB access log, level 1 131.85 M -> 134.53 M (+2.0%) +# z000033, levels 1 / 13 / 19 (+0.6..1.3%) +# 2 MiB incompressible, level 1 (-0.1%) +# +# It is slower on everything that moves. Two hand-picked cases said otherwise +# before the grid was run, which is the whole reason the grid gets run. +# +# The layout shift itself is also binary-dependent, so it is not a property of +# any change: the same pair of encoders that differ by up to 10% in the small +# `encode_loop_dict` example differ by 0.02-0.8% in the CLI. Read instruction +# counts for what a change did to the work, and treat a cycle delta with +# identical instruction counts as the binary it was measured in, not as a +# regression to chase. From 87c04c4ff6497e3297c927b19b2ff40eeb0f8fc7 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 20:07:17 +0300 Subject: [PATCH 7/8] fix(opt): pass the sufficient length to the wasm and portable dispatch arms The finder's dispatcher has six arms and no host compiles more than half of them. Threading the sufficient match length through it reached the NEON arm and the three x86 ones, which is everything an aarch64 or x86 check builds, and left the wasm simd128 arm and the portable fallback calling with one argument short. Both are compile errors on their own targets, and the wasm CI job is where it surfaced. Verified by running what CI runs: clippy for wasm32-unknown-unknown with kernel-simd128 and +simd128, the same for kernel-scalar, and the embedded --no-default-features --features kernel-scalar,hash build. All clean. The dispatcher now says in its docs that its arms have to be updated by reading rather than by compiling, and which two commands cover the ones a development host cannot see. --- zstd/src/encoding/hc/optimal.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/zstd/src/encoding/hc/optimal.rs b/zstd/src/encoding/hc/optimal.rs index ed4aa05f5..60a49007d 100644 --- a/zstd/src/encoding/hc/optimal.rs +++ b/zstd/src/encoding/hc/optimal.rs @@ -1956,6 +1956,16 @@ impl HcMatchGenerator { /// calls the matching `_` variant directly. This entry is kept /// for the cfg(test)-only `collect_optimal_candidates` shim and any /// future caller that isn't already inside a kernel umbrella. + /// + /// Six arms, and no host compiles more than half of them: aarch64 sees the + /// NEON one, x86 sees three behind a runtime match, and the wasm and + /// portable ones are only reachable on their own targets. So a change to + /// this function's argument list has to be applied to every arm by reading, + /// not by compiling — a local `cargo check` on either development + /// architecture will happily accept a call that is missing an argument in + /// the arms it cannot see. `cargo clippy --target wasm32-unknown-unknown` + /// and a `--no-default-features --features kernel-scalar` build are what + /// cover the rest. #[allow(dead_code)] #[inline(always)] pub(crate) fn collect_optimal_candidates_initialized( @@ -2036,6 +2046,7 @@ impl HcMatchGenerator { self.collect_optimal_candidates_initialized_simd128::( abs_pos, current_abs_end, + sufficient_match_len, query, out, ) @@ -2058,6 +2069,7 @@ impl HcMatchGenerator { self.collect_optimal_candidates_initialized_scalar::( abs_pos, current_abs_end, + sufficient_match_len, query, out, ) From 22f613856e38675e3010b33244de6006329566d5 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 20:39:52 +0300 Subject: [PATCH 8/8] docs(opt): record what the litLength-0 slot's wrap costs as a branch Rejecting the synthetic `rep[0] - 1` slot at its origin, with a plain subtraction behind a `reps[0] <= 1` guard, is the shape a per-position gate is supposed to take here: a branch rather than a value the following bound happens to discard. Measured, it is the more expensive shape. Per frame, arms alternating in one session, three rounds: 10 KiB random + 1,280 B dict, level 13 1,964,624 -> 2,109,753 (+7.39%) 10 KiB random + 1,280 B dict, level 19 2,077,461 -> 2,199,987 (+5.90%) decodecorpus + 16 KiB dict, level 17 646.51 M -> 662.77 M (+2.52%) incompressible, level 1 (control) (-0.87%) Retired instructions rise with the cycles, about 2% on the small fixture, and the control arm's are bit-identical, so this is added work and not layout: one more branch in one of three slots stops the three folding together. Output is byte-identical either way, over sixty fixture, level and dictionary rows. So the wrap stays, and the reason is now at the code with its numbers. Upstream writes the same rejection the same way, as an intentional unsigned overflow that "discards 0 and -1" (zstd_opt.c:653). Part of #493. --- zstd/src/encoding/hc/generator.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/zstd/src/encoding/hc/generator.rs b/zstd/src/encoding/hc/generator.rs index 276217c01..b5a371739 100644 --- a/zstd/src/encoding/hc/generator.rs +++ b/zstd/src/encoding/hc/generator.rs @@ -546,6 +546,22 @@ macro_rules! bt_insert_and_collect_matches_body { }; let ll0 = usize::from($lit_len == 0); for rep_code in ll0..3 + ll0 { + // The synthetic slot's wrap is deliberate and measured. Guarding + // `reps[0] <= 1` before a plain subtraction, so the slot is + // rejected at its origin rather than through a value the bound + // below discards, is the shape this codebase asks for on a + // per-position path — and here it costs: +7.4% cycles at level + // 13 and +5.9% at level 19 on 10 KiB random with a dictionary, + // +2.5% on the corpus at level 17, with retired instructions up + // 2% alongside them and the control arm flat, so it is added + // work rather than layout. One extra branch in one of three + // slots stops the three from folding together. + // + // Upstream writes the same rejection the same way, as an + // intentional unsigned overflow that "discards 0 and -1" + // (zstd_opt.c:653). The outcome is identical either way: a + // `reps[0]` of 0 wraps past `abs_pos` and one of 1 becomes the + // zero the next line rejects. let rep = if rep_code == 3 { ($reps[0] as usize).wrapping_sub(1) } else {