From a2b6fee53b6908a34e095f342d4fdfc26b03f754 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 21:48:55 +0300 Subject: [PATCH 01/21] perf(encode): write the wire offset into the sequence instead of copying it The emitter built a second array of the same length as the block's sequences, whose only new content was the four-byte offset code, then read that array back to histogram and to write the bit stream. On a level-1 profile the copy pass was over half of the block-parts stage. `RawSequence` now carries the code itself, filled in place just before the partition it belongs to is encoded, so the history a raw partition restores still governs the partition after it. The histogram, the last-sequence lookup and the bit writer read the same array the matcher filled. Upstream never builds the second array either: `ZSTD_storeSeq` puts `offBase` in the `SeqDef` at match time, and `ZSTD_seqToCodes` writes three small byte arrays rather than copying the sequences. The splitter's estimator still copies, because it prices sub-ranges repeatedly from a scratch history while the array itself has to stay as the matcher left it. It runs only on the levels that probe a split. Output byte-identical over 60 rows: three fixture shapes against ten levels, each run both plain and dictionary-primed. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 144 ++++++++++--------- zstd/src/encoding/blocks/compressed/tests.rs | 33 ++--- 2 files changed, 85 insertions(+), 92 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index c2f4a38f1..dd4e257dc 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -144,7 +144,6 @@ pub(crate) struct CompressedBlockScratch { partitions: Vec, prefix_sums: SequencePrefixSums, compressed: Vec, - estimator_sequences: Vec, /// Lazily allocated: only the block-split estimator path uses it, and /// `compress_block`'s `mem::take` constructs a throwaway `Default` /// scratch every block — an eager workspace made that default pay four @@ -181,8 +180,6 @@ impl CompressedBlockScratch { + self.partitions.capacity() * core::mem::size_of::() + self.prefix_sums.heap_size() + self.compressed.capacity() - + self.estimator_sequences.capacity() - * core::mem::size_of::() + self .estimator_workspace .as_ref() @@ -247,11 +244,24 @@ impl SequencePrefixSums { } } +/// One collected sequence, carrying both the offset the matcher found and the +/// wire code it encodes to. +/// +/// The code is filled by a pass over the collected sequences rather than at +/// collection time, because it depends on the repeat-offset history and that +/// history must not advance across a partition the emitter ends up writing raw. +/// Filling it in place, in the same array the matcher wrote, is what keeps the +/// encoder from copying every sequence into a second one; upstream likewise +/// stores its `offBase` in the sequence it already has (`ZSTD_storeSeq`) and +/// never rebuilds the array. #[derive(Clone, Copy)] struct RawSequence { ll: u32, ml: u32, offset: u32, + /// Wire offset code: 1/2/3 are the repeat offsets, N+3 an explicit N. + /// Meaningless until the fill pass has run over this sequence. + of: u32, } struct EntropyOnlyMatcher; @@ -294,12 +304,11 @@ impl Matcher for EntropyOnlyMatcher { pub fn compress_block(state: &mut CompressState, output: &mut Vec) { let mut scratch = core::mem::take(&mut state.block_scratch); collect_block_parts(state, &mut scratch.parts); - let decisions = encode_block_parts_with_sequence_scratch( + let decisions = encode_block_parts( state, &scratch.parts.literals, - &scratch.parts.sequences, + &mut scratch.parts.sequences, output, - &mut scratch.estimator_sequences, ); // This path writes the block it just encoded, so the tables it chose are // what the next block reads. @@ -338,14 +347,13 @@ pub(crate) fn compress_block_with_post_split( let mut emit_buffers = SingleSequenceEmitBuffers { output, compressed: &mut scratch.compressed, - sequence_scratch: &mut scratch.estimator_sequences, }; let emitted_raw = emit_single_sequence_block( state, last_block, source_len, &scratch.parts.literals, - &scratch.parts.sequences, + &mut scratch.parts.sequences, &mut emit_buffers, ); if emitted_raw { @@ -448,14 +456,13 @@ pub(crate) fn compress_block_with_post_split( let mut emit_buffers = SingleSequenceEmitBuffers { output, compressed: &mut scratch.compressed, - sequence_scratch: &mut scratch.estimator_sequences, }; let emitted_raw = emit_single_sequence_block( state, last_block && last_partition, src_size, &scratch.parts.literals[lit_start..lit_end], - &scratch.parts.sequences[seq_start..seq_end], + &mut scratch.parts.sequences[seq_start..seq_end], &mut emit_buffers, ); if emitted_raw { @@ -583,32 +590,35 @@ fn collect_block_parts(state: &mut CompressState, parts: &mut Enc ll, ml: match_len as u32, offset: offset as u32, + // Filled by `fill_wire_offsets` once the partition this + // sequence lands in is about to be encoded, since the code + // depends on a history that partition boundaries can rewind. + of: 0, }); } }); } -fn encode_block_parts_with_sequence_scratch( +fn encode_block_parts( state: &mut CompressState, literals_vec: &[u8], - raw_sequences: &[RawSequence], + raw_sequences: &mut [RawSequence], output: &mut Vec, - sequences: &mut Vec, // What each axis decided, for the caller to apply once it knows the block // is kept. LL, ML, OF. ) -> [LastUsedTable; 3] { // A block with no sequences writes no tables, so every axis keeps what it // had whatever the caller decides. let mut decisions = [LastUsedTable::Keep; 3]; - encode_raw_sequences_into( + fill_wire_offsets( raw_sequences, &mut state.offset_hist, - sequences, matches!( state.strategy_tag, crate::encoding::strategy::StrategyTag::Fast ), ); + let raw_sequences: &[RawSequence] = raw_sequences; // literals section @@ -684,10 +694,10 @@ fn encode_block_parts_with_sequence_scratch( // sequences section - if sequences.is_empty() { + if raw_sequences.is_empty() { writer.write_bits(0u8, 8); } else { - encode_seqnum(sequences.len(), &mut writer); + encode_seqnum(raw_sequences.len(), &mut writer); // Single-pass histogram of ll/ml/of codes across all sequences. // Previously did three separate `sequences.iter().map(...)` @@ -703,7 +713,7 @@ fn encode_block_parts_with_sequence_scratch( let mut ll_max = 0usize; let mut ml_max = 0usize; let mut of_max = 0usize; - for seq in sequences.iter() { + for seq in raw_sequences.iter() { let ll_code = encode_literal_length(seq.ll).0 as usize; let ml_code = encode_match_len(seq.ml).0 as usize; let of_code = encode_offset(seq.of).0 as usize; @@ -714,13 +724,13 @@ fn encode_block_parts_with_sequence_scratch( ml_max = ml_max.max(ml_code); of_max = of_max.max(of_code); } - let total = sequences.len(); + let total = raw_sequences.len(); // Stream codes of the LAST sequence: upstream zstd codes the final symbol // of each stream via the FSE init-state and drops one occurrence of it // from the emitted table's histogram (see `build_seq_ctable`). `Some` // here because these modes are written to the frame. - let (last_ll, last_ml, last_of) = sequences.last().map_or((0, 0, 0), |seq| { + let (last_ll, last_ml, last_of) = raw_sequences.last().map_or((0, 0, 0), |seq| { ( encode_literal_length(seq.ll).0 as usize, encode_match_len(seq.ml).0 as usize, @@ -786,7 +796,7 @@ fn encode_block_parts_with_sequence_scratch( encode_table(&ml_mode, &mut writer); encode_sequences( - sequences, + raw_sequences, &mut writer, &ll_mode, &ml_mode, @@ -816,7 +826,7 @@ struct EstimatorWorkspace { ll_counts: Box<[usize; 256]>, ml_counts: Box<[usize; 256]>, of_counts: Box<[usize; 256]>, - sequences: Vec, + sequences: Vec, } impl EstimatorWorkspace { @@ -824,8 +834,7 @@ impl EstimatorWorkspace { /// to. All four boxes are always present once the workspace exists. fn heap_size(&self) -> usize { 4 * core::mem::size_of::<[usize; 256]>() - + self.sequences.capacity() - * core::mem::size_of::() + + self.sequences.capacity() * core::mem::size_of::() } } @@ -841,7 +850,7 @@ impl Default for EstimatorWorkspace { } } -/// Dry-run analog of [`encode_block_parts_with_sequence_scratch`]: mirrors the +/// Dry-run analog of [`encode_block_parts`]: mirrors the /// real encoder's `compress_literals` and `choose_table` decisions byte-for-byte /// (same `last_huff_table` lookup, same FSE mode selection, same /// `remember_last_used_tables` mutation), and computes the would-be output size @@ -855,10 +864,21 @@ fn estimate_block_parts_size( raw_sequences: &[RawSequence], workspace: &mut EstimatorWorkspace, ) -> usize { - encode_raw_sequences_into( - raw_sequences, - &mut state.offset_hist, + // The probe cannot fill in place: it walks sub-ranges of the block's + // sequences repeatedly, from a scratch history, while the array itself is + // borrowed immutably by the estimator for the whole search. So it keeps a + // copy — which costs nothing that matters, since block splitting only runs + // from level 11 up and never on the band this array's copy was hurting. + workspace.sequences.clear(); + if workspace.sequences.capacity() < raw_sequences.len() { + workspace + .sequences + .reserve_exact(raw_sequences.len() - workspace.sequences.len()); + } + workspace.sequences.extend_from_slice(raw_sequences); + fill_wire_offsets( &mut workspace.sequences, + &mut state.offset_hist, matches!( state.strategy_tag, crate::encoding::strategy::StrategyTag::Fast @@ -907,7 +927,7 @@ fn estimate_literals_section_bytes( weight_scratch: &mut huff0_encoder::WeightScratch, suspected_incompressible: bool, ) -> usize { - // Mirror `encode_block_parts_with_sequence_scratch` literal-mode branches + // Mirror `encode_block_parts` literal-mode branches // **in the same order**. The disabled gate (negative levels: raw literals, // no Huffman) is checked FIRST exactly as the emitter does. if lit_disabled { @@ -1068,7 +1088,7 @@ fn estimate_literals_section_bytes( } fn estimate_sequences_section_bytes( - sequences: &[crate::blocks::sequence_section::Sequence], + sequences: &[RawSequence], fse_tables: &mut FseTables, ll_counts: &mut [usize; 256], ml_counts: &mut [usize; 256], @@ -1160,7 +1180,7 @@ fn estimate_sequences_section_bytes( }; let stream_bytes = (bit_content + padding_bits) / 8; - // Mirror state mutation done by `encode_block_parts_with_sequence_scratch`. + // Mirror state mutation done by `encode_block_parts`. let decisions = [ into_last_used_table(ll_mode), into_last_used_table(ml_mode), @@ -1397,7 +1417,6 @@ fn compressed_literals_header_bytes(lit_size: usize) -> usize { struct SingleSequenceEmitBuffers<'a> { output: &'a mut Vec, compressed: &'a mut Vec, - sequence_scratch: &'a mut Vec, } fn emit_single_sequence_block( @@ -1405,7 +1424,7 @@ fn emit_single_sequence_block( last_block: bool, source_len: usize, literals: &[u8], - sequences: &[RawSequence], + sequences: &mut [RawSequence], buffers: &mut SingleSequenceEmitBuffers<'_>, ) -> bool { let saved_offset_hist = state.offset_hist; @@ -1426,13 +1445,7 @@ fn emit_single_sequence_block( // the decisions below. Copying them was also what kept the built table's // handle shared, which forced a fresh one per block. buffers.compressed.clear(); - let fse_decisions = encode_block_parts_with_sequence_scratch( - state, - literals, - sequences, - buffers.compressed, - buffers.sequence_scratch, - ); + let fse_decisions = encode_block_parts(state, literals, sequences, buffers.compressed); let min_gain = (source_len >> 8) + 2; if buffers.compressed.len() >= source_len.saturating_sub(min_gain) { state.offset_hist = saved_offset_hist; @@ -1479,18 +1492,25 @@ fn emit_single_sequence_block( } } -fn encode_raw_sequences_into( - raw_sequences: &[RawSequence], +/// Fill each sequence's wire offset code in place, advancing the repeat-offset +/// history across the run. +/// +/// This ran as a copy into a second array of the same length, which is a read +/// and a twelve-byte write per sequence for the sake of one field; the encoder +/// now writes the four bytes it computes into the sequence it already has. +/// Upstream never builds the second array either: `ZSTD_storeSeq` puts +/// `offBase` in the `SeqDef` at match time, and `ZSTD_seqToCodes` writes three +/// small byte arrays rather than copying the sequences. +/// +/// Per PARTITION, not per block: the emitter can write a partition raw, and +/// when it does it restores the history, so the partition after it must be +/// filled from the restored one. Filling here, just before each partition is +/// encoded, is what keeps that true. +fn fill_wire_offsets( + raw_sequences: &mut [RawSequence], offset_hist: &mut [u32; 3], - out: &mut Vec, fast_repcode: bool, ) { - out.clear(); - // `reserve_exact` argument is the increment over LENGTH, not capacity — - // see `SequencePrefixSums::rebuild` for the full rationale. - if out.capacity() < raw_sequences.len() { - out.reserve_exact(raw_sequences.len() - out.len()); - } // The strategy branch is hoisted out of the per-sequence loop so the // offBase-policy choice is paid once per block, not per sequence. Upstream // zstd's fast matcher emits only offBase 1 (rep[0] when litLength > 0, @@ -1498,25 +1518,13 @@ fn encode_raw_sequences_into( // offset — it never emits offBase 2/3. greedy+ search all three repeat // offsets, which is what the full `encode_offset_with_history` mirrors. if fast_repcode { - out.extend( - raw_sequences - .iter() - .map(|seq| crate::blocks::sequence_section::Sequence { - ll: seq.ll, - ml: seq.ml, - of: encode_offset_with_history_fast(seq.offset, seq.ll, offset_hist), - }), - ); + for seq in raw_sequences.iter_mut() { + seq.of = encode_offset_with_history_fast(seq.offset, seq.ll, offset_hist); + } } else { - out.extend( - raw_sequences - .iter() - .map(|seq| crate::blocks::sequence_section::Sequence { - ll: seq.ll, - ml: seq.ml, - of: encode_offset_with_history(seq.offset, seq.ll, offset_hist), - }), - ); + for seq in raw_sequences.iter_mut() { + seq.of = encode_offset_with_history(seq.offset, seq.ll, offset_hist); + } } } @@ -2242,7 +2250,7 @@ fn commit_last_used_table( } fn encode_sequences( - sequences: &[crate::blocks::sequence_section::Sequence], + sequences: &[RawSequence], writer: &mut BitWriter<&mut Vec>, ll_mode: &FseTableMode<'_>, ml_mode: &FseTableMode<'_>, diff --git a/zstd/src/encoding/blocks/compressed/tests.rs b/zstd/src/encoding/blocks/compressed/tests.rs index e1824121e..b6f8f0f58 100644 --- a/zstd/src/encoding/blocks/compressed/tests.rs +++ b/zstd/src/encoding/blocks/compressed/tests.rs @@ -270,8 +270,8 @@ fn decide_huff_reuse_prefer_repeat_forces_reuse_for_fast_band() { #[test] fn estimator_literals_section_mirrors_emit_for_short_inputs() { use super::{ - CompressedBlockScratch, EntropyOnlyMatcher, EstimatorWorkspace, - encode_block_parts_with_sequence_scratch, estimate_block_parts_size, + CompressedBlockScratch, EntropyOnlyMatcher, EstimatorWorkspace, encode_block_parts, + estimate_block_parts_size, }; // For each strategy at boundary literal lengths around `min_lits` // and across the all-identical RLE pre-check (fires for any @@ -383,14 +383,7 @@ fn estimator_literals_section_mirrors_emit_for_short_inputs() { let mut workspace = EstimatorWorkspace::default(); let est = estimate_block_parts_size(&mut est_state, &literals, &[], &mut workspace); let mut emitted: Vec = Vec::new(); - let mut scratch: Vec = Vec::new(); - encode_block_parts_with_sequence_scratch( - &mut emit_state, - &literals, - &[], - &mut emitted, - &mut scratch, - ); + encode_block_parts(&mut emit_state, &literals, &mut [], &mut emitted); assert_eq!( est, emitted.len(), @@ -409,8 +402,8 @@ fn estimator_literals_section_mirrors_emit_for_short_inputs() { #[test] fn a_section_with_flat_ends_costs_what_the_emitter_writes_for_it() { use super::{ - CompressedBlockScratch, EntropyOnlyMatcher, EstimatorWorkspace, - encode_block_parts_with_sequence_scratch, estimate_block_parts_size, + CompressedBlockScratch, EntropyOnlyMatcher, EstimatorWorkspace, encode_block_parts, + estimate_block_parts_size, }; // The shape the end-sample shortcut exists for, and the one where the // estimator and the emitter can disagree: both ends look random, the @@ -450,14 +443,7 @@ fn a_section_with_flat_ends_costs_what_the_emitter_writes_for_it() { let mut workspace = EstimatorWorkspace::default(); let est = estimate_block_parts_size(&mut est_state, &literals, &[], &mut workspace); let mut emitted: Vec = Vec::new(); - let mut scratch: Vec = Vec::new(); - encode_block_parts_with_sequence_scratch( - &mut emit_state, - &literals, - &[], - &mut emitted, - &mut scratch, - ); + encode_block_parts(&mut emit_state, &literals, &mut [], &mut emitted); assert_eq!( est, @@ -505,26 +491,25 @@ fn raw_partition_fallback_restores_repeat_offset_history() { literal_compression_disabled: false, }; let source = [0xA5; 8]; - let sequences = [RawSequence { + let mut sequences = [RawSequence { ll: 0, ml: 5, offset: 20, + of: 0, }]; let mut output = Vec::new(); let mut compressed_scratch = Vec::new(); - let mut sequence_scratch = Vec::new(); let mut emit_buffers = super::SingleSequenceEmitBuffers { output: &mut output, compressed: &mut compressed_scratch, - sequence_scratch: &mut sequence_scratch, }; let emitted_raw = emit_single_sequence_block( &mut state, true, source.len(), &[], - &sequences, + &mut sequences, &mut emit_buffers, ); if emitted_raw { From 8afee288c790038c03a193cfd22bbea9ed93c9ef Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 22:01:59 +0300 Subject: [PATCH 02/21] perf(encode): keep the sequence at three words by reusing the offset slot The wire code went into a fourth field, which widened every sequence in the block from twelve bytes to sixteen. Measured on the i9, that cost more in memory traffic than the copy pass it removed: retired instructions flat to within a tenth of a percent, cycles up 10.3% on the access log at level 5 and 2.3% at level 9 and on decodecorpus at level 5. The found offset has no reader once its code exists, so the code goes into that slot instead of beside it, which is the single slot upstream keeps (`SeqDef::offBase`, written by `ZSTD_storeSeq`). Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 50 ++++++++++---------- zstd/src/encoding/blocks/compressed/tests.rs | 3 +- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index dd4e257dc..67c616e61 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -244,24 +244,22 @@ impl SequencePrefixSums { } } -/// One collected sequence, carrying both the offset the matcher found and the -/// wire code it encodes to. +/// One collected sequence. /// -/// The code is filled by a pass over the collected sequences rather than at -/// collection time, because it depends on the repeat-offset history and that -/// history must not advance across a partition the emitter ends up writing raw. -/// Filling it in place, in the same array the matcher wrote, is what keeps the -/// encoder from copying every sequence into a second one; upstream likewise -/// stores its `offBase` in the sequence it already has (`ZSTD_storeSeq`) and -/// never rebuilds the array. +/// `off_base` holds the offset the matcher found until [`fill_wire_offsets`] +/// runs over the sequence, and the wire code from then on: 1/2/3 for the repeat +/// offsets, N+3 for an explicit N. It is one field rather than two because the +/// found offset has no reader once its code exists, and a fourth word would +/// widen every sequence in the block for a value with a lifetime of one pass. +/// Upstream keeps the same single slot, filled at match time +/// (`SeqDef::offBase`, written by `ZSTD_storeSeq`); ours cannot be filled that +/// early because the code depends on a repeat-offset history that must not +/// advance across a partition the emitter ends up writing raw. #[derive(Clone, Copy)] struct RawSequence { ll: u32, ml: u32, - offset: u32, - /// Wire offset code: 1/2/3 are the repeat offsets, N+3 an explicit N. - /// Meaningless until the fill pass has run over this sequence. - of: u32, + off_base: u32, } struct EntropyOnlyMatcher; @@ -589,11 +587,11 @@ fn collect_block_parts(state: &mut CompressState, parts: &mut Enc parts.sequences.push(RawSequence { ll, ml: match_len as u32, - offset: offset as u32, - // Filled by `fill_wire_offsets` once the partition this - // sequence lands in is about to be encoded, since the code - // depends on a history that partition boundaries can rewind. - of: 0, + // The found offset. `fill_wire_offsets` replaces it with its + // code once the partition this sequence lands in is about to be + // encoded, since the code depends on a history that partition + // boundaries can rewind. + off_base: offset as u32, }); } }); @@ -716,7 +714,7 @@ fn encode_block_parts( for seq in raw_sequences.iter() { let ll_code = encode_literal_length(seq.ll).0 as usize; let ml_code = encode_match_len(seq.ml).0 as usize; - let of_code = encode_offset(seq.of).0 as usize; + let of_code = encode_offset(seq.off_base).0 as usize; ll_counts[ll_code] += 1; ml_counts[ml_code] += 1; of_counts[of_code] += 1; @@ -734,7 +732,7 @@ fn encode_block_parts( ( encode_literal_length(seq.ll).0 as usize, encode_match_len(seq.ml).0 as usize, - encode_offset(seq.of).0 as usize, + encode_offset(seq.off_base).0 as usize, ) }); @@ -1102,7 +1100,7 @@ fn estimate_sequences_section_bytes( for seq in sequences { let (ll, _, ll_bits) = encode_literal_length(seq.ll); let (ml, _, ml_bits) = encode_match_len(seq.ml); - let (of, _, _) = encode_offset(seq.of); + let (of, _, _) = encode_offset(seq.off_base); ll_counts[ll as usize] += 1; ml_counts[ml as usize] += 1; of_counts[of as usize] += 1; @@ -1148,7 +1146,7 @@ fn estimate_sequences_section_bytes( let of_mode = choose_table( of_previous.as_ref(), of_default, - sequences.iter().map(|seq| encode_offset(seq.of).0), + sequences.iter().map(|seq| encode_offset(seq.off_base).0), 8, strategy, of_next, @@ -1519,11 +1517,11 @@ fn fill_wire_offsets( // offsets, which is what the full `encode_offset_with_history` mirrors. if fast_repcode { for seq in raw_sequences.iter_mut() { - seq.of = encode_offset_with_history_fast(seq.offset, seq.ll, offset_hist); + seq.off_base = encode_offset_with_history_fast(seq.off_base, seq.ll, offset_hist); } } else { for seq in raw_sequences.iter_mut() { - seq.of = encode_offset_with_history(seq.offset, seq.ll, offset_hist); + seq.off_base = encode_offset_with_history(seq.off_base, seq.ll, offset_hist); } } } @@ -2266,7 +2264,7 @@ fn encode_sequences( let sequence = sequences[sequences.len() - 1]; let (ll_code, ll_add_bits, ll_num_bits) = encode_literal_length(sequence.ll); - let (of_code, of_add_bits, of_num_bits) = encode_offset(sequence.of); + let (of_code, of_add_bits, of_num_bits) = encode_offset(sequence.off_base); let (ml_code, ml_add_bits, ml_num_bits) = encode_match_len(sequence.ml); let [ll_default, ml_default, of_default] = defaults; let ll_table = mode_table(ll_mode, ll_default); @@ -2316,7 +2314,7 @@ fn encode_sequences( for sequence in (0..=sequences.len() - 2).rev() { let sequence = sequences[sequence]; let (ll_code, ll_add_bits, ll_num_bits) = encode_literal_length(sequence.ll); - let (of_code, of_add_bits, of_num_bits) = encode_offset(sequence.of); + let (of_code, of_add_bits, of_num_bits) = encode_offset(sequence.off_base); let (ml_code, ml_add_bits, ml_num_bits) = encode_match_len(sequence.ml); // State diffs burst: max 30 bits (10+10+9 worst case for diff --git a/zstd/src/encoding/blocks/compressed/tests.rs b/zstd/src/encoding/blocks/compressed/tests.rs index b6f8f0f58..05b82f667 100644 --- a/zstd/src/encoding/blocks/compressed/tests.rs +++ b/zstd/src/encoding/blocks/compressed/tests.rs @@ -494,8 +494,7 @@ fn raw_partition_fallback_restores_repeat_offset_history() { let mut sequences = [RawSequence { ll: 0, ml: 5, - offset: 20, - of: 0, + off_base: 20, }]; let mut output = Vec::new(); let mut compressed_scratch = Vec::new(); From d4c16da953ff37c1f83976bdf30cdbd7367555b2 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 22:11:36 +0300 Subject: [PATCH 03/21] perf(encode): make the dense hash-table prime a counted loop again A call-graph profile of a level-1 encode puts a fifth of the whole frame in the prime, and half of that inside `RangeInclusive::next` and its `lt`: the iterator's exhausted flag, live across every position, on a loop whose body is one hash and one store. The comment above it already recorded that `step_by(1)` had cost a factor of two here for the same reason and had been removed; the inclusive range it was removed in favour of carries the other half of the same problem. A half-open range is what compiles to a counted loop. Part of #493. --- zstd/src/encoding/simple/fast_matcher.rs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/zstd/src/encoding/simple/fast_matcher.rs b/zstd/src/encoding/simple/fast_matcher.rs index bbd0a7677..31e444cee 100644 --- a/zstd/src/encoding/simple/fast_matcher.rs +++ b/zstd/src/encoding/simple/fast_matcher.rs @@ -1925,12 +1925,15 @@ impl FastKernelMatcher { }}; } // Dense is the ordinary path — every searched block primes through it — - // and it has to stay a plain counted loop. Running it as `step_by(1)` - // over an inclusive range keeps the iterator's stride and exhausted - // flag live, which the optimiser does not reduce back to an induction - // variable: that alone cost a factor of two on every fast-level frame. + // and it has to stay a plain counted loop. Two shapes have cost it that: + // running it as `step_by(1)` keeps the iterator's stride live, and an + // INCLUSIVE range keeps its exhausted flag live. Neither is reduced back + // to an induction variable, and both show up as `next` / `lt` frames + // inside this function's profile. The half-open range is the one that + // compiles to a counted loop. The bound cannot overflow: `last_hashable` + // is `history.len() - HASH_READ_SIZE`. if step == 1 { - for pos in range_start..=last_hashable { + for pos in range_start..last_hashable + 1 { index_at!(pos); } return; @@ -1949,11 +1952,11 @@ impl FastKernelMatcher { // avoid. const SEAM: usize = 8; let head_end = last_hashable.min(range_start + SEAM); - for pos in range_start..=head_end { + for pos in range_start..head_end + 1 { index_at!(pos); } let tail_start = last_hashable.saturating_sub(SEAM).max(range_start); - for pos in tail_start..=last_hashable { + for pos in tail_start..last_hashable + 1 { index_at!(pos); } } From a79da07940f5aa5c2c6ce15c91c236b8863554b4 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 22:32:22 +0300 Subject: [PATCH 04/21] perf(huff0): size the tree node like upstream's The node was four `usize` fields plus an `Option`: forty bytes, twenty kilobytes of node table for a full alphabet, walked several times per block beside a six-kilobyte histogram. Upstream's `nodeElt` is eight bytes, and the difference showed up as eight-byte moves and a forty-byte stride throughout the build's profile. Every field is bounded by the block, so none of them needed a machine word: counts sum to the literal count, symbols index a 256-entry alphabet, node indices reach `2 * 256 - 1`, and a natural code depth is under the leaf count. The root's absent parent is a sentinel index rather than an `Option`, which is what the `Option` was costing sixteen bytes for. Part of #493. --- zstd/src/huff0/huff0_encoder.rs | 95 +++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 39 deletions(-) diff --git a/zstd/src/huff0/huff0_encoder.rs b/zstd/src/huff0/huff0_encoder.rs index c349e60f8..9d55c008b 100644 --- a/zstd/src/huff0/huff0_encoder.rs +++ b/zstd/src/huff0/huff0_encoder.rs @@ -1193,14 +1193,27 @@ fn huffman_weight_sum_is_power_of_two(weights: &[usize]) -> bool { sum.is_power_of_two() } -#[derive(Clone)] +/// A leaf or internal node of the Huffman tree, sized like upstream's +/// `nodeElt`: the table is `2 * leaf_count - 1` entries and is walked several +/// times per block, so its width is what decides whether it stays in L1 +/// alongside the histogram. In `usize` fields it was forty bytes a node, +/// twenty kilobytes for a full alphabet. +/// +/// Every field's range is bounded by the block: counts sum to the literal +/// count, symbols index a 256-entry alphabet, and node indices reach +/// `2 * 256 - 1`. [`NO_PARENT`] is the root's parent. +#[derive(Clone, Copy)] struct HuffNode { - count: usize, - symbol: usize, - parent: Option, - nb_bits: usize, + count: u32, + symbol: u16, + parent: u16, + nb_bits: u8, } +/// `parent` of a node with no parent yet, and of the root once the tree is +/// built. Out of range for a real node index, which is under `2 * 256 - 1`. +const NO_PARENT: u16 = u16::MAX; + /// Build the count-sorted Huffman leaves with their natural (unlimited) code /// lengths in `nb_bits`. The tree shape is independent of any maximum-length /// limit, so this is computed once per block and shared across every @@ -1230,9 +1243,9 @@ fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { if leaf_count == 1 { let (symbol, &count) = counts.iter().enumerate().find(|&(_, &c)| c > 0).unwrap(); nodes.push(HuffNode { - count, - symbol, - parent: None, + count: count as u32, + symbol: symbol as u16, + parent: NO_PARENT, nb_bits: 0, }); return; @@ -1278,7 +1291,7 @@ fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { HuffNode { count: 0, symbol: 0, - parent: None, + parent: NO_PARENT, nb_bits: 0, }, ); @@ -1290,9 +1303,9 @@ fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { let pos = cursor[bucket] as usize; cursor[bucket] += 1; nodes[pos] = HuffNode { - count, - symbol, - parent: None, + count: count as u32, + symbol: symbol as u16, + parent: NO_PARENT, nb_bits: 0, }; } @@ -1309,12 +1322,16 @@ fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { } } + // The unbuilt internal nodes start at the maximum count so the merge loop + // below never selects one: it always takes the smaller of the next leaf and + // the next built node, and a count no real subtree can reach is what keeps + // an unbuilt slot out of that comparison. nodes.resize( 2 * leaf_count - 1, HuffNode { - count: usize::MAX, - symbol: usize::MAX, - parent: None, + count: u32::MAX, + symbol: u16::MAX, + parent: NO_PARENT, nb_bits: 0, }, ); @@ -1326,13 +1343,13 @@ fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { // Plain `+`: node counts are symbol frequencies whose tree-wide sum is the // block's symbol count (<= MAX_BLOCK_SIZE), so a merged parent count cannot - // overflow usize. Saturation would only mask a corrupt frequency table. + // overflow u32. Saturation would only mask a corrupt frequency table. nodes[node_nb].count = nodes[low_s as usize].count + nodes[(low_s - 1) as usize].count; nodes[node_nb].symbol = nodes[(low_s - 1) as usize] .symbol .min(nodes[low_s as usize].symbol); - nodes[low_s as usize].parent = Some(node_nb); - nodes[(low_s - 1) as usize].parent = Some(node_nb); + nodes[low_s as usize].parent = node_nb as u16; + nodes[(low_s - 1) as usize].parent = node_nb as u16; node_nb += 1; low_s -= 2; @@ -1341,7 +1358,7 @@ fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { let leaf_count = if low_s >= 0 { nodes[low_s as usize].count } else { - usize::MAX + u32::MAX }; let node_count = nodes[low_n].count; if leaf_count < node_count { @@ -1358,7 +1375,7 @@ fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { let leaf_count = if low_s >= 0 { nodes[low_s as usize].count } else { - usize::MAX + u32::MAX }; let node_count = nodes[low_n].count; if leaf_count < node_count { @@ -1374,17 +1391,17 @@ fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { // Plain `+`: see the leaf-merge above — counts sum to <= MAX_BLOCK_SIZE. nodes[node_nb].count = nodes[first].count + nodes[second].count; nodes[node_nb].symbol = nodes[first].symbol.min(nodes[second].symbol); - nodes[first].parent = Some(node_nb); - nodes[second].parent = Some(node_nb); + nodes[first].parent = node_nb as u16; + nodes[second].parent = node_nb as u16; node_nb += 1; } for leaf_idx in 0..leaf_count { - let mut depth = 0usize; + let mut depth = 0u8; let mut parent = nodes[leaf_idx].parent; - while let Some(parent_idx) = parent { + while parent != NO_PARENT { depth += 1; - parent = nodes[parent_idx].parent; + parent = nodes[parent as usize].parent; } nodes[leaf_idx].nb_bits = depth; } @@ -1418,7 +1435,7 @@ fn limited_weights_into( out.resize(counts_len, 0); if leaves.len() <= 1 { if let Some(leaf) = leaves.first() { - out[leaf.symbol] = 1; + out[leaf.symbol as usize] = 1; } return true; } @@ -1433,19 +1450,19 @@ fn limited_weights_into( // upstream), leaving the code under- or over-full, which projects to a // non-power-of-two weight sum the table builder rejects; detect that here // and fall back to the distributed-weight construction. - if work.iter().any(|leaf| leaf.nb_bits > max_nb_bits) { + if work.iter().any(|leaf| leaf.nb_bits as usize > max_nb_bits) { return false; } let kraft_sum = work .iter() - .map(|leaf| 1usize << (max_nb_bits - leaf.nb_bits)) + .map(|leaf| 1usize << (max_nb_bits - leaf.nb_bits as usize)) .sum::(); if kraft_sum != 1usize << max_nb_bits { return false; } for leaf in work.iter() { - out[leaf.symbol] = max_nb_bits - leaf.nb_bits + 1; + out[leaf.symbol as usize] = max_nb_bits - leaf.nb_bits as usize + 1; } true } @@ -1547,7 +1564,7 @@ fn legacy_distributed_weights(counts: &[usize]) -> Vec { } fn enforce_max_height(nodes: &mut [HuffNode], target_nb_bits: usize) { - let Some(largest_bits) = nodes.iter().map(|node| node.nb_bits).max() else { + let Some(largest_bits) = nodes.iter().map(|node| node.nb_bits as usize).max() else { return; }; if largest_bits <= target_nb_bits { @@ -1569,15 +1586,15 @@ fn enforce_max_height(nodes: &mut [HuffNode], target_nb_bits: usize) { let base_cost = 1usize << (largest_bits - target_nb_bits); let mut total_cost = 0isize; let mut n = nodes.len() - 1; - while nodes[n].nb_bits > target_nb_bits { - total_cost += (base_cost - (1usize << (largest_bits - nodes[n].nb_bits))) as isize; - nodes[n].nb_bits = target_nb_bits; + while nodes[n].nb_bits as usize > target_nb_bits { + total_cost += (base_cost - (1usize << (largest_bits - nodes[n].nb_bits as usize))) as isize; + nodes[n].nb_bits = target_nb_bits as u8; if n == 0 { break; } n -= 1; } - while n > 0 && nodes[n].nb_bits == target_nb_bits { + while n > 0 && nodes[n].nb_bits as usize == target_nb_bits { n -= 1; } total_cost >>= largest_bits - target_nb_bits; @@ -1594,10 +1611,10 @@ fn enforce_max_height(nodes: &mut [HuffNode], target_nb_bits: usize) { let mut rank_last = [NO_SYMBOL; 14]; let mut current_nb_bits = target_nb_bits; for pos in (0..=n).rev() { - if nodes[pos].nb_bits >= current_nb_bits { + if nodes[pos].nb_bits as usize >= current_nb_bits { continue; } - current_nb_bits = nodes[pos].nb_bits; + current_nb_bits = nodes[pos].nb_bits as usize; rank_last[target_nb_bits - current_nb_bits] = pos; } @@ -1636,7 +1653,7 @@ fn enforce_max_height(nodes: &mut [HuffNode], target_nb_bits: usize) { } else { let next = pos - 1; rank_last[bits_to_decrease] = - if nodes[next].nb_bits == target_nb_bits - bits_to_decrease { + if nodes[next].nb_bits as usize == target_nb_bits - bits_to_decrease { next } else { NO_SYMBOL @@ -1656,7 +1673,7 @@ fn enforce_max_height(nodes: &mut [HuffNode], target_nb_bits: usize) { while total_cost < 0 { if rank_last[1] == NO_SYMBOL { // No rank-1 symbol yet: create one from the largest rank-0 node. - while n > 0 && nodes[n].nb_bits == target_nb_bits { + while n > 0 && nodes[n].nb_bits as usize == target_nb_bits { n -= 1; } // Upstream relies on an over-sized node buffer here and reads into @@ -1664,7 +1681,7 @@ fn enforce_max_height(nodes: &mut [HuffNode], target_nb_bits: usize) { // is no real rank-0 node left to borrow, the distribution is too // degenerate to height-limit, so stop and let the caller fall back // to the distributed-weight construction. - if nodes[n].nb_bits == target_nb_bits || n + 1 >= nodes.len() { + if nodes[n].nb_bits as usize == target_nb_bits || n + 1 >= nodes.len() { break; } nodes[n + 1].nb_bits -= 1; From 6a23c9d74a053fd319222c28d1fe0819a04a4bc4 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 8 Sep 2026 23:50:59 +0300 Subject: [PATCH 05/21] perf(encode): derive and count the sequence codes in one pass Deriving each sequence's offset code and histogramming the three code streams were two walks of the same array, and the second read back what the first had just written. On a level-1 profile the derivation pass alone was 6.2% of the frame. They are now one pass, with the offBase policy a const-generic so the strategy branch stays out of the loop body. Upstream splits the two (`ZSTD_seqToCodes` then `HIST_countFast_wksp`) because its codes go to three byte arrays of their own; ours are already in the sequence. The pass moves after the literals section, which reads none of what it writes. The block-split estimator keeps a fill-only version: it prices sub-ranges repeatedly from a scratch history and counts them itself. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 115 ++++++++++++++++--------- 1 file changed, 76 insertions(+), 39 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index 67c616e61..e9877f2fb 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -608,15 +608,6 @@ fn encode_block_parts( // A block with no sequences writes no tables, so every axis keeps what it // had whatever the caller decides. let mut decisions = [LastUsedTable::Keep; 3]; - fill_wire_offsets( - raw_sequences, - &mut state.offset_hist, - matches!( - state.strategy_tag, - crate::encoding::strategy::StrategyTag::Fast - ), - ); - let raw_sequences: &[RawSequence] = raw_sequences; // literals section @@ -705,23 +696,24 @@ fn encode_block_parts( let mut ll_counts = [0usize; 256]; let mut ml_counts = [0usize; 256]; let mut of_counts = [0usize; 256]; - // Track the highest code per stream while histogramming so the table - // selector skips the full-256 reverse scan for `max_symbol` (the small - // sequence-code alphabets leave ~200 high slots permanently zero). - let mut ll_max = 0usize; - let mut ml_max = 0usize; - let mut of_max = 0usize; - for seq in raw_sequences.iter() { - let ll_code = encode_literal_length(seq.ll).0 as usize; - let ml_code = encode_match_len(seq.ml).0 as usize; - let of_code = encode_offset(seq.off_base).0 as usize; - ll_counts[ll_code] += 1; - ml_counts[ml_code] += 1; - of_counts[of_code] += 1; - ll_max = ll_max.max(ll_code); - ml_max = ml_max.max(ml_code); - of_max = of_max.max(of_code); - } + // The offset codes are derived in this same pass rather than by a + // walk of their own ahead of the literals section: both passes read + // every sequence, and the second one was reading back what the first + // had just written. + let counts = SequenceCodeCounts { + ll: &mut ll_counts, + ml: &mut ml_counts, + of: &mut of_counts, + }; + let (ll_max, ml_max, of_max) = if matches!( + state.strategy_tag, + crate::encoding::strategy::StrategyTag::Fast + ) { + fill_and_count::(raw_sequences, &mut state.offset_hist, counts) + } else { + fill_and_count::(raw_sequences, &mut state.offset_hist, counts) + }; + let raw_sequences: &[RawSequence] = raw_sequences; let total = raw_sequences.len(); // Stream codes of the LAST sequence: upstream zstd codes the final symbol @@ -1490,31 +1482,76 @@ fn emit_single_sequence_block( } } +/// The three sequence-code histograms, passed as one argument so the pass that +/// fills them stays under the register-pressure of six. +struct SequenceCodeCounts<'a> { + ll: &'a mut [usize; 256], + ml: &'a mut [usize; 256], + of: &'a mut [usize; 256], +} + /// Fill each sequence's wire offset code in place, advancing the repeat-offset -/// history across the run. +/// history across the run, and histogram the three code streams while the +/// sequence is in hand. Returns the highest code seen per stream, which the +/// table selector needs and would otherwise find by scanning ~200 always-zero +/// slots. /// -/// This ran as a copy into a second array of the same length, which is a read -/// and a twelve-byte write per sequence for the sake of one field; the encoder -/// now writes the four bytes it computes into the sequence it already has. -/// Upstream never builds the second array either: `ZSTD_storeSeq` puts -/// `offBase` in the `SeqDef` at match time, and `ZSTD_seqToCodes` writes three -/// small byte arrays rather than copying the sequences. +/// One pass, not two: deriving the offset codes and counting them both read +/// every sequence, and the counting pass was reading back what the filling pass +/// had just written. Upstream splits them (`ZSTD_seqToCodes` then +/// `HIST_countFast_wksp`) because its codes go to three separate byte arrays; +/// ours are already where they belong. +/// +/// `FAST_REPCODE` picks the offBase policy once per block instead of per +/// sequence. Upstream's fast matcher emits only offBase 1 (rep[0] when +/// litLength > 0, rep[1] when litLength == 0 via the secondary-position check) +/// or an explicit offset, and never 2/3; greedy and above search all three +/// repeat offsets, which is what the full `encode_offset_with_history` mirrors. /// /// Per PARTITION, not per block: the emitter can write a partition raw, and /// when it does it restores the history, so the partition after it must be /// filled from the restored one. Filling here, just before each partition is /// encoded, is what keeps that true. +fn fill_and_count( + raw_sequences: &mut [RawSequence], + offset_hist: &mut [u32; 3], + counts: SequenceCodeCounts<'_>, +) -> (usize, usize, usize) { + let SequenceCodeCounts { + ll: ll_counts, + ml: ml_counts, + of: of_counts, + } = counts; + let mut ll_max = 0usize; + let mut ml_max = 0usize; + let mut of_max = 0usize; + for seq in raw_sequences.iter_mut() { + let off_base = if FAST_REPCODE { + encode_offset_with_history_fast(seq.off_base, seq.ll, offset_hist) + } else { + encode_offset_with_history(seq.off_base, seq.ll, offset_hist) + }; + seq.off_base = off_base; + let ll_code = encode_literal_length(seq.ll).0 as usize; + let ml_code = encode_match_len(seq.ml).0 as usize; + let of_code = encode_offset(off_base).0 as usize; + ll_counts[ll_code] += 1; + ml_counts[ml_code] += 1; + of_counts[of_code] += 1; + ll_max = ll_max.max(ll_code); + ml_max = ml_max.max(ml_code); + of_max = of_max.max(of_code); + } + (ll_max, ml_max, of_max) +} + +/// [`fill_and_count`] without the histogram, for the block-split estimator: it +/// prices sub-ranges repeatedly from a scratch history and counts them itself. fn fill_wire_offsets( raw_sequences: &mut [RawSequence], offset_hist: &mut [u32; 3], fast_repcode: bool, ) { - // The strategy branch is hoisted out of the per-sequence loop so the - // offBase-policy choice is paid once per block, not per sequence. Upstream - // zstd's fast matcher emits only offBase 1 (rep[0] when litLength > 0, - // rep[1] when litLength == 0 via the secondary-position check) or an explicit - // offset — it never emits offBase 2/3. greedy+ search all three repeat - // offsets, which is what the full `encode_offset_with_history` mirrors. if fast_repcode { for seq in raw_sequences.iter_mut() { seq.off_base = encode_offset_with_history_fast(seq.off_base, seq.ll, offset_hist); From 78d9df2495b7aea5ec2a1d57e0d640cfd3a125aa Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 00:03:24 +0300 Subject: [PATCH 06/21] perf(encode): slide the fast table's indices instead of rebuilding it Every window slide cleared the hash table and rehashed the whole retained tail, a pass over every byte the window kept. The window slides once per `max_window_size` bytes, so on an 8 MiB access log at level 1 that rehash was a fifth of the encode, in the CLI as much as in the loop harness. Upstream does not rebuild: `ZSTD_reduceIndex` walks the table and subtracts the correction from each stored position. That is a pass over the table's own entries rather than over the window's bytes, and it is also the more faithful state -- the rehash indexed every position, including the ones the matcher's step had skipped and never stored, so the table came out of a slide holding more than it held going in. Output changes on the Fast band of inputs long enough to slide, in both directions and by under a tenth of a percent, and stays smaller than libzstd's on every one. On the 8 MiB access log: --fast=5 2,298,225 -> 2,297,015 bytes, --fast=1 1,926,093 -> 1,924,368, level 1 1,466,481 -> 1,467,858, level 2 1,515,569 -> 1,516,103, against libzstd's 2,300,531 / 1,927,913 / 1,468,937 / 1,517,054. Level 3 and above are a different backend and unchanged; incompressible input is unchanged at every level. Part of #493. --- .../encoding/simple/fast_kernel/hash_table.rs | 22 +++++++++++++ zstd/src/encoding/simple/fast_matcher.rs | 31 ++++++++++--------- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/zstd/src/encoding/simple/fast_kernel/hash_table.rs b/zstd/src/encoding/simple/fast_kernel/hash_table.rs index 94a75d3bf..0e99a37c6 100644 --- a/zstd/src/encoding/simple/fast_kernel/hash_table.rs +++ b/zstd/src/encoding/simple/fast_kernel/hash_table.rs @@ -231,6 +231,28 @@ impl FastHashTable { self.bias = 0; } + /// Slide every stored position down by `drop_n`, the way upstream's + /// `ZSTD_reduceIndex` does when the window moves: the table keeps naming + /// the same bytes, and a position that named part of the dropped prefix + /// becomes the empty sentinel. + /// + /// This is what a window slide costs when the caller does not rebuild the + /// table: one pass over the table's own entries, against a pass over every + /// byte the window kept. It also leaves the table holding exactly what it + /// held before the slide, where a rebuild adds entries for positions the + /// matcher had skipped and never indexed. + /// + /// `saturating_sub` is the semantics on both lines, not a guard: below the + /// bias a slot is already empty, and at or under `drop_n` the position it + /// names is gone. Both floor at the sentinel. + pub(crate) fn reduce_indices(&mut self, drop_n: u32) { + let bias = self.bias; + for slot in self.table.iter_mut() { + *slot = slot.saturating_sub(bias).saturating_sub(drop_n); + } + self.bias = 0; + } + /// Continue-mode frame reset (upstream zstd `ZSTD_continueCCtx` cadence): keep /// the table contents and advance the epoch bias past every entry the /// previous frame stored, so all of them read back as the empty diff --git a/zstd/src/encoding/simple/fast_matcher.rs b/zstd/src/encoding/simple/fast_matcher.rs index 31e444cee..906df2d0c 100644 --- a/zstd/src/encoding/simple/fast_matcher.rs +++ b/zstd/src/encoding/simple/fast_matcher.rs @@ -963,16 +963,20 @@ impl FastKernelMatcher { /// 2. Reset `prefix_start_index` to `INITIAL_PREFIX_START_INDEX = 1` /// — drain re-indexes the retained tail; the sentinel-0 /// filter restores via this fixed baseline. - /// 3. Clear the hash table — entries hold pre-drain absolute - /// positions that no longer reference live bytes. + /// 3. Slide the hash table's stored positions down by `drop_n` + /// ([`FastHashTable::reduce_indices`], upstream `ZSTD_reduceIndex`) + /// so they keep naming the same bytes, with anything that named + /// the dropped prefix falling to the empty sentinel. /// 4. `saturating_sub` `last_block_start` by `drop_n`. - /// 5. Rehash retained tail starting at the sentinel-0 floor - /// ([`INITIAL_PREFIX_START_INDEX`] = 1) so block N+1 can find - /// matches against the kept bytes (without this they'd be - /// "dead history" — visible in the Vec but unlookupable). - /// Starting from index 1 instead of 0 avoids hashing a position - /// that the kernel's `match_idx >= prefix_start_index` filter - /// would reject anyway. + /// + /// Step 3 replaced a clear plus a dense rehash of the whole retained + /// tail, which was a pass over every byte the window kept — at level 1 + /// on an 8 MiB input the rehash alone was a fifth of the encode, for a + /// window that slides every `max_window_size` bytes. Sliding the indices + /// is a pass over the table instead, and it is also the more faithful + /// state: the rehash indexed every position, including the ones the + /// matcher's step had skipped and never stored, so the table came out of + /// a slide holding more than it held going in. fn drain_real_prefix(&mut self, drop_n: usize) { let drain_end = HISTORY_DRAIN_BASE + drop_n; self.history.drain(HISTORY_DRAIN_BASE..drain_end); @@ -991,12 +995,11 @@ impl FastKernelMatcher { // prefix floor reverts to the windowed value (upstream zstd // `ZSTD_window_enforceMaxDist` zeroing `loadedDictEnd`). self.loaded_dict_end = 0; - self.hash_table.clear(); + // `drop_n` is a length inside `history`, which the window ceiling + // (`window_log <= 30`, retained at most `2 * max_window_size`) keeps + // under 2^31. + self.hash_table.reduce_indices(drop_n as u32); self.last_block_start = self.last_block_start.saturating_sub(drop_n); - // Skip position 0 — `prefix_start_index = 1` means the kernel - // rejects any match resolving to index 0, so populating that - // slot would just pollute the table with an unreachable entry. - self.prime_hash_table_for_range(INITIAL_PREFIX_START_INDEX as usize); } /// Internal: drain `self.pending` into `self.history`, applying From 2925d08273797756f643cbe0dd4210fee204bfd3 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 00:14:30 +0300 Subject: [PATCH 07/21] perf(encode): keep the repeat-offset history in registers across the pass The pass that derives the offset codes rotates the repeat-offset history on every sequence and reads it back on the next one. Held behind the caller's reference it compiled to three stores into the compressor per sequence: the loop also writes through the sequence slice, and the optimiser would not keep the array in registers across that. They are the second, third and fourth hottest instructions in the function's own profile, after the offset code's bit scan. A local copy, written back once when the pass ends, is the same three words moved once instead of once per sequence. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index e9877f2fb..84a89178c 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -1525,11 +1525,17 @@ fn fill_and_count( let mut ll_max = 0usize; let mut ml_max = 0usize; let mut of_max = 0usize; + // The history is rotated by every sequence and read by the next one. Held + // behind the caller's reference it was three stores into the compressor per + // sequence, because the loop also writes through the sequence slice and the + // optimiser would not keep the array in registers across that. A local copy + // written back once is the same three words, moved once. + let mut hist = *offset_hist; for seq in raw_sequences.iter_mut() { let off_base = if FAST_REPCODE { - encode_offset_with_history_fast(seq.off_base, seq.ll, offset_hist) + encode_offset_with_history_fast(seq.off_base, seq.ll, &mut hist) } else { - encode_offset_with_history(seq.off_base, seq.ll, offset_hist) + encode_offset_with_history(seq.off_base, seq.ll, &mut hist) }; seq.off_base = off_base; let ll_code = encode_literal_length(seq.ll).0 as usize; @@ -1542,6 +1548,7 @@ fn fill_and_count( ml_max = ml_max.max(ml_code); of_max = of_max.max(of_code); } + *offset_hist = hist; (ll_max, ml_max, of_max) } @@ -1552,15 +1559,18 @@ fn fill_wire_offsets( offset_hist: &mut [u32; 3], fast_repcode: bool, ) { + // Local copy for the same reason as `fill_and_count`. + let mut hist = *offset_hist; if fast_repcode { for seq in raw_sequences.iter_mut() { - seq.off_base = encode_offset_with_history_fast(seq.off_base, seq.ll, offset_hist); + seq.off_base = encode_offset_with_history_fast(seq.off_base, seq.ll, &mut hist); } } else { for seq in raw_sequences.iter_mut() { - seq.off_base = encode_offset_with_history(seq.off_base, seq.ll, offset_hist); + seq.off_base = encode_offset_with_history(seq.off_base, seq.ll, &mut hist); } } + *offset_hist = hist; } fn clone_fse_tables(fse_tables: &FseTables) -> FseTables { From 52d6c8becb3d474551f619fcb098f79060e2d12c Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 00:23:54 +0300 Subject: [PATCH 08/21] perf(encode): count the sequence codes into four interleaved tables One table a stream serialises the counting loop: consecutive sequences share a code often enough that the increment waits on store-to-load forwarding of the slot the previous one just wrote. Upstream counts into four tables and sums them at the end (`HIST_count_parallel_wksp`), which breaks the chain. Four narrow tables are also smaller than one wide one here. The three sequence alphabets top out at 35, 52 and 31, so four `[u32; 64]` tables a stream come to 3 KiB against the 6 KiB the caller's three `[usize; 256]` arrays already take; the sum writes the caller's arrays at the end and the slots above the alphabets stay zero, as they were. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 36 +++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index 84a89178c..f7f060915 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -1525,13 +1525,27 @@ fn fill_and_count( let mut ll_max = 0usize; let mut ml_max = 0usize; let mut of_max = 0usize; + // Counting into four interleaved tables per stream, as upstream's + // `HIST_count_parallel_wksp` does. Consecutive sequences share a code often + // enough that a single table serialises the loop on store-to-load + // forwarding of the same slot; rotating over four breaks that chain. + // + // They are also the counts the caller wants, in a narrower form: the three + // sequence alphabets top out at 35, 52 and 31, so four `[u32; 64]` tables a + // stream are 3 KiB in total against the 6 KiB the caller's three + // `[usize; 256]` arrays already occupy. + const LANES: usize = 4; + const CODES: usize = 64; + let mut ll_lanes = [[0u32; CODES]; LANES]; + let mut ml_lanes = [[0u32; CODES]; LANES]; + let mut of_lanes = [[0u32; CODES]; LANES]; // The history is rotated by every sequence and read by the next one. Held // behind the caller's reference it was three stores into the compressor per // sequence, because the loop also writes through the sequence slice and the // optimiser would not keep the array in registers across that. A local copy // written back once is the same three words, moved once. let mut hist = *offset_hist; - for seq in raw_sequences.iter_mut() { + for (index, seq) in raw_sequences.iter_mut().enumerate() { let off_base = if FAST_REPCODE { encode_offset_with_history_fast(seq.off_base, seq.ll, &mut hist) } else { @@ -1541,14 +1555,28 @@ fn fill_and_count( let ll_code = encode_literal_length(seq.ll).0 as usize; let ml_code = encode_match_len(seq.ml).0 as usize; let of_code = encode_offset(off_base).0 as usize; - ll_counts[ll_code] += 1; - ml_counts[ml_code] += 1; - of_counts[of_code] += 1; + let lane = index % LANES; + ll_lanes[lane][ll_code] += 1; + ml_lanes[lane][ml_code] += 1; + of_lanes[lane][of_code] += 1; ll_max = ll_max.max(ll_code); ml_max = ml_max.max(ml_code); of_max = of_max.max(of_code); } *offset_hist = hist; + // Plain `+`: the four lanes of one code sum to at most the block's sequence + // count, which a block header caps well under `u32::MAX`. + for code in 0..CODES { + ll_counts[code] = + (ll_lanes[0][code] + ll_lanes[1][code] + ll_lanes[2][code] + ll_lanes[3][code]) + as usize; + ml_counts[code] = + (ml_lanes[0][code] + ml_lanes[1][code] + ml_lanes[2][code] + ml_lanes[3][code]) + as usize; + of_counts[code] = + (of_lanes[0][code] + of_lanes[1][code] + of_lanes[2][code] + of_lanes[3][code]) + as usize; + } (ll_max, ml_max, of_max) } From ae9a71f9dc63f82dcc19e5e35ec356ffc4b71596 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 00:31:14 +0300 Subject: [PATCH 09/21] perf(encode): count into one table per stream after all Four interleaved count tables, upstream's `HIST_count_parallel_wksp` shape, cost more than the store-forwarding chain they remove. Measured on the i9, arms alternating, three rounds, against the commit before them: 8 MiB access log, level 1 +1.14% cycles, +2.26% instructions decodecorpus z000033, level 4 +2.18%, +0.75% decodecorpus z000033, level 5 +1.75%, +0.39% 8 MiB access log, level 9 +1.33%, +0.20% The lane index, the three extra address computations it forces, and the 192-entry fold at the end of each block are more work than the chain is worth at these sequence counts. Upstream pays neither: it counts over prepared byte arrays, where the loop carries nothing else. Recorded so it is not tried again from the same reasoning. --- zstd/src/encoding/blocks/compressed.rs | 36 +++----------------------- 1 file changed, 4 insertions(+), 32 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index f7f060915..84a89178c 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -1525,27 +1525,13 @@ fn fill_and_count( let mut ll_max = 0usize; let mut ml_max = 0usize; let mut of_max = 0usize; - // Counting into four interleaved tables per stream, as upstream's - // `HIST_count_parallel_wksp` does. Consecutive sequences share a code often - // enough that a single table serialises the loop on store-to-load - // forwarding of the same slot; rotating over four breaks that chain. - // - // They are also the counts the caller wants, in a narrower form: the three - // sequence alphabets top out at 35, 52 and 31, so four `[u32; 64]` tables a - // stream are 3 KiB in total against the 6 KiB the caller's three - // `[usize; 256]` arrays already occupy. - const LANES: usize = 4; - const CODES: usize = 64; - let mut ll_lanes = [[0u32; CODES]; LANES]; - let mut ml_lanes = [[0u32; CODES]; LANES]; - let mut of_lanes = [[0u32; CODES]; LANES]; // The history is rotated by every sequence and read by the next one. Held // behind the caller's reference it was three stores into the compressor per // sequence, because the loop also writes through the sequence slice and the // optimiser would not keep the array in registers across that. A local copy // written back once is the same three words, moved once. let mut hist = *offset_hist; - for (index, seq) in raw_sequences.iter_mut().enumerate() { + for seq in raw_sequences.iter_mut() { let off_base = if FAST_REPCODE { encode_offset_with_history_fast(seq.off_base, seq.ll, &mut hist) } else { @@ -1555,28 +1541,14 @@ fn fill_and_count( let ll_code = encode_literal_length(seq.ll).0 as usize; let ml_code = encode_match_len(seq.ml).0 as usize; let of_code = encode_offset(off_base).0 as usize; - let lane = index % LANES; - ll_lanes[lane][ll_code] += 1; - ml_lanes[lane][ml_code] += 1; - of_lanes[lane][of_code] += 1; + ll_counts[ll_code] += 1; + ml_counts[ml_code] += 1; + of_counts[of_code] += 1; ll_max = ll_max.max(ll_code); ml_max = ml_max.max(ml_code); of_max = of_max.max(of_code); } *offset_hist = hist; - // Plain `+`: the four lanes of one code sum to at most the block's sequence - // count, which a block header caps well under `u32::MAX`. - for code in 0..CODES { - ll_counts[code] = - (ll_lanes[0][code] + ll_lanes[1][code] + ll_lanes[2][code] + ll_lanes[3][code]) - as usize; - ml_counts[code] = - (ml_lanes[0][code] + ml_lanes[1][code] + ml_lanes[2][code] + ml_lanes[3][code]) - as usize; - of_counts[code] = - (of_lanes[0][code] + of_lanes[1][code] + of_lanes[2][code] + of_lanes[3][code]) - as usize; - } (ll_max, ml_max, of_max) } From 982e81c50a712517b57adaf3047776bed84384dd Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 00:36:34 +0300 Subject: [PATCH 10/21] perf(encode): let the histogram ask for the code without the extra bits `encode_literal_length` and `encode_match_len` return the symbol together with the extra bits it carries, and the extra-bit width comes from a table indexed by the symbol. A caller that wants only the symbol cannot have that lookup optimised away: its bounds check can panic, which makes it observable. So the counting pass paid two loads, two compares and two branches per sequence for two values it dropped -- they are four of the ten hottest instructions in the function. The symbol alone is now its own function, and the three call sites that want only the symbol call it. The offset code is `ilog2` with no table at all, so those sites take it directly rather than through the triple. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 52 +++++++++++++++++--------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index 84a89178c..4df577cf9 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -722,9 +722,9 @@ fn encode_block_parts( // here because these modes are written to the frame. let (last_ll, last_ml, last_of) = raw_sequences.last().map_or((0, 0, 0), |seq| { ( - encode_literal_length(seq.ll).0 as usize, - encode_match_len(seq.ml).0 as usize, - encode_offset(seq.off_base).0 as usize, + literal_length_code(seq.ll) as usize, + match_len_code(seq.ml) as usize, + seq.off_base.ilog2() as usize, ) }); @@ -1092,7 +1092,7 @@ fn estimate_sequences_section_bytes( for seq in sequences { let (ll, _, ll_bits) = encode_literal_length(seq.ll); let (ml, _, ml_bits) = encode_match_len(seq.ml); - let (of, _, _) = encode_offset(seq.off_base); + let of = seq.off_base.ilog2() as u8; ll_counts[ll as usize] += 1; ml_counts[ml as usize] += 1; of_counts[of as usize] += 1; @@ -1122,7 +1122,7 @@ fn estimate_sequences_section_bytes( let ll_mode = choose_table( ll_previous.as_ref(), ll_default, - sequences.iter().map(|seq| encode_literal_length(seq.ll).0), + sequences.iter().map(|seq| literal_length_code(seq.ll)), 9, strategy, ll_next, @@ -1130,7 +1130,7 @@ fn estimate_sequences_section_bytes( let ml_mode = choose_table( ml_previous.as_ref(), ml_default, - sequences.iter().map(|seq| encode_match_len(seq.ml).0), + sequences.iter().map(|seq| match_len_code(seq.ml)), 9, strategy, ml_next, @@ -1138,7 +1138,7 @@ fn estimate_sequences_section_bytes( let of_mode = choose_table( of_previous.as_ref(), of_default, - sequences.iter().map(|seq| encode_offset(seq.off_base).0), + sequences.iter().map(|seq| seq.off_base.ilog2() as u8), 8, strategy, of_next, @@ -1538,9 +1538,9 @@ fn fill_and_count( encode_offset_with_history(seq.off_base, seq.ll, &mut hist) }; seq.off_base = off_base; - let ll_code = encode_literal_length(seq.ll).0 as usize; - let ml_code = encode_match_len(seq.ml).0 as usize; - let of_code = encode_offset(off_base).0 as usize; + let ll_code = literal_length_code(seq.ll) as usize; + let ml_code = match_len_code(seq.ml) as usize; + let of_code = off_base.ilog2() as usize; ll_counts[ll_code] += 1; ml_counts[ml_code] += 1; of_counts[of_code] += 1; @@ -2578,13 +2578,24 @@ const ML_EXTRA_BITS: [u8; 53] = [ /// table index. Every code's baseline is a multiple of its own extra-bit width, /// so masking off those bits is the same subtraction the ranges spelled out. #[inline] -fn encode_literal_length(len: u32) -> (u8, u32, usize) { +fn literal_length_code(len: u32) -> u8 { debug_assert!(len < 131_072, "literal length {len} out of encodable range"); - let code = if len < 64 { + if len < 64 { LL_CODE[len as usize] } else { (len.ilog2() + LL_DELTA_CODE) as u8 - }; + } +} + +/// [`literal_length_code`] plus the extra bits it carries. +/// +/// Separate from the code alone because the extra-bit lookup is a bounds +/// check, which can panic and so cannot be dropped as dead: a caller that +/// wants only the symbol — the histogram pass — was paying the load and the +/// check for a value it discarded. +#[inline] +fn encode_literal_length(len: u32) -> (u8, u32, usize) { + let code = literal_length_code(len); let bits = LL_EXTRA_BITS[code as usize] as usize; (code, len & ((1u32 << bits) - 1), bits) } @@ -2595,19 +2606,26 @@ fn encode_literal_length(len: u32) -> (u8, u32, usize) { /// /// Table-driven for the same reason as [`encode_literal_length`]. #[inline] -fn encode_match_len(len: u32) -> (u8, u32, usize) { +fn match_len_code(len: u32) -> u8 { debug_assert!( (3..131_075).contains(&len), "match length {len} out of encodable range", ); let base = len - 3; - let code = if base < 128 { + if base < 128 { ML_CODE[base as usize] } else { (base.ilog2() + ML_DELTA_CODE) as u8 - }; + } +} + +/// [`match_len_code`] plus the extra bits it carries. Split for the same +/// reason as [`encode_literal_length`]. +#[inline] +fn encode_match_len(len: u32) -> (u8, u32, usize) { + let code = match_len_code(len); let bits = ML_EXTRA_BITS[code as usize] as usize; - (code, base & ((1u32 << bits) - 1), bits) + (code, (len - 3) & ((1u32 << bits) - 1), bits) } /// Convert an actual byte offset into the encoded offset code, using repeat offset From c9e2eb8fa8ee375499baf1ef4e669635800d52d1 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 00:45:30 +0300 Subject: [PATCH 11/21] perf(encode): inline the code-only helpers into their call sites As a hint the split-out symbol functions were left out of line at the histogram's call site, so the pass paid a call per sequence for a table lookup it had stopped doing: retired instructions went up rather than down, by 1.71% at level 1 on the access log. Forcing the inline is what the split was for. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index 4df577cf9..fa623f86f 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -2577,7 +2577,10 @@ const ML_EXTRA_BITS: [u8; 53] = [ /// compiled to a chain of comparisons; upstream reaches the same answer with a /// table index. Every code's baseline is a multiple of its own extra-bit width, /// so masking off those bits is the same subtraction the ranges spelled out. -#[inline] +// `inline(always)`: as a hint it was left out of line at the histogram's call +// site, and a call per sequence costs more than the table lookup this split +// removes. +#[inline(always)] fn literal_length_code(len: u32) -> u8 { debug_assert!(len < 131_072, "literal length {len} out of encodable range"); if len < 64 { @@ -2605,7 +2608,8 @@ fn encode_literal_length(len: u32) -> (u8, u32, usize) { /// does). Codes are keyed on `len - 3`, the form the sequence section stores. /// /// Table-driven for the same reason as [`encode_literal_length`]. -#[inline] +// `inline(always)` for the same reason as [`literal_length_code`]. +#[inline(always)] fn match_len_code(len: u32) -> u8 { debug_assert!( (3..131_075).contains(&len), From 9edf2466af70c13d5a95a47b8477a2e44718ef57 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 00:52:45 +0300 Subject: [PATCH 12/21] perf(encode): keep the one code-plus-bits helper after all Splitting the symbol out of `encode_literal_length` / `encode_match_len` so the histogram could skip the extra-bit lookup did remove the two bounds checks from its loop -- they are gone from the disassembly -- and still measured worse, twice, on the i9 with arms alternating: 8 MiB access log, level 1 +1.18% cycles, +1.71% instructions decodecorpus z000033, level 4 +2.00%, +0.50% decodecorpus z000033, level 5 +0.69%, +0.26% 8 MiB access log, level 9 +3.12%, +0.16% `inline(always)` on the split helpers changed the instruction count by two in fifteen billion, so they were already inlined and the call-overhead explanation was wrong. Whatever the compiler does differently with the narrower helper costs more than the lookup it saves, and the lookup is not where the loop's time goes -- the offset code's bit scan is, at four times the share. Recorded so the same reasoning does not produce the same patch again. --- zstd/src/encoding/blocks/compressed.rs | 60 ++++++++------------------ 1 file changed, 19 insertions(+), 41 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index fa623f86f..84a89178c 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -722,9 +722,9 @@ fn encode_block_parts( // here because these modes are written to the frame. let (last_ll, last_ml, last_of) = raw_sequences.last().map_or((0, 0, 0), |seq| { ( - literal_length_code(seq.ll) as usize, - match_len_code(seq.ml) as usize, - seq.off_base.ilog2() as usize, + encode_literal_length(seq.ll).0 as usize, + encode_match_len(seq.ml).0 as usize, + encode_offset(seq.off_base).0 as usize, ) }); @@ -1092,7 +1092,7 @@ fn estimate_sequences_section_bytes( for seq in sequences { let (ll, _, ll_bits) = encode_literal_length(seq.ll); let (ml, _, ml_bits) = encode_match_len(seq.ml); - let of = seq.off_base.ilog2() as u8; + let (of, _, _) = encode_offset(seq.off_base); ll_counts[ll as usize] += 1; ml_counts[ml as usize] += 1; of_counts[of as usize] += 1; @@ -1122,7 +1122,7 @@ fn estimate_sequences_section_bytes( let ll_mode = choose_table( ll_previous.as_ref(), ll_default, - sequences.iter().map(|seq| literal_length_code(seq.ll)), + sequences.iter().map(|seq| encode_literal_length(seq.ll).0), 9, strategy, ll_next, @@ -1130,7 +1130,7 @@ fn estimate_sequences_section_bytes( let ml_mode = choose_table( ml_previous.as_ref(), ml_default, - sequences.iter().map(|seq| match_len_code(seq.ml)), + sequences.iter().map(|seq| encode_match_len(seq.ml).0), 9, strategy, ml_next, @@ -1138,7 +1138,7 @@ fn estimate_sequences_section_bytes( let of_mode = choose_table( of_previous.as_ref(), of_default, - sequences.iter().map(|seq| seq.off_base.ilog2() as u8), + sequences.iter().map(|seq| encode_offset(seq.off_base).0), 8, strategy, of_next, @@ -1538,9 +1538,9 @@ fn fill_and_count( encode_offset_with_history(seq.off_base, seq.ll, &mut hist) }; seq.off_base = off_base; - let ll_code = literal_length_code(seq.ll) as usize; - let ml_code = match_len_code(seq.ml) as usize; - let of_code = off_base.ilog2() as usize; + let ll_code = encode_literal_length(seq.ll).0 as usize; + let ml_code = encode_match_len(seq.ml).0 as usize; + let of_code = encode_offset(off_base).0 as usize; ll_counts[ll_code] += 1; ml_counts[ml_code] += 1; of_counts[of_code] += 1; @@ -2577,28 +2577,14 @@ const ML_EXTRA_BITS: [u8; 53] = [ /// compiled to a chain of comparisons; upstream reaches the same answer with a /// table index. Every code's baseline is a multiple of its own extra-bit width, /// so masking off those bits is the same subtraction the ranges spelled out. -// `inline(always)`: as a hint it was left out of line at the histogram's call -// site, and a call per sequence costs more than the table lookup this split -// removes. -#[inline(always)] -fn literal_length_code(len: u32) -> u8 { +#[inline] +fn encode_literal_length(len: u32) -> (u8, u32, usize) { debug_assert!(len < 131_072, "literal length {len} out of encodable range"); - if len < 64 { + let code = if len < 64 { LL_CODE[len as usize] } else { (len.ilog2() + LL_DELTA_CODE) as u8 - } -} - -/// [`literal_length_code`] plus the extra bits it carries. -/// -/// Separate from the code alone because the extra-bit lookup is a bounds -/// check, which can panic and so cannot be dropped as dead: a caller that -/// wants only the symbol — the histogram pass — was paying the load and the -/// check for a value it discarded. -#[inline] -fn encode_literal_length(len: u32) -> (u8, u32, usize) { - let code = literal_length_code(len); + }; let bits = LL_EXTRA_BITS[code as usize] as usize; (code, len & ((1u32 << bits) - 1), bits) } @@ -2608,28 +2594,20 @@ fn encode_literal_length(len: u32) -> (u8, u32, usize) { /// does). Codes are keyed on `len - 3`, the form the sequence section stores. /// /// Table-driven for the same reason as [`encode_literal_length`]. -// `inline(always)` for the same reason as [`literal_length_code`]. -#[inline(always)] -fn match_len_code(len: u32) -> u8 { +#[inline] +fn encode_match_len(len: u32) -> (u8, u32, usize) { debug_assert!( (3..131_075).contains(&len), "match length {len} out of encodable range", ); let base = len - 3; - if base < 128 { + let code = if base < 128 { ML_CODE[base as usize] } else { (base.ilog2() + ML_DELTA_CODE) as u8 - } -} - -/// [`match_len_code`] plus the extra bits it carries. Split for the same -/// reason as [`encode_literal_length`]. -#[inline] -fn encode_match_len(len: u32) -> (u8, u32, usize) { - let code = match_len_code(len); + }; let bits = ML_EXTRA_BITS[code as usize] as usize; - (code, (len - 3) & ((1u32 << bits) - 1), bits) + (code, base & ((1u32 << bits) - 1), bits) } /// Convert an actual byte offset into the encoded offset code, using repeat offset From f25016ff71f73a64fe7155920776a4e6a19820fe Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 02:47:22 +0300 Subject: [PATCH 13/21] perf(encode): walk the sequence writer's loop as a slice The bit-writing loop counted down over indices and read `sequences[i]`, paying a bounds check and the index arithmetic on every sequence. Iterating the slice in reverse is the same order and the same last-sequence exclusion, as a pointer walk. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index 84a89178c..d6471b9c3 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -2358,8 +2358,11 @@ fn encode_sequences( unsafe { writer.flush_bulk(); } - for sequence in (0..=sequences.len() - 2).rev() { - let sequence = sequences[sequence]; + // Walked as a slice rather than by index: the same order, without the + // bounds check and the index arithmetic that `sequences[i]` pays on + // every sequence. The last one is coded through the FSE init states + // above, so it is not in this range. + for &sequence in sequences[..sequences.len() - 1].iter().rev() { let (ll_code, ll_add_bits, ll_num_bits) = encode_literal_length(sequence.ll); let (of_code, of_add_bits, of_num_bits) = encode_offset(sequence.off_base); let (ml_code, ml_add_bits, ml_num_bits) = encode_match_len(sequence.ml); From 4a61f882ec76885fc4a65424d99aebf9a4c37a91 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 02:55:58 +0300 Subject: [PATCH 14/21] perf(encode): carry the sequence codes from the derivation pass to the writer The bit writer is a quarter of a level-1 frame and two and a half times libzstd's; a fifth of what it spends goes on re-deriving the three FSE symbols and their extra-bit widths, which the pass that ran a moment earlier already had in hand. Upstream keeps them between the same two passes -- the byte arrays `ZSTD_seqToCodes` writes. The five values are packed into one word a sequence: the two codes at six bits, the offset code at five, and the two extra-bit widths at five, with the offset's width being its own code. The derivation pass writes it through the buffer's spare capacity, so the store costs no capacity test and no zeroing pass, and the writer reads one word instead of two bounds-checked table lookups, a bit scan and the branches that pick between the small-value tables and the logarithmic form. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 160 ++++++++++++++++--- zstd/src/encoding/blocks/compressed/tests.rs | 18 ++- 2 files changed, 156 insertions(+), 22 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index d6471b9c3..df1a2fe09 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -141,6 +141,11 @@ struct EncodedBlockParts { #[derive(Default)] pub(crate) struct CompressedBlockScratch { parts: EncodedBlockParts, + /// One packed [`SequenceCode`] per sequence of the partition being + /// encoded, filled by the pass that derives the offset codes and read by + /// the bit writer. Kept here so it is allocated once for the compressor + /// rather than per block. + sequence_codes: Vec, partitions: Vec, prefix_sums: SequencePrefixSums, compressed: Vec, @@ -306,6 +311,7 @@ pub fn compress_block(state: &mut CompressState, output: &mut Vec state, &scratch.parts.literals, &mut scratch.parts.sequences, + &mut scratch.sequence_codes, output, ); // This path writes the block it just encoded, so the tables it chose are @@ -345,6 +351,7 @@ pub(crate) fn compress_block_with_post_split( let mut emit_buffers = SingleSequenceEmitBuffers { output, compressed: &mut scratch.compressed, + codes: &mut scratch.sequence_codes, }; let emitted_raw = emit_single_sequence_block( state, @@ -454,6 +461,7 @@ pub(crate) fn compress_block_with_post_split( let mut emit_buffers = SingleSequenceEmitBuffers { output, compressed: &mut scratch.compressed, + codes: &mut scratch.sequence_codes, }; let emitted_raw = emit_single_sequence_block( state, @@ -601,6 +609,9 @@ fn encode_block_parts( state: &mut CompressState, literals_vec: &[u8], raw_sequences: &mut [RawSequence], + // Scratch for the packed per-sequence codes, carried by the caller so it is + // allocated once rather than per block. + codes: &mut Vec, output: &mut Vec, // What each axis decided, for the caller to apply once it knows the block // is kept. LL, ML, OF. @@ -709,11 +720,12 @@ fn encode_block_parts( state.strategy_tag, crate::encoding::strategy::StrategyTag::Fast ) { - fill_and_count::(raw_sequences, &mut state.offset_hist, counts) + fill_and_count::(raw_sequences, &mut state.offset_hist, counts, codes) } else { - fill_and_count::(raw_sequences, &mut state.offset_hist, counts) + fill_and_count::(raw_sequences, &mut state.offset_hist, counts, codes) }; let raw_sequences: &[RawSequence] = raw_sequences; + let codes: &[u32] = codes; let total = raw_sequences.len(); // Stream codes of the LAST sequence: upstream zstd codes the final symbol @@ -787,6 +799,7 @@ fn encode_block_parts( encode_sequences( raw_sequences, + codes, &mut writer, &ll_mode, &ml_mode, @@ -1407,6 +1420,7 @@ fn compressed_literals_header_bytes(lit_size: usize) -> usize { struct SingleSequenceEmitBuffers<'a> { output: &'a mut Vec, compressed: &'a mut Vec, + codes: &'a mut Vec, } fn emit_single_sequence_block( @@ -1435,7 +1449,13 @@ fn emit_single_sequence_block( // the decisions below. Copying them was also what kept the built table's // handle shared, which forced a fresh one per block. buffers.compressed.clear(); - let fse_decisions = encode_block_parts(state, literals, sequences, buffers.compressed); + let fse_decisions = encode_block_parts( + state, + literals, + sequences, + buffers.codes, + buffers.compressed, + ); let min_gain = (source_len >> 8) + 2; if buffers.compressed.len() >= source_len.saturating_sub(min_gain) { state.offset_hist = saved_offset_hist; @@ -1482,6 +1502,67 @@ fn emit_single_sequence_block( } } +/// One sequence's three FSE symbols and the two extra-bit widths that are not +/// derivable from a symbol alone, packed into a word. +/// +/// The derivation pass has all five in hand, and the bit writer needs all five +/// again a moment later; recomputing them there costs two table lookups with +/// their bounds checks, a bit scan and the branches that pick between the +/// small-value tables and the logarithmic form. Upstream keeps the same values +/// between the same two passes, as the three byte arrays `ZSTD_seqToCodes` +/// writes. +/// +/// The offset code is its own extra-bit width, so only two widths are stored. +/// Layout, low bits first: ll code 6, ml code 6, of code 5, ll bits 5, ml bits +/// 5 — 27 bits, and every field's range is fixed by the sequence-section +/// format. +struct SequenceCode(u32); + +impl SequenceCode { + #[inline(always)] + fn pack(ll_code: u8, ml_code: u8, of_code: u8, ll_bits: usize, ml_bits: usize) -> u32 { + debug_assert!(ll_code < 64 && ml_code < 64 && of_code < 32); + debug_assert!(ll_bits < 32 && ml_bits < 32); + ll_code as u32 + | (ml_code as u32) << 6 + | (of_code as u32) << 12 + | (ll_bits as u32) << 17 + | (ml_bits as u32) << 22 + } + + #[inline(always)] + fn ll_code(&self) -> u8 { + (self.0 & 63) as u8 + } + + #[inline(always)] + fn ml_code(&self) -> u8 { + (self.0 >> 6 & 63) as u8 + } + + #[inline(always)] + fn of_code(&self) -> u8 { + (self.0 >> 12 & 31) as u8 + } + + #[inline(always)] + fn ll_bits(&self) -> usize { + (self.0 >> 17 & 31) as usize + } + + #[inline(always)] + fn ml_bits(&self) -> usize { + (self.0 >> 22 & 31) as usize + } + + /// The offset code doubles as its own extra-bit width (upstream's + /// `ofBits = ofCode`). + #[inline(always)] + fn of_bits(&self) -> usize { + self.of_code() as usize + } +} + /// The three sequence-code histograms, passed as one argument so the pass that /// fills them stays under the register-pressure of six. struct SequenceCodeCounts<'a> { @@ -1516,6 +1597,7 @@ fn fill_and_count( raw_sequences: &mut [RawSequence], offset_hist: &mut [u32; 3], counts: SequenceCodeCounts<'_>, + codes: &mut Vec, ) -> (usize, usize, usize) { let SequenceCodeCounts { ll: ll_counts, @@ -1525,30 +1607,44 @@ fn fill_and_count( let mut ll_max = 0usize; let mut ml_max = 0usize; let mut of_max = 0usize; + // Written through the spare capacity rather than pushed: the length is + // known, so a push's capacity test per sequence buys nothing, and resizing + // first would zero the buffer only to overwrite all of it. + codes.clear(); + codes.reserve(raw_sequences.len()); + let code_slots = &mut codes.spare_capacity_mut()[..raw_sequences.len()]; // The history is rotated by every sequence and read by the next one. Held // behind the caller's reference it was three stores into the compressor per // sequence, because the loop also writes through the sequence slice and the // optimiser would not keep the array in registers across that. A local copy // written back once is the same three words, moved once. let mut hist = *offset_hist; - for seq in raw_sequences.iter_mut() { + for (slot, seq) in code_slots.iter_mut().zip(raw_sequences.iter_mut()) { let off_base = if FAST_REPCODE { encode_offset_with_history_fast(seq.off_base, seq.ll, &mut hist) } else { encode_offset_with_history(seq.off_base, seq.ll, &mut hist) }; seq.off_base = off_base; - let ll_code = encode_literal_length(seq.ll).0 as usize; - let ml_code = encode_match_len(seq.ml).0 as usize; - let of_code = encode_offset(off_base).0 as usize; - ll_counts[ll_code] += 1; - ml_counts[ml_code] += 1; - of_counts[of_code] += 1; - ll_max = ll_max.max(ll_code); - ml_max = ml_max.max(ml_code); - of_max = of_max.max(of_code); + let (ll_code, _, ll_bits) = encode_literal_length(seq.ll); + let (ml_code, _, ml_bits) = encode_match_len(seq.ml); + let (of_code, _, _) = encode_offset(off_base); + slot.write(SequenceCode::pack( + ll_code, ml_code, of_code, ll_bits, ml_bits, + )); + ll_counts[ll_code as usize] += 1; + ml_counts[ml_code as usize] += 1; + of_counts[of_code as usize] += 1; + ll_max = ll_max.max(ll_code as usize); + ml_max = ml_max.max(ml_code as usize); + of_max = of_max.max(of_code as usize); } *offset_hist = hist; + // SAFETY: the loop wrote every one of the `raw_sequences.len()` slots it + // took from the spare capacity, which `reserve` above guaranteed. + unsafe { + codes.set_len(raw_sequences.len()); + } (ll_max, ml_max, of_max) } @@ -2296,6 +2392,9 @@ fn commit_last_used_table( fn encode_sequences( sequences: &[RawSequence], + // One packed [`SequenceCode`] per sequence, in the same order, from the + // pass that derived the offset codes. + codes: &[u32], writer: &mut BitWriter<&mut Vec>, ll_mode: &FseTableMode<'_>, ml_mode: &FseTableMode<'_>, @@ -2309,10 +2408,15 @@ fn encode_sequences( mode.as_table(default) } + debug_assert_eq!(codes.len(), sequences.len()); let sequence = sequences[sequences.len() - 1]; - let (ll_code, ll_add_bits, ll_num_bits) = encode_literal_length(sequence.ll); - let (of_code, of_add_bits, of_num_bits) = encode_offset(sequence.off_base); - let (ml_code, ml_add_bits, ml_num_bits) = encode_match_len(sequence.ml); + let code = SequenceCode(codes[codes.len() - 1]); + let (ll_code, ll_num_bits) = (code.ll_code(), code.ll_bits()); + let (ml_code, ml_num_bits) = (code.ml_code(), code.ml_bits()); + let (of_code, of_num_bits) = (code.of_code(), code.of_bits()); + let ll_add_bits = low_bits(sequence.ll, ll_num_bits); + let ml_add_bits = low_bits(sequence.ml - 3, ml_num_bits); + let of_add_bits = low_bits(sequence.off_base, of_num_bits); let [ll_default, ml_default, of_default] = defaults; let ll_table = mode_table(ll_mode, ll_default); let ml_table = mode_table(ml_mode, ml_default); @@ -2362,10 +2466,18 @@ fn encode_sequences( // bounds check and the index arithmetic that `sequences[i]` pays on // every sequence. The last one is coded through the FSE init states // above, so it is not in this range. - for &sequence in sequences[..sequences.len() - 1].iter().rev() { - let (ll_code, ll_add_bits, ll_num_bits) = encode_literal_length(sequence.ll); - let (of_code, of_add_bits, of_num_bits) = encode_offset(sequence.off_base); - let (ml_code, ml_add_bits, ml_num_bits) = encode_match_len(sequence.ml); + for (&sequence, &code) in sequences[..sequences.len() - 1] + .iter() + .zip(codes[..codes.len() - 1].iter()) + .rev() + { + let code = SequenceCode(code); + let (ll_code, ll_num_bits) = (code.ll_code(), code.ll_bits()); + let (ml_code, ml_num_bits) = (code.ml_code(), code.ml_bits()); + let (of_code, of_num_bits) = (code.of_code(), code.of_bits()); + let ll_add_bits = low_bits(sequence.ll, ll_num_bits); + let ml_add_bits = low_bits(sequence.ml - 3, ml_num_bits); + let of_add_bits = low_bits(sequence.off_base, of_num_bits); // State diffs burst: max 30 bits (10+10+9 worst case for // acc_log ≤ 9 ll/ml + acc_log ≤ 8 of) + ≤ 7 leftover from @@ -2716,6 +2828,14 @@ pub(in crate::encoding) fn encode_offset_with_history_fast( actual_offset + 3 } +/// The low `bits` bits of `value`, the extra-bit field the sequence section +/// carries beside a code. Every baseline in the format is a multiple of its +/// code's field width, so masking is the subtraction of the baseline. +#[inline(always)] +fn low_bits(value: u32, bits: usize) -> u32 { + value & ((1u32 << bits) - 1) +} + fn encode_offset(len: u32) -> (u8, u32, usize) { let log = len.ilog2(); let lower = len & ((1 << log) - 1); diff --git a/zstd/src/encoding/blocks/compressed/tests.rs b/zstd/src/encoding/blocks/compressed/tests.rs index 05b82f667..3e51b1d68 100644 --- a/zstd/src/encoding/blocks/compressed/tests.rs +++ b/zstd/src/encoding/blocks/compressed/tests.rs @@ -383,7 +383,13 @@ fn estimator_literals_section_mirrors_emit_for_short_inputs() { let mut workspace = EstimatorWorkspace::default(); let est = estimate_block_parts_size(&mut est_state, &literals, &[], &mut workspace); let mut emitted: Vec = Vec::new(); - encode_block_parts(&mut emit_state, &literals, &mut [], &mut emitted); + encode_block_parts( + &mut emit_state, + &literals, + &mut [], + &mut Vec::new(), + &mut emitted, + ); assert_eq!( est, emitted.len(), @@ -443,7 +449,13 @@ fn a_section_with_flat_ends_costs_what_the_emitter_writes_for_it() { let mut workspace = EstimatorWorkspace::default(); let est = estimate_block_parts_size(&mut est_state, &literals, &[], &mut workspace); let mut emitted: Vec = Vec::new(); - encode_block_parts(&mut emit_state, &literals, &mut [], &mut emitted); + encode_block_parts( + &mut emit_state, + &literals, + &mut [], + &mut Vec::new(), + &mut emitted, + ); assert_eq!( est, @@ -499,9 +511,11 @@ fn raw_partition_fallback_restores_repeat_offset_history() { let mut output = Vec::new(); let mut compressed_scratch = Vec::new(); + let mut code_scratch = Vec::new(); let mut emit_buffers = super::SingleSequenceEmitBuffers { output: &mut output, compressed: &mut compressed_scratch, + codes: &mut code_scratch, }; let emitted_raw = emit_single_sequence_block( &mut state, From d13cb9327ecb6c010f29e5fd39b931c96894c8c2 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 03:14:50 +0300 Subject: [PATCH 15/21] perf(bitio): drop the zero-width branch from the unchecked bit add The unchecked add returned early on a zero width, to keep a full accumulator from shifting a `u64` by 64. That is a branch on every call and there are six a sequence in the FSE writer, which is a quarter of a level-1 frame. Upstream's `BIT_addBitsFast` has no such branch: it keeps `bitPos` strictly under 64 and shifts unconditionally. Masking the shift count says the same thing without the branch, and costs nothing to say -- x86 and AArch64 shift instructions mask the count themselves. The one case where the mask changes the arithmetic is the case the early return existed for, and there the caller's own precondition forces the value to zero, so the accumulator takes nothing either way. Part of #493. --- zstd/src/bit_io/bit_writer.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/zstd/src/bit_io/bit_writer.rs b/zstd/src/bit_io/bit_writer.rs index c9abddc73..e4ad9ad07 100644 --- a/zstd/src/bit_io/bit_writer.rs +++ b/zstd/src/bit_io/bit_writer.rs @@ -157,23 +157,13 @@ impl>> BitWriter { /// pays. #[inline(always)] pub unsafe fn write_bits_64_no_check(&mut self, bits: u64, num_bits: usize) { - // num_bits == 0 short-circuit: matches upstream zstd `BIT_addBits` no-op - // semantics AND guards the `bits << self.bits_in_partial` below - // from a `<< 64` undefined-behaviour evaluation when the - // accumulator is already full (`bits_in_partial == 64`). Callers - // that legitimately drain a full container (e.g. the FSE encoder - // hitting a state-diff burst boundary) can call this with - // `num_bits = 0` as a no-op without tripping UB. - if num_bits == 0 { - return; - } debug_assert!( num_bits + self.bits_in_partial <= 64, "write_bits_64_no_check would overflow partial: would push to {} bits", num_bits + self.bits_in_partial, ); debug_assert!( - self.bits_in_partial < 64, + self.bits_in_partial < 64 || num_bits == 0, "write_bits_64_no_check called with full accumulator and num_bits>0; \ caller must flush_bulk before adding more bits", ); @@ -181,7 +171,19 @@ impl>> BitWriter { num_bits == 64 || bits >> num_bits == 0, "value has dirty high bits beyond num_bits={num_bits}", ); - self.partial |= bits << self.bits_in_partial; + // Masked so a full accumulator cannot make this a shift by 64, which is + // undefined for `u64`. It is not a guard costing anything: x86 and + // AArch64 shift instructions mask the count themselves, so the mask + // disappears. It replaces a `num_bits == 0` early return, which was a + // branch on every call, and there are six a sequence. Upstream needs + // neither, because its `BIT_addBitsFast` keeps `bitPos` strictly under + // 64 and can shift unconditionally. + // + // Correct in the one case the mask changes: the accumulator is full + // only where the caller passes `num_bits == 0`, and the second + // precondition then forces `bits == 0`, so the OR contributes nothing + // whatever the shift count. + self.partial |= bits << (self.bits_in_partial & 63); self.bits_in_partial += num_bits; } From 3518d22710fafc9fa30ed32a526242ba9d9fe62d Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 03:27:16 +0300 Subject: [PATCH 16/21] perf(encode): find the highest sequence code from the counts, not per sequence The table selector needs the highest code with a non-zero count in each stream. It was carried as a running maximum through the counting loop -- three compares a sequence -- because deriving it afterwards meant a reverse scan of all 256 slots, which is the dominant cost on a small frame. The scan does not have to be 256 slots. The format's three sequence alphabets end at 35, 52 and 31, so nothing above 63 is ever counted, and 64 slots a stream a block is cheaper than three compares a sequence at any block worth encoding. The bound is asserted in debug builds against the counts themselves. Part of #493. --- zstd/src/encoding/blocks/compressed.rs | 30 ++++++++++++++++++++------ 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index df1a2fe09..4d56d07c1 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -1604,9 +1604,6 @@ fn fill_and_count( ml: ml_counts, of: of_counts, } = counts; - let mut ll_max = 0usize; - let mut ml_max = 0usize; - let mut of_max = 0usize; // Written through the spare capacity rather than pushed: the length is // known, so a push's capacity test per sequence buys nothing, and resizing // first would zero the buffer only to overwrite all of it. @@ -1635,9 +1632,6 @@ fn fill_and_count( ll_counts[ll_code as usize] += 1; ml_counts[ml_code as usize] += 1; of_counts[of_code as usize] += 1; - ll_max = ll_max.max(ll_code as usize); - ml_max = ml_max.max(ml_code as usize); - of_max = of_max.max(of_code as usize); } *offset_hist = hist; // SAFETY: the loop wrote every one of the `raw_sequences.len()` slots it @@ -1645,7 +1639,29 @@ fn fill_and_count( unsafe { codes.set_len(raw_sequences.len()); } - (ll_max, ml_max, of_max) + ( + highest_used_code(ll_counts), + highest_used_code(ml_counts), + highest_used_code(of_counts), + ) +} + +/// The highest code with a non-zero count, which the table selector needs and +/// would otherwise find by scanning all 256 slots. +/// +/// Carried as a running maximum through the counting loop until it was three +/// compares a sequence there against one bounded scan a stream a block. The +/// bound is the format's: the three sequence alphabets end at 35, 52 and 31, so +/// nothing above 63 is ever counted. +fn highest_used_code(counts: &[usize; 256]) -> usize { + debug_assert!( + counts[64..].iter().all(|&count| count == 0), + "a sequence code above 63 was counted; the alphabets end at 35 / 52 / 31", + ); + counts[..64] + .iter() + .rposition(|&count| count != 0) + .unwrap_or(0) } /// [`fill_and_count`] without the histogram, for the block-split estimator: it From 07a6854e8ea27ae62a90649b9a77ddb8d15b4924 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 10:23:43 +0300 Subject: [PATCH 17/21] fix(encode): count the sequence-code buffer and bound the Huffman histogram MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects this branch introduced, each with the regression test that catches it. The per-sequence code buffer is retained in the block scratch across blocks, like every other buffer there, but it was left out of `retained_heap_size`. A context therefore under-reported itself through `ZSTD_sizeof_CCtx` by four bytes per sequence of retained capacity, which is exactly the number a caller budgeting memory is given. The tree node's count narrowed from a machine word to a `u32`, which the encoder's own inputs cannot overflow — a literals section is at most 128 KiB. The entry points are public, though, and a histogram that does not fit truncated on the way into a leaf, overflowed at the first merge that crossed the boundary, and at exactly `u32::MAX` produced a leaf indistinguishable from the sentinel marking a node the tree has not built yet, which the merge loop would then take as a child. The bound is now stated where the narrowing happens, folded into the pass that already counts the leaves, so every entry point reaches it. Also adds the estimator/emitter case the parity tests were missing: they ran empty sequence arrays, so nothing compared the two on the path this branch rewrote. The two agree exactly on the literals section and cannot on the sequences one — the FSE cost model prices a symbol at its average width from the normalised probability, as upstream's `ZSTD_fseBitCost` does, where the writer pays what the state trajectory costs. The test asserts what does hold: the same repeat-offset history out of both, and a price within the model's rounding, measured at two bytes for its fixture on every strategy. Output byte-identical over 60 rows: three fixture shapes against ten levels, each run both plain and dictionary-primed. --- zstd/src/encoding/blocks/compressed.rs | 1 + zstd/src/encoding/blocks/compressed/tests.rs | 111 +++++++++++++++++++ zstd/src/huff0/huff0_encoder.rs | 27 ++++- zstd/src/huff0/huff0_encoder/tests.rs | 40 +++++++ 4 files changed, 178 insertions(+), 1 deletion(-) diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index 4d56d07c1..0d161fa50 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -182,6 +182,7 @@ impl CompressedBlockScratch { pub(crate) fn retained_heap_size(&self) -> usize { self.parts.literals.capacity() + self.parts.sequences.capacity() * core::mem::size_of::() + + self.sequence_codes.capacity() * core::mem::size_of::() + self.partitions.capacity() * core::mem::size_of::() + self.prefix_sums.heap_size() + self.compressed.capacity() diff --git a/zstd/src/encoding/blocks/compressed/tests.rs b/zstd/src/encoding/blocks/compressed/tests.rs index 3e51b1d68..a09228bdf 100644 --- a/zstd/src/encoding/blocks/compressed/tests.rs +++ b/zstd/src/encoding/blocks/compressed/tests.rs @@ -484,6 +484,117 @@ fn encode_match_len_uses_correct_upper_range_base() { assert_eq!(encode_match_len(131074), (52, 65535, 16)); } +/// The scratch is taken and put back around a block, so every buffer it keeps +/// is retained allocation a context reports through `ZSTD_sizeof_CCtx`. The +/// per-sequence code buffer is one of them, and a caller budgeting memory sees +/// whatever this sum leaves out. +#[test] +fn retained_heap_size_counts_the_sequence_code_buffer() { + let mut scratch = super::CompressedBlockScratch::new(); + let before = scratch.retained_heap_size(); + let codes = 1024; + scratch.sequence_codes.reserve_exact(codes); + let after = scratch.retained_heap_size(); + assert!( + after - before >= codes * core::mem::size_of::(), + "reserving {codes} sequence codes grew the reported retained size by only {} bytes", + after - before, + ); +} + +/// The estimator prices a block the splitter is thinking about; the emitter +/// writes the one it chose. They walk the sequences by different routes: the +/// emitter derives each offset code, packs it with the extra-bit widths and +/// runs the FSE writer, while the estimator counts from a scratch history and +/// prices the streams from a per-symbol cost model. The parity tests above use +/// empty sequence arrays; this one carries sequences through both. +/// +/// The two agree EXACTLY on the literals section, whose Huffman code lengths +/// are known per symbol. They cannot on the sequences section, and are not +/// meant to: the FSE cost model prices a symbol at its average width from the +/// normalised probability (upstream does the same in `ZSTD_fseBitCost`), where +/// the writer pays what the state trajectory actually costs. What must hold is +/// that the two stay within that model's rounding of each other, and that they +/// walked the same sequences — a wrong offset code on either side moves the +/// histogram, the table choice and the price by far more than rounding. +#[test] +fn estimator_and_emitter_agree_on_a_block_with_sequences() { + use super::{ + CompressedBlockScratch, EntropyOnlyMatcher, EstimatorWorkspace, encode_block_parts, + estimate_block_parts_size, + }; + // Enough sequences for the section to carry real FSE tables rather than + // the degenerate single-symbol shapes, with repeats among the offsets so + // the repeat-offset codes are exercised beside the explicit ones. + let literals: Vec = (0..512u32).map(|i| (i % 251) as u8).collect(); + let sequences: Vec = (0..64u32) + .map(|i| RawSequence { + ll: i % 8, + ml: 4 + i % 13, + off_base: 1 + (i % 5) * 7, + }) + .collect(); + + for strat in [StrategyTag::Fast, StrategyTag::Lazy, StrategyTag::BtUltra2] { + let make_state = || CompressState:: { + matcher: EntropyOnlyMatcher, + copy_tier: crate::decoding::simd_copy::ExactCopyTier::resolve(), + last_huff_table: None, + huff_table_spare: None, + huff_rollback: None, + huff_weights: Default::default(), + seen_content: Default::default(), + fse_tables: FseTables::new(), + block_scratch: CompressedBlockScratch::new(), + offset_hist: [1, 4, 8], + strategy_tag: strat, + pre_split: None, + huf_optimal_search: true, + literal_compression_disabled: false, + }; + let mut est_state = make_state(); + let mut emit_state = make_state(); + + let mut workspace = EstimatorWorkspace::default(); + let est = estimate_block_parts_size(&mut est_state, &literals, &sequences, &mut workspace); + + let mut emitted: Vec = Vec::new(); + let mut to_emit = sequences.clone(); + encode_block_parts( + &mut emit_state, + &literals, + &mut to_emit, + &mut Vec::new(), + &mut emitted, + ); + + // Both sides must have advanced the repeat-offset history the same + // way. This is the assertion that catches a wrong offset code: the + // byte counts could coincide, the histories cannot. + assert_eq!( + est_state.offset_hist, emit_state.offset_hist, + "estimator and emitter disagree on the repeat-offset history at {strat:?}", + ); + // The block really did carry a sequence section — otherwise the rest + // of this test would be passing on the literals path alone. + assert!(emitted.len() > literals.len() / 2); + // One byte per sixteen sequences plus four, against a model that + // rounds each symbol's width down from a 256-scale log table. The + // measured divergence for this fixture is two bytes on every strategy; + // the bound leaves room for the rounding to accumulate without letting + // a real divergence through, which moves the price by tens of bytes. + let allowed = 4 + sequences.len() / 16; + let diff = est.abs_diff(emitted.len()); + assert!( + diff <= allowed, + "estimator priced {est} bytes against {} emitted for {} sequences at {strat:?}: \ + off by {diff}, more than the {allowed} the cost model can round away", + emitted.len(), + sequences.len(), + ); + } +} + #[test] fn raw_partition_fallback_restores_repeat_offset_history() { let mut state = CompressState { diff --git a/zstd/src/huff0/huff0_encoder.rs b/zstd/src/huff0/huff0_encoder.rs index 9d55c008b..cd5372d0b 100644 --- a/zstd/src/huff0/huff0_encoder.rs +++ b/zstd/src/huff0/huff0_encoder.rs @@ -1231,7 +1231,32 @@ fn build_huffman_leaf_depths(counts: &[usize]) -> Vec { /// that builds a tree per block reuses one allocation instead of taking a fresh /// one every time. fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { - let leaf_count = counts.iter().filter(|&&count| count > 0).count(); + // The leaves and the total in one pass, since both walk the histogram. + // + // A node carries its count in a `u32` ([`HuffNode`]), which the encoder's + // own inputs cannot overflow: a literals section is at most 128 KiB, so its + // counts sum to that. The entry points are public, though, and a histogram + // that does not fit would truncate on the way into a leaf, overflow at the + // first merge that crosses the boundary, and — at exactly `u32::MAX` — + // produce a leaf indistinguishable from the sentinel that marks a node the + // tree has not built yet, which the merge loop would then select as a + // child. Refuse the histogram instead of producing a tree from any of the + // three. Strictly under `u32::MAX` so the sentinel stays unambiguous, and + // checked so the total itself cannot wrap on the way to the comparison. + let mut leaf_count = 0usize; + let mut total = 0usize; + for &count in counts { + if count > 0 { + leaf_count += 1; + total = total + .checked_add(count) + .expect("symbol counts sum to more than a histogram can describe"); + } + } + assert!( + total < u32::MAX as usize, + "symbol counts sum to {total}, which a tree node's count cannot hold", + ); // Pre-size to the final node count (`2 * leaf_count - 1`) so the tree // build's resize never reallocates. nodes.clear(); diff --git a/zstd/src/huff0/huff0_encoder/tests.rs b/zstd/src/huff0/huff0_encoder/tests.rs index 2158fde68..b3e73b73a 100644 --- a/zstd/src/huff0/huff0_encoder/tests.rs +++ b/zstd/src/huff0/huff0_encoder/tests.rs @@ -726,3 +726,43 @@ fn a_rejected_description_is_recorded_and_the_raw_form_written() { } assert_eq!(encoded, expected); } + +/// The tree node carries its count in a `u32`, which the encoder's own inputs +/// can never overflow: a literals section is at most 128 KiB, so its counts sum +/// to that. The entry point is public, though, and a caller handing it a +/// histogram whose counts do not fit gets a merge that overflows, a count that +/// truncates on the way into a node, and — at exactly `u32::MAX` — a leaf +/// indistinguishable from the sentinel that marks a node the tree has not built +/// yet. Say so at the boundary rather than let any of the three happen. +#[test] +#[should_panic(expected = "symbol counts sum to")] +fn build_from_counts_rejects_a_histogram_wider_than_a_node_count() { + let counts = [u32::MAX as usize, 1, 1]; + let _ = HuffmanTable::build_from_counts(&counts); +} + +/// The cheap path skips the table-log search but reaches the same tree +/// builder, so it needs the same bound. +#[test] +#[should_panic(expected = "symbol counts sum to")] +fn build_from_counts_gated_rejects_a_histogram_wider_than_a_node_count() { + let counts = [u32::MAX as usize, 1, 1]; + let _ = HuffmanTable::build_from_counts_gated(&counts, false); +} + +/// The bound is on the SUM, and the largest histogram the encoder can produce +/// has to stay well inside it: a full 128 KiB literals section over one symbol. +#[test] +fn build_from_counts_accepts_the_largest_section_the_encoder_can_produce() { + let mut counts = [0usize; 256]; + counts[b'a' as usize] = 128 * 1024 - 2; + counts[b'b' as usize] = 1; + counts[b'c' as usize] = 1; + let table = HuffmanTable::build_from_counts(&counts); + // Three symbols so the depths can differ at all — a two-symbol alphabet + // gives both a one-bit code whatever their counts. The frequent one taking + // the shorter code is the tree having been built from these counts rather + // than from truncated ones. + assert!(table.codes[b'a' as usize].1 < table.codes[b'b' as usize].1); + assert_eq!(table.codes[b'b' as usize].1, table.codes[b'c' as usize].1); +} From aee5c0fcc74c6565a4d3a4455e53bf0b445431c0 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 10:37:59 +0300 Subject: [PATCH 18/21] fix(huff0): bound the histogram at the entry points, not at the narrowing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit put the bound in the tree builder, reasoning that both entry points reach it so one check covers them by construction. It does not: the cheap path picks its table log first, and that sums the counts in a `usize`, which on a 32-bit target overflows on the very input the bound exists to reject. The i686 job caught it — the panic arrived from the sum rather than from the check, with a different message. The check moves to where the reviewer said it belonged, ahead of anything that reads the histogram, and the builder keeps a `debug_assert` for the invariant it now holds by construction. The total accumulates in a `u64`, so the bound and its message read the same on 32- and 64-bit targets, and saturates rather than wrapping: a total that saturates is far past the bound and refused either way. The alphabet bound moves with it. Only the search path asserted it; the cheap path reached the weight buffers and the node indices — both sized for 256 symbols — without one. Carries the test. Output byte-identical over 60 rows. --- zstd/src/huff0/huff0_encoder.rs | 69 +++++++++++++++++---------- zstd/src/huff0/huff0_encoder/tests.rs | 15 +++++- 2 files changed, 57 insertions(+), 27 deletions(-) diff --git a/zstd/src/huff0/huff0_encoder.rs b/zstd/src/huff0/huff0_encoder.rs index cd5372d0b..bd1313518 100644 --- a/zstd/src/huff0/huff0_encoder.rs +++ b/zstd/src/huff0/huff0_encoder.rs @@ -636,6 +636,7 @@ impl HuffmanTable { use_search: bool, scratch: &mut WeightScratch, ) -> Self { + assert_histogram_fits_nodes(counts); if use_search { Self::build_from_counts(counts) } else { @@ -649,7 +650,7 @@ impl HuffmanTable { } pub fn build_from_counts(counts: &[usize]) -> Self { - assert!(counts.len() <= 256); + assert_histogram_fits_nodes(counts); let symbol_cardinality = counts.iter().filter(|&&count| count > 0).count(); if symbol_cardinality <= 1 { return Self::build_from_weights(&build_limited_weights(counts, 11)); @@ -1214,6 +1215,41 @@ struct HuffNode { /// built. Out of range for a real node index, which is under `2 * 256 - 1`. const NO_PARENT: u16 = u16::MAX; +/// Refuse a histogram the tree cannot describe, before anything reads it. +/// +/// The encoder's own inputs always fit: a literals section is at most 128 KiB +/// over at most 256 symbols. The entry points are public, though, and a +/// histogram outside those bounds would not fail — it would build a wrong +/// tree. Counts wider than a node's `u32` truncate on the way into a leaf and +/// overflow at the first merge that crosses the boundary, and one of exactly +/// `u32::MAX` is indistinguishable from the sentinel marking a node the tree +/// has not built yet, which the merge loop would then take as a child. More +/// than 256 symbols overruns what the weight buffers and the node indices are +/// sized for. +/// +/// Called at the entry points rather than at the narrowing itself, because +/// what runs in between reads the histogram too: the cheap path's table-log +/// pick sums the counts in a `usize`, which on a 32-bit target overflows on +/// the same input this exists to reject, before the tree is ever built. +/// +/// The total accumulates in `u64` so the bound reads the same on 32- and +/// 64-bit targets, and saturates rather than wrapping — a total that saturates +/// is far past the bound and is refused either way. +fn assert_histogram_fits_nodes(counts: &[usize]) { + assert!( + counts.len() <= MAX_HUFFMAN_ALPHABET, + "histogram has {} symbols, more than the {MAX_HUFFMAN_ALPHABET} a Huffman table describes", + counts.len(), + ); + let total = counts + .iter() + .fold(0u64, |sum, &count| sum.saturating_add(count as u64)); + assert!( + total < u32::MAX as u64, + "symbol counts sum to {total}, which a tree node's count cannot hold", + ); +} + /// Build the count-sorted Huffman leaves with their natural (unlimited) code /// lengths in `nb_bits`. The tree shape is independent of any maximum-length /// limit, so this is computed once per block and shared across every @@ -1231,32 +1267,13 @@ fn build_huffman_leaf_depths(counts: &[usize]) -> Vec { /// that builds a tree per block reuses one allocation instead of taking a fresh /// one every time. fn build_huffman_leaf_depths_into(counts: &[usize], nodes: &mut Vec) { - // The leaves and the total in one pass, since both walk the histogram. - // - // A node carries its count in a `u32` ([`HuffNode`]), which the encoder's - // own inputs cannot overflow: a literals section is at most 128 KiB, so its - // counts sum to that. The entry points are public, though, and a histogram - // that does not fit would truncate on the way into a leaf, overflow at the - // first merge that crosses the boundary, and — at exactly `u32::MAX` — - // produce a leaf indistinguishable from the sentinel that marks a node the - // tree has not built yet, which the merge loop would then select as a - // child. Refuse the histogram instead of producing a tree from any of the - // three. Strictly under `u32::MAX` so the sentinel stays unambiguous, and - // checked so the total itself cannot wrap on the way to the comparison. - let mut leaf_count = 0usize; - let mut total = 0usize; - for &count in counts { - if count > 0 { - leaf_count += 1; - total = total - .checked_add(count) - .expect("symbol counts sum to more than a histogram can describe"); - } - } - assert!( - total < u32::MAX as usize, - "symbol counts sum to {total}, which a tree node's count cannot hold", + // Held by construction: both entry points run `assert_histogram_fits_nodes` + // before anything reads the histogram. + debug_assert!( + counts.iter().map(|&count| count as u64).sum::() < u32::MAX as u64, + "histogram reached the tree builder without passing the entry check", ); + let leaf_count = counts.iter().filter(|&&count| count > 0).count(); // Pre-size to the final node count (`2 * leaf_count - 1`) so the tree // build's resize never reallocates. nodes.clear(); diff --git a/zstd/src/huff0/huff0_encoder/tests.rs b/zstd/src/huff0/huff0_encoder/tests.rs index b3e73b73a..58c837739 100644 --- a/zstd/src/huff0/huff0_encoder/tests.rs +++ b/zstd/src/huff0/huff0_encoder/tests.rs @@ -742,7 +742,10 @@ fn build_from_counts_rejects_a_histogram_wider_than_a_node_count() { } /// The cheap path skips the table-log search but reaches the same tree -/// builder, so it needs the same bound. +/// builder, so it needs the same bound. It also reads the histogram on the way +/// there — its table-log pick sums the counts in a `usize`, which overflows on +/// this input on a 32-bit target — so the bound has to be stated at the entry, +/// not at the narrowing. #[test] #[should_panic(expected = "symbol counts sum to")] fn build_from_counts_gated_rejects_a_histogram_wider_than_a_node_count() { @@ -750,6 +753,16 @@ fn build_from_counts_gated_rejects_a_histogram_wider_than_a_node_count() { let _ = HuffmanTable::build_from_counts_gated(&counts, false); } +/// The alphabet bound belongs at the entry too: the search path asserted it, +/// the cheap path did not, and the weight buffers and node indices are sized +/// for 256 symbols on both. +#[test] +#[should_panic(expected = "more than the 256")] +fn build_from_counts_gated_rejects_an_alphabet_wider_than_a_huffman_table() { + let counts = alloc::vec![1usize; 257]; + let _ = HuffmanTable::build_from_counts_gated(&counts, false); +} + /// The bound is on the SUM, and the largest histogram the encoder can produce /// has to stay well inside it: a full 128 KiB literals section over one symbol. #[test] From ed74c5b3fce31aa7c88452103cf7fe26d8c07ba6 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 9 Sep 2026 12:48:36 +0300 Subject: [PATCH 19/21] perf(encode): one correction per slid slot, one histogram walk per build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The window slide subtracted the epoch bias and the drop separately from every table entry, though both are fixed for the whole slide and their sum lands at `u32::MAX` at the very most — the epoch keeps the bias under `u32::MAX - 2^31` and the drop is a length inside a history capped at `2 * max_window_size`. Summed once before the loop, it is one saturating subtraction a slot instead of two, on a loop that runs once per table entry. The bound is asserted rather than assumed: a wrap there would not fail, it would quietly resurrect dropped positions. The gated Huffman build validated the histogram and then delegated to `build_from_counts`, which validated it again — a second walk on every searched build, including each block-split candidate. Only the branch that does not delegate needs the check. Also adds the harness the window-slide question was settled with. Sliding the table's indices costs a pass over its entries where rebuilding costs a pass over the window's bytes, so the choice between them would matter if a table could be much larger than the window it indexes. It cannot, and the reasoning is now recorded at `drain_real_prefix`: a frame without a dictionary caps `hash_log` at `window_log + 1`, and a dictionary frame takes its table width from the dictionary's own cParams. Measured over `windowLog` 10 to 16 with `hashLog` pinned at 20: identical time without a dictionary, up to twice as fast with one, same output bytes on every row. Output byte-identical over 60 rows. --- ffi-bench/Cargo.toml | 4 + zstd/examples/slide_oversized_table.rs | 87 +++++++++++++++++++ .../encoding/simple/fast_kernel/hash_table.rs | 23 +++-- zstd/src/encoding/simple/fast_matcher.rs | 14 +++ zstd/src/huff0/huff0_encoder.rs | 5 +- 5 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 zstd/examples/slide_oversized_table.rs diff --git a/ffi-bench/Cargo.toml b/ffi-bench/Cargo.toml index 9e7a8e0c7..3c5891b7a 100644 --- a/ffi-bench/Cargo.toml +++ b/ffi-bench/Cargo.toml @@ -160,6 +160,10 @@ path = "../zstd/tests/he_level22_ratio.rs" name = "incompressibility_falsepos" path = "../zstd/tests/incompressibility_falsepos.rs" +[[example]] +name = "slide_oversized_table" +path = "../zstd/examples/slide_oversized_table.rs" + [[example]] name = "alloc_audit_decode" path = "../zstd/examples/alloc_audit_decode.rs" diff --git a/zstd/examples/slide_oversized_table.rs b/zstd/examples/slide_oversized_table.rs new file mode 100644 index 000000000..f99c4a891 --- /dev/null +++ b/zstd/examples/slide_oversized_table.rs @@ -0,0 +1,87 @@ +//! Window slides with a hash table far larger than the window. +//! +//! `hashLog` is capped at `windowLog + 1` for a frame with no dictionary, so +//! the table is at most twice the window there. The cap is lifted when a +//! dictionary is attached, because the main and dictionary tables share one +//! `hashLog` — which leaves a configuration the advanced API can ask for and +//! the level presets never produce: a table of a million entries over a window +//! of a kilobyte, sliding once per kilobyte of input. +//! +//! That is the shape where sliding the table's indices and rebuilding it from +//! the retained bytes cost very different amounts, so it is the fixture for +//! deciding between them. +//! +//! Build: cargo build --profile bench -p structured-zstd +//! --example slide_oversized_table --features hash,std,dict-builder +//! Run: ./target/release/examples/slide_oversized_table +//! + +use std::env; + +use structured_zstd::encoding::{ + CompressionLevel, CompressionParameters, FrameCompressor, Strategy, +}; + +/// Compressible but not degenerate: repeated lines with a rotating field, so +/// the matcher finds real matches across the window without the whole frame +/// collapsing to one repeat. +fn body(len: usize) -> Vec { + let mut bytes = Vec::with_capacity(len); + let mut counter = 0u32; + while bytes.len() < len { + let line = format!( + "ts=2026-03-26T21:39:{:02}Z level=INFO msg=\"flush memtable\" seq={counter} \ + tenant=demo table=orders region=eu-west\n", + counter % 60, + ); + let remaining = len - bytes.len(); + bytes.extend_from_slice(&line.as_bytes()[..line.len().min(remaining)]); + counter += 1; + } + bytes +} + +fn main() { + let args: Vec = env::args().collect(); + let window_log: u32 = args.get(1).and_then(|s| s.parse().ok()).unwrap_or(10); + let hash_log: u32 = args.get(2).and_then(|s| s.parse().ok()).unwrap_or(20); + let frame_bytes: usize = args + .get(3) + .and_then(|s| s.parse().ok()) + .unwrap_or(4 * 1024 * 1024); + let iters: u32 = args.get(4).and_then(|s| s.parse().ok()).unwrap_or(4); + let dict_path: Option<&str> = args.get(5).map(|s| s.as_str()); + + let src = body(frame_bytes); + + let params = CompressionParameters::builder(CompressionLevel::Level(1)) + .strategy(Strategy::Fast) + .window_log(window_log) + .hash_log(hash_log) + .build() + .expect("parameters within bounds"); + + let mut cctx: FrameCompressor = FrameCompressor::new(CompressionLevel::Level(1)); + cctx.set_parameters(¶ms); + if let Some(path) = dict_path { + let dict = std::fs::read(path).expect("read dict file"); + cctx.set_dictionary_from_bytes(&dict) + .expect("dictionary should attach"); + } + + let mut out: Vec = Vec::new(); + let mut sink: usize = 0; + for _ in 0..iters { + cctx.compress_independent_frame_into(&src, &mut out); + sink = sink.wrapping_add(out.len()); + core::hint::black_box(&out); + } + + eprintln!( + "windowLog={window_log} hashLog={hash_log} frame={frame_bytes} iters={iters} \ + dict={} out={} sum={sink} heap={}", + dict_path.unwrap_or("none"), + out.len(), + cctx.heap_size(), + ); +} diff --git a/zstd/src/encoding/simple/fast_kernel/hash_table.rs b/zstd/src/encoding/simple/fast_kernel/hash_table.rs index 0e99a37c6..3e0bcdbf9 100644 --- a/zstd/src/encoding/simple/fast_kernel/hash_table.rs +++ b/zstd/src/encoding/simple/fast_kernel/hash_table.rs @@ -242,13 +242,26 @@ impl FastHashTable { /// held before the slide, where a rebuild adds entries for positions the /// matcher had skipped and never indexed. /// - /// `saturating_sub` is the semantics on both lines, not a guard: below the - /// bias a slot is already empty, and at or under `drop_n` the position it - /// names is gone. Both floor at the sentinel. + /// `saturating_sub` is the semantics here, not a guard: below the bias a + /// slot is already empty, and at or under `drop_n` the position it names is + /// gone. Both floor at the sentinel, and both floor at the same place, so + /// the two are one subtraction of their sum — which is what the loop does, + /// since it runs once per table entry and both terms are fixed for the + /// whole slide. pub(crate) fn reduce_indices(&mut self, drop_n: u32) { - let bias = self.bias; + // Plain `+`: [`Self::advance_epoch`] keeps `bias <= u32::MAX - 2^31`, + // and `drop_n` is a length inside a history the matcher caps at + // `2 * max_window_size <= 2^31`, so the sum lands at `u32::MAX` at the + // very most. Asserted rather than assumed, because a wrap here would + // not fail — it would quietly resurrect dropped positions. + debug_assert!( + self.bias.checked_add(drop_n).is_some(), + "epoch bias {} plus a {drop_n}-byte slide overflows a stored position", + self.bias, + ); + let correction = self.bias + drop_n; for slot in self.table.iter_mut() { - *slot = slot.saturating_sub(bias).saturating_sub(drop_n); + *slot = slot.saturating_sub(correction); } self.bias = 0; } diff --git a/zstd/src/encoding/simple/fast_matcher.rs b/zstd/src/encoding/simple/fast_matcher.rs index 906df2d0c..3e630bd21 100644 --- a/zstd/src/encoding/simple/fast_matcher.rs +++ b/zstd/src/encoding/simple/fast_matcher.rs @@ -977,6 +977,20 @@ impl FastKernelMatcher { /// state: the rehash indexed every position, including the ones the /// matcher's step had skipped and never stored, so the table came out of /// a slide holding more than it held going in. + /// + /// The two costs scale differently — the slide with the table's entries, + /// the rehash with the window's bytes — so the choice between them would + /// matter if a table could be much larger than the window it indexes. It + /// cannot: a frame without a dictionary caps `hash_log` at + /// `window_log + 1` (upstream `ZSTD_adjustCParams_internal`), which bounds + /// the table at two entries per window byte, and a dictionary frame takes + /// its table width from the dictionary's own cParams rather than from a + /// caller's `hashLog`. Measured over `windowLog` 10 to 16 with `hashLog` + /// pinned at 20 (`examples/slide_oversized_table.rs`): identical time + /// without a dictionary, and up to twice as fast with one. So there is no + /// size-dependent choice here to make, and adding one would buy a branch + /// and two code paths for a configuration the parameter resolution does + /// not produce. fn drain_real_prefix(&mut self, drop_n: usize) { let drain_end = HISTORY_DRAIN_BASE + drop_n; self.history.drain(HISTORY_DRAIN_BASE..drain_end); diff --git a/zstd/src/huff0/huff0_encoder.rs b/zstd/src/huff0/huff0_encoder.rs index bd1313518..941529576 100644 --- a/zstd/src/huff0/huff0_encoder.rs +++ b/zstd/src/huff0/huff0_encoder.rs @@ -636,10 +636,13 @@ impl HuffmanTable { use_search: bool, scratch: &mut WeightScratch, ) -> Self { - assert_histogram_fits_nodes(counts); if use_search { + // Validated by the delegate, which is an entry point of its own. + // Checking here as well would walk the histogram a second time on + // every searched build, including each splitter candidate. Self::build_from_counts(counts) } else { + assert_histogram_fits_nodes(counts); // Match upstream's cheap path: tableLog = FSE_optimalTableLog(11, // srcSize, maxSV, minus=1) (huf_compress.c:1286), height-limit to it, // not the raw natural height (11) which can cost a few bytes vs C. From 99e6c81884d63058cebe13a6f6b2671f52d2926f Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 14 Sep 2026 20:15:57 +0300 Subject: [PATCH 20/21] docs(encode): describe the index slide and fix doc links - The slide_oversized_table header claimed a dictionary lifts the hashLog cap and yields a million-entry table over a 1 KiB window. It does not: a dictionary frame runs the dictionary's own table geometry, and runs at hashLog 11 and 20 print the same heap and output in both modes. The header now says what the example measures; its build command names ffi-bench, the package that registers it, without the feature list ffi-bench does not have. - extend_history_with_pending and trim_to_window still described the eviction as a table clear plus rehash; the drain slides stored positions down instead. - Repair 17 broken intra-doc links in the touched files (moved kernels, Self:: paths, the private MatcherStorage enum, rep[0] parsed as a link). Part of #493 --- zstd/examples/slide_oversized_table.rs | 28 +++++----- zstd/src/encoding/blocks/compressed.rs | 6 +-- .../encoding/simple/fast_kernel/hash_table.rs | 15 +++--- zstd/src/encoding/simple/fast_matcher.rs | 54 +++++++++---------- 4 files changed, 53 insertions(+), 50 deletions(-) diff --git a/zstd/examples/slide_oversized_table.rs b/zstd/examples/slide_oversized_table.rs index f99c4a891..ca1e76b36 100644 --- a/zstd/examples/slide_oversized_table.rs +++ b/zstd/examples/slide_oversized_table.rs @@ -1,20 +1,22 @@ -//! Window slides with a hash table far larger than the window. +//! Window slides under a small window with the widest `hashLog` a caller can +//! request. //! -//! `hashLog` is capped at `windowLog + 1` for a frame with no dictionary, so -//! the table is at most twice the window there. The cap is lifted when a -//! dictionary is attached, because the main and dictionary tables share one -//! `hashLog` — which leaves a configuration the advanced API can ask for and -//! the level presets never produce: a table of a million entries over a window -//! of a kilobyte, sliding once per kilobyte of input. +//! Sliding the table's indices costs a pass over the table; rebuilding it from +//! the retained bytes costs a pass over the window. The two diverge only when +//! the table is much larger than the window, so this asks for a table of a +//! million entries over a window of a kilobyte, sliding once per kilobyte of +//! input, and reports what the parameter resolution actually built. //! -//! That is the shape where sliding the table's indices and rebuilding it from -//! the retained bytes cost very different amounts, so it is the fixture for -//! deciding between them. +//! It does not build that table in either mode. Without a dictionary `hashLog` +//! is capped at `windowLog + 1`, two entries per window byte. With one, the +//! frame runs the dictionary's own table geometry and the requested `hashLog` +//! is not read at all. The `heap=` figure shows which table was built, and +//! running the same arguments with a smaller `hash_log` must print the same +//! figure in both modes. //! -//! Build: cargo build --profile bench -p structured-zstd -//! --example slide_oversized_table --features hash,std,dict-builder +//! Build: cargo build --profile bench -p ffi-bench --example slide_oversized_table //! Run: ./target/release/examples/slide_oversized_table -//! +//! [dict_path] use std::env; diff --git a/zstd/src/encoding/blocks/compressed.rs b/zstd/src/encoding/blocks/compressed.rs index 0d161fa50..f2717aeaa 100644 --- a/zstd/src/encoding/blocks/compressed.rs +++ b/zstd/src/encoding/blocks/compressed.rs @@ -304,7 +304,7 @@ impl Matcher for EntropyOnlyMatcher { } } -/// A block of [`crate::common::BlockType::Compressed`] +/// A block of [`crate::blocks::block::BlockType::Compressed`] pub fn compress_block(state: &mut CompressState, output: &mut Vec) { let mut scratch = core::mem::take(&mut state.block_scratch); collect_block_parts(state, &mut scratch.parts); @@ -1585,8 +1585,8 @@ struct SequenceCodeCounts<'a> { /// ours are already where they belong. /// /// `FAST_REPCODE` picks the offBase policy once per block instead of per -/// sequence. Upstream's fast matcher emits only offBase 1 (rep[0] when -/// litLength > 0, rep[1] when litLength == 0 via the secondary-position check) +/// sequence. Upstream's fast matcher emits only offBase 1 (`rep[0]` when +/// litLength > 0, `rep[1]` when litLength == 0 via the secondary-position check) /// or an explicit offset, and never 2/3; greedy and above search all three /// repeat offsets, which is what the full `encode_offset_with_history` mirrors. /// diff --git a/zstd/src/encoding/simple/fast_kernel/hash_table.rs b/zstd/src/encoding/simple/fast_kernel/hash_table.rs index 3e0bcdbf9..bb446b9bd 100644 --- a/zstd/src/encoding/simple/fast_kernel/hash_table.rs +++ b/zstd/src/encoding/simple/fast_kernel/hash_table.rs @@ -346,10 +346,11 @@ impl FastHashTable { (self.table.as_mut_slice(), self.hash_log) } - /// Like [`hot_state`] but also exposes the epoch `bias`, so a hot loop on a - /// POSSIBLY-biased table (the dict-attach kernels) can hoist the backing - /// slice + `hash_log` and apply the bias inline — `slot.saturating_sub(bias)` - /// on read, `pos + bias` on write — exactly as [`get`]/[`put`] do, without + /// Like [`Self::hot_state`] but also exposes the epoch `bias`, so a hot + /// loop on a POSSIBLY-biased table (the dict-attach kernels) can hoist the + /// backing slice + `hash_log` and apply the bias inline — + /// `slot.saturating_sub(bias)` on read, `pos + bias` on write — exactly as + /// [`Self::get`]/[`Self::put`] do, without /// re-reading the `Vec` header / `hash_log` / `bias` through `&mut self` on /// every access. On a bias-0 table this is identical to `hot_state` + raw /// access (`saturating_sub(0)` / `+ 0` fold away). @@ -365,7 +366,7 @@ impl FastHashTable { /// /// # Safety /// - /// `hash` MUST be a value returned by [`hash_ptr`] on this table + /// `hash` MUST be a value returned by [`Self::hash_ptr`] on this table /// (or on another table with the same `hash_log`), so that /// `hash < 1 << hash_log = table.len()`. #[inline(always)] @@ -383,11 +384,11 @@ impl FastHashTable { } /// Direct table write — `table[hash] = pos`. Same bounds reasoning - /// as [`get`]. + /// as [`Self::get`]. /// /// # Safety /// - /// `hash` MUST be a value returned by [`hash_ptr`] on this table. + /// `hash` MUST be a value returned by [`Self::hash_ptr`] on this table. #[inline(always)] pub(crate) unsafe fn put(&mut self, hash: u32, pos: u32) { debug_assert!((hash as usize) < self.table.len()); diff --git a/zstd/src/encoding/simple/fast_matcher.rs b/zstd/src/encoding/simple/fast_matcher.rs index 3e630bd21..405504d3d 100644 --- a/zstd/src/encoding/simple/fast_matcher.rs +++ b/zstd/src/encoding/simple/fast_matcher.rs @@ -19,7 +19,7 @@ //! Replaces the SuffixStore-based `MatchGenerator` for the Fast strategy //! path with a upstream zstd-parity hash table and tight per-block loop. //! -//! Wired into production: [`crate::encoding::match_generator::MatcherStorage::Simple`] +//! Wired into production: the driver's `MatcherStorage::Simple` variant //! holds `FastKernelMatcher` directly; the driver's Matcher trait //! methods (`commit_space` / `start_matching` / `skip_matching_with_hint` //! / `reset` / `prime_with_dictionary` / `trim_after_budget_retire`) @@ -85,7 +85,7 @@ pub(crate) const FAST_INITIAL_REP: [u32; 2] = [1, 4]; /// Initial offset-history seed for the encoder's repcode-coded /// offsets — matches upstream zstd's `repToConfirm[] = { 1, 4, 8 }` at frame -/// start and mirrors the value the old [`super::MatchGenerator`] used. +/// start. pub(crate) const FAST_INITIAL_OFFSET_HIST: [u32; 3] = [1, 4, 8]; /// Drain start offset used by eviction / drain paths. Set to 0: @@ -247,8 +247,9 @@ pub(crate) struct FastKernelMatcher { /// the dictionary region at the front of `history` (positions /// `[1, region_len)`), using the same `(hash_log, mls)` as /// [`Self::hash_table`] so a single hash keys both. Attached - /// (`is_attached()`) activates the dual-probe [`compress_block_fast_dict`] - /// kernel; invalidated on any history eviction (absolute dict positions + /// (`is_attached()`) activates the dual-probe + /// [`super::fast_kernel::kernel::compress_block_fast_dict`] kernel; + /// invalidated on any history eviction (absolute dict positions /// would otherwise go stale) so the no-dict kernel takes over — /// correctness-safe, only the dict ratio benefit is lost when the input is /// large enough to slide the dictionary out of the window. `region_len()` @@ -864,7 +865,7 @@ impl FastKernelMatcher { /// Accept a freshly-committed block from the driver. /// /// Upstream zstd's `ZSTD_window_update`: the new bytes are stashed for - /// the next [`Self::start_matching`] / [`Self::skip_matching`] + /// the next [`Self::start_matching`] / [`Self::skip_matching_with_hint`] /// call but NOT yet appended to `history` — that delay lets the /// driver-side `get_last_space` peek at the still-pending buffer /// without committing it to the matcher's hot path. @@ -1022,15 +1023,13 @@ impl FastKernelMatcher { /// `currentBlockStart` — what the kernel receives as /// `block_start`). /// - /// Eviction rule mirrors upstream zstd's `ZSTD_window_correctOverflow`: - /// when total retained bytes would exceed `2 × max_window_size`, - /// drop the oldest bytes back down to a `max_window_size` tail - /// and clear the hash table. The clear is forced because absolute - /// positions stored in the table would otherwise reference - /// evicted bytes; upstream zstd avoids the clear via a base-pointer trick - /// (`base += correction`) that the flat-`Vec` history can't - /// reuse, but pays for it with a one-time eviction every - /// `max_window_size` worth of input — amortised constant. + /// Eviction happens earlier, in `accept_data`: when total retained bytes + /// would exceed `2 × max_window_size`, the oldest bytes are dropped back + /// down to a `max_window_size` tail and the hash table's stored positions + /// slide down by the same amount ([`Self::drain_real_prefix`], upstream + /// zstd `ZSTD_reduceIndex`), so the retained entries keep naming the same + /// bytes. One eviction every `max_window_size` of input: amortised + /// constant. fn extend_history_with_pending(&mut self) -> usize { let mut space = self .pending @@ -1350,8 +1349,8 @@ impl FastKernelMatcher { /// C performs with `dictBase` / `ZSTD_count_2segments`, which the flat /// single-base path cannot. Dispatches the active `(mls, use_cmov)` pair to /// the monomorphised dual-base kernel - /// [`compress_block_fast_dict_borrowed`], which carries the owned dict - /// kernel's full machinery (repcode probe, step-ramp two-position + /// [`super::fast_kernel::kernel::compress_block_fast_dict_borrowed`], + /// which carries the owned dict kernel's full machinery (repcode probe, step-ramp two-position /// lookahead, dense fills, backward extension, immediate repcode-2 loop) — /// the prior scalar greedy scan had none of these and was +68% slower. /// Validated by roundtrip + cross-validation + the FFI ratio gate. @@ -1759,9 +1758,9 @@ impl FastKernelMatcher { /// Drop history bytes past `max_window_size` via /// [`Self::drain_real_prefix`] (resets `prefix_start_index` to - /// `INITIAL_PREFIX_START_INDEX` = 1 — the sentinel-0 floor — and - /// clears + rehashes the table). Returns evicted byte count; - /// idempotent when `real_len <= max_window_size`. + /// `INITIAL_PREFIX_START_INDEX` = 1, the sentinel-0 floor, and slides + /// the table's stored positions down by the evicted count). Returns + /// evicted byte count; idempotent when `real_len <= max_window_size`. pub(crate) fn trim_to_window(&mut self) -> usize { let real_len = self.history.len().saturating_sub(HISTORY_DRAIN_BASE); if real_len <= self.max_window_size { @@ -1994,12 +1993,11 @@ impl FastKernelMatcher { /// Dictionary-priming entry for the upstream zstd `dictMatchState` Fast path. /// Appends the pending dict slice to `history` and indexes its positions - /// into the SEPARATE immutable [`Self::dict_table`] — NOT the main hash - /// table. Keeping dict positions out of the main table is what lets the - /// dual-probe kernel prefer recent-input matches (main) over dictionary - /// matches (dict fallback), matching the upstream zstd's `prefixStart`/dict split. - /// Replaces the [`Self::skip_matching_with_hint`]`(Some(false))` call the - /// driver used to make for Fast-backend priming. + /// into the SEPARATE immutable dictionary table held by [`Self::dict`], NOT + /// the main hash table. Keeping dict positions out of the main table is + /// what lets the dual-probe kernel prefer recent-input matches (main) over + /// dictionary matches (dict fallback), matching the upstream zstd's + /// `prefixStart`/dict split. pub(crate) fn skip_matching_for_dict_prime(&mut self, dict_len: usize) { let block_start = self.extend_history_with_pending(); self.prime_dict_table_for_range(block_start, dict_len); @@ -2055,7 +2053,8 @@ impl FastKernelMatcher { self.dict.invalidate(); } - /// Build (or extend) [`Self::dict_table`] over `history[range_start..]`, + /// Build (or extend) the dictionary table in [`Self::dict`] over + /// `history[range_start..]`, /// the freshly-appended dictionary bytes. Lazily allocates the dict table /// at the CDict geometry of the whole `dict_len`-byte dictionary and the /// main table's `mls`, so one hash keys both. @@ -2394,7 +2393,8 @@ fn run_fast_kernel_block( } /// Dictionary-primed counterpart of [`run_fast_kernel_block`]: dispatches the -/// `(mls, use_cmov)` pair to [`compress_block_fast_dict`], threading the +/// `(mls, use_cmov)` pair to +/// [`super::fast_kernel::kernel::compress_block_fast_dict`], threading the /// immutable `dict_table` alongside the main table. Emits any terminal tail /// literals exactly as the no-dict helper does. #[allow(clippy::too_many_arguments)] From 60bcc6f0150356ba0409c1085e7372ed0fb4e0da Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 14 Sep 2026 21:19:50 +0300 Subject: [PATCH 21/21] perf(huff0): sum the histogram once on the cheap path - assert_histogram_fits_nodes already folds the counts to bound them; it now returns that total, and cheap_huf_table_log takes it instead of summing the same slice again. One pass less on every cheap table build, which is every block below btultra and every block-splitter candidate. Output is unchanged: a debug assertion checks the passed total against the sum over the whole debug suite. - slide_oversized_table names its two modes: its header describes the capped run (the default) and the dictionary run as separate benchmarks, and the output line starts with mode=capped or mode=dictionary. Part of #493 --- zstd/examples/slide_oversized_table.rs | 25 ++++++++++++++------- zstd/src/huff0/huff0_encoder.rs | 31 +++++++++++++++++--------- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/zstd/examples/slide_oversized_table.rs b/zstd/examples/slide_oversized_table.rs index ca1e76b36..040adfd25 100644 --- a/zstd/examples/slide_oversized_table.rs +++ b/zstd/examples/slide_oversized_table.rs @@ -7,12 +7,16 @@ //! million entries over a window of a kilobyte, sliding once per kilobyte of //! input, and reports what the parameter resolution actually built. //! -//! It does not build that table in either mode. Without a dictionary `hashLog` -//! is capped at `windowLog + 1`, two entries per window byte. With one, the -//! frame runs the dictionary's own table geometry and the requested `hashLog` -//! is not read at all. The `heap=` figure shows which table was built, and -//! running the same arguments with a smaller `hash_log` must print the same -//! figure in both modes. +//! Neither mode builds that table; each is a benchmark of the table the +//! resolution does build, and the output line names which one ran: +//! +//! - `mode=capped` (no `dict_path`, the default): `hashLog` is capped at +//! `windowLog + 1`, two entries per window byte. +//! - `mode=dictionary` (with `dict_path`): the frame runs the dictionary's own +//! table geometry, and the requested `hashLog` is not read at all. +//! +//! The `heap=` figure shows the table that was built; the same arguments with +//! a smaller `hash_log` print the same figure in both modes. //! //! Build: cargo build --profile bench -p ffi-bench --example slide_oversized_table //! Run: ./target/release/examples/slide_oversized_table @@ -79,9 +83,14 @@ fn main() { core::hint::black_box(&out); } + let mode = if dict_path.is_some() { + "dictionary" + } else { + "capped" + }; eprintln!( - "windowLog={window_log} hashLog={hash_log} frame={frame_bytes} iters={iters} \ - dict={} out={} sum={sink} heap={}", + "mode={mode} windowLog={window_log} hashLog={hash_log} frame={frame_bytes} \ + iters={iters} dict={} out={} sum={sink} heap={}", dict_path.unwrap_or("none"), out.len(), cctx.heap_size(), diff --git a/zstd/src/huff0/huff0_encoder.rs b/zstd/src/huff0/huff0_encoder.rs index 941529576..c2f3f3504 100644 --- a/zstd/src/huff0/huff0_encoder.rs +++ b/zstd/src/huff0/huff0_encoder.rs @@ -642,18 +642,22 @@ impl HuffmanTable { // every searched build, including each splitter candidate. Self::build_from_counts(counts) } else { - assert_histogram_fits_nodes(counts); + let total = assert_histogram_fits_nodes(counts); // Match upstream's cheap path: tableLog = FSE_optimalTableLog(11, // srcSize, maxSV, minus=1) (huf_compress.c:1286), height-limit to it, // not the raw natural height (11) which can cost a few bytes vs C. - build_limited_weights_into(counts, cheap_huf_table_log(counts), scratch); + build_limited_weights_into(counts, cheap_huf_table_log(counts, total), scratch); let spare = scratch.spare_table.take(); Self::build_from_weights_reusing(&scratch.weights, spare) } } pub fn build_from_counts(counts: &[usize]) -> Self { - assert_histogram_fits_nodes(counts); + let total = assert_histogram_fits_nodes(counts); + // Read only by the measurement-only cheap override below; the search + // does not need it. + #[cfg(not(feature = "bench-internals"))] + let _ = total; let symbol_cardinality = counts.iter().filter(|&&count| count > 0).count(); if symbol_cardinality <= 1 { return Self::build_from_weights(&build_limited_weights(counts, 11)); @@ -666,7 +670,7 @@ impl HuffmanTable { if FORCE_CHEAP_HUF.load(core::sync::atomic::Ordering::Relaxed) { return Self::build_from_weights(&build_limited_weights( counts, - cheap_huf_table_log(counts), + cheap_huf_table_log(counts, total), )); } @@ -1232,13 +1236,17 @@ const NO_PARENT: u16 = u16::MAX; /// /// Called at the entry points rather than at the narrowing itself, because /// what runs in between reads the histogram too: the cheap path's table-log -/// pick sums the counts in a `usize`, which on a 32-bit target overflows on -/// the same input this exists to reject, before the tree is ever built. +/// pick needs the total, and summing it in a `usize` would overflow on a +/// 32-bit target on the same input this exists to reject, before the tree is +/// ever built. /// /// The total accumulates in `u64` so the bound reads the same on 32- and /// 64-bit targets, and saturates rather than wrapping — a total that saturates /// is far past the bound and is refused either way. -fn assert_histogram_fits_nodes(counts: &[usize]) { +/// +/// Returns that total, so a caller that needs it (the cheap path's table-log +/// pick) does not walk the histogram a second time to get the same number. +fn assert_histogram_fits_nodes(counts: &[usize]) -> usize { assert!( counts.len() <= MAX_HUFFMAN_ALPHABET, "histogram has {} symbols, more than the {MAX_HUFFMAN_ALPHABET} a Huffman table describes", @@ -1251,6 +1259,8 @@ fn assert_histogram_fits_nodes(counts: &[usize]) { total < u32::MAX as u64, "symbol counts sum to {total}, which a tree node's count cannot hold", ); + // Under `u32::MAX` by the assert above, so it fits a 32-bit `usize`. + total as usize } /// Build the count-sorted Huffman leaves with their natural (unlimited) code @@ -1517,9 +1527,10 @@ fn limited_weights_into( /// probe is gated off: the single-shot /// `FSE_optimalTableLog_internal(HUF_TABLELOG_DEFAULT = 11, srcSize, maxSV, minus = 1)`. /// Degenerate `srcSize <= 1` (RLE-shaped, where `ilog2(srcSize - 1)` is undefined) -/// falls back to the natural-height cap of 11. -fn cheap_huf_table_log(counts: &[usize]) -> usize { - let total: usize = counts.iter().sum(); +/// falls back to the natural-height cap of 11. `total` is the sum of `counts`, +/// as [`assert_histogram_fits_nodes`] returns it. +fn cheap_huf_table_log(counts: &[usize], total: usize) -> usize { + debug_assert_eq!(total, counts.iter().sum::()); if total <= 1 { return 11; }