From a9ab166dfdf84785306fd4e51d0b8b06f71c4514 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 17:12:31 +0300 Subject: [PATCH 01/15] fix(dictionary): load raw content where upstream loads it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ZSTD_CCtx_loadDictionary` takes its buffer in `ZSTD_dct_auto` mode (`ZSTD_compress_insertDictionary`, zstd_compress.c:5216-5222): bytes that do not start with `ZSTD_MAGIC_DICTIONARY` are raw content, which is why any file can be handed to `zstd -D`. Our two setters for that same entry point rejected such a buffer as `BadMagicNum`, so a caller that works against libzstd failed here. - `FrameCompressor::set_dictionary_from_bytes` and the streaming encoder's namesake now load either kind. `EncoderDictionary::from_bytes` stays strict, so `ZSTD_dct_fullDict` remains reachable by parsing first and attaching the result. - `DictionaryHandle::from_serialized_or_raw_content` mirrors the `Dictionary` constructor, giving the decode side the same pair (`ZSTD_createDDict` is auto too, zstd_ddict.c:102-107). - C ABI: a `fullDict` selector over bytes that are not a dictionary now answers `dictionary_wrong` on the compression side, as upstream does (zstd_compress.c:5207 and 5223); the decompression side keeps `dictionary_corrupted` (zstd_ddict.c:99 and 105). A caller that branches on the code saw the wrong one. Each fix carries the regression test that failed without it. The CLI and the rest of the C ABI already classified on the magic, so `-D` over a plain file works in both directions and is left alone: `structured-zstd -6 -D rawdict4k` produces 3289 bytes where `zstd -6 -D` produces 3368, and each decodes the other's frame. Upstream additionally skips the dictionary content entirely below 8 bytes while still applying its dictionary-sized cParams, which costs it: on a 64 KiB record fixture with a 6-byte `-D`, `zstd -6` emits 5256 bytes against 3965 with no dictionary at all, while ours emits 3865 and decodes there. Not copied — it would trade ratio for a byte-identity that is not a goal. Closes #470 --- c-api/src/attach.rs | 8 ++- c-api/src/tests.rs | 34 +++++++++++ zstd/src/decoding/dictionary.rs | 8 +++ zstd/src/decoding/dictionary/tests.rs | 18 ++++++ zstd/src/encoding/frame_compressor.rs | 20 +++++-- zstd/src/encoding/frame_compressor/tests.rs | 63 ++++++++++++++++++++ zstd/src/encoding/streaming_encoder.rs | 18 +++--- zstd/src/encoding/streaming_encoder/tests.rs | 45 ++++++++++++++ 8 files changed, 201 insertions(+), 13 deletions(-) diff --git a/c-api/src/attach.rs b/c-api/src/attach.rs index 33dc56893..3dcaa0307 100644 --- a/c-api/src/attach.rs +++ b/c-api/src/attach.rs @@ -159,7 +159,13 @@ fn encode_raw_content(dict: &[u8], content_type: c_int) -> Result Ok(true), ZSTD_DCT_FULL_DICT => { if !has_magic { - return Err(ZSTD_ErrorCode::ZSTD_error_dictionary_corrupted); + // `dictionary_wrong`, not `corrupted`: on the compression side + // fullDict over bytes that are not a dictionary is the caller + // having named the wrong kind (`ZSTD_compress_insertDictionary`, + // zstd_compress.c:5223). The decode side answers `corrupted` + // for the same bytes (`ZSTD_loadEntropy_intoDDict`, + // zstd_ddict.c:105) — see `parse_decode_dict`. + return Err(ZSTD_ErrorCode::ZSTD_error_dictionary_wrong); } Ok(false) } diff --git a/c-api/src/tests.rs b/c-api/src/tests.rs index 0f329e386..61287611f 100644 --- a/c-api/src/tests.rs +++ b/c-api/src/tests.rs @@ -585,6 +585,40 @@ fn create_cdict_treats_unmagicked_bytes_as_raw_content() { assert!(cdict.is_null(), "corrupt full dict must fail"); } +/// The two sides report a `ZSTD_dct_fullDict` selector over bytes that are not +/// a dictionary with DIFFERENT codes, and a caller that branches on the code +/// sees the difference: the compression side answers `dictionary_wrong` +/// (`ZSTD_compress_insertDictionary`, zstd_compress.c:5207 and 5223), the +/// decompression side `dictionary_corrupted` (`ZSTD_loadEntropy_intoDDict`, +/// zstd_ddict.c:99 and 105). +#[test] +fn full_dict_over_unmagicked_bytes_reports_the_side_it_came_from() { + let raw = [0xABu8; 64]; + const FULL_DICT: c_int = 2; + + let cctx = ZSTD_createCCtx(); + let rc = + unsafe { ZSTD_CCtx_loadDictionary_advanced(cctx, raw.as_ptr(), raw.len(), 0, FULL_DICT) }; + assert_ne!(ZSTD_isError(rc), 0); + assert_eq!( + ZSTD_getErrorCode(rc), + ZSTD_ErrorCode::ZSTD_error_dictionary_wrong, + "the compression side calls a non-dictionary under fullDict `wrong`", + ); + unsafe { ZSTD_freeCCtx(cctx) }; + + let dctx = ZSTD_createDCtx(); + let rc = + unsafe { ZSTD_DCtx_loadDictionary_advanced(dctx, raw.as_ptr(), raw.len(), 0, FULL_DICT) }; + assert_ne!(ZSTD_isError(rc), 0); + assert_eq!( + ZSTD_getErrorCode(rc), + ZSTD_ErrorCode::ZSTD_error_dictionary_corrupted, + "the decompression side calls the same bytes `corrupted`", + ); + unsafe { ZSTD_freeDCtx(dctx) }; +} + // ---- Phase 6.2: advanced parameters + streaming ---- use crate::params::{ diff --git a/zstd/src/decoding/dictionary.rs b/zstd/src/decoding/dictionary.rs index 1f373383a..62a17d7b8 100644 --- a/zstd/src/decoding/dictionary.rs +++ b/zstd/src/decoding/dictionary.rs @@ -313,6 +313,14 @@ impl DictionaryHandle { Dictionary::decode_dict(raw).map(Self::from_dictionary) } + /// Load whichever kind of dictionary `raw` holds, as `ZSTD_createDDict` + /// does: a blob starting with [`MAGIC_NUM`] is a serialized dictionary, + /// anything else is raw content. See + /// [`Dictionary::from_serialized_or_raw_content`]. + pub fn from_serialized_or_raw_content(raw: &[u8]) -> Result { + Dictionary::from_serialized_or_raw_content(raw).map(Self::from_dictionary) + } + pub fn id(&self) -> u32 { self.inner.id } diff --git a/zstd/src/decoding/dictionary/tests.rs b/zstd/src/decoding/dictionary/tests.rs index 3754542af..24d26a3f2 100644 --- a/zstd/src/decoding/dictionary/tests.rs +++ b/zstd/src/decoding/dictionary/tests.rs @@ -105,6 +105,24 @@ fn dictionary_handle_from_raw_content_supports_as_ref() { assert_eq!(dict_ref.dict_content.as_slice(), &[42]); } +/// `ZSTD_createDDict` loads in `ZSTD_dct_auto` mode (zstd_ddict.c:102-107): a +/// buffer without the magic is raw content with no id and no entropy tables. +/// The handle is the shared form the decoder is handed, so it takes the same +/// two kinds the `Dictionary` constructor does. +#[test] +fn dictionary_handle_takes_serialized_or_raw_content() { + let serialized = include_bytes!("../../../dict_tests/dictionary"); + let parsed = DictionaryHandle::from_serialized_or_raw_content(serialized) + .expect("a magic-prefixed blob parses as a full dictionary"); + assert_ne!(parsed.id(), 0, "a full dictionary carries its id"); + + let raw = b"tenant=demo table=orders op=put".repeat(8); + let handle = DictionaryHandle::from_serialized_or_raw_content(&raw) + .expect("anything else is raw content"); + assert_eq!(handle.id(), 0, "raw content has no header to carry an id"); + assert_eq!(handle.as_dict().dict_content.as_slice(), raw.as_slice()); +} + #[test] fn dictionary_handle_clones_share_inner() { let raw = include_bytes!("../../../dict_tests/dictionary"); diff --git a/zstd/src/encoding/frame_compressor.rs b/zstd/src/encoding/frame_compressor.rs index 5805bd35f..88cebac1a 100644 --- a/zstd/src/encoding/frame_compressor.rs +++ b/zstd/src/encoding/frame_compressor.rs @@ -3113,16 +3113,26 @@ impl FrameCompressor { self.attach_dictionary(EncoderDictionary::from_dictionary(dictionary)) } - /// Parse and attach a serialized dictionary blob. + /// Parse and attach a dictionary blob, in either of the two forms upstream + /// `ZSTD_CCtx_loadDictionary` takes (`ZSTD_dct_auto`): a blob prefixed with + /// [`DICTIONARY_MAGIC`](crate::decoding::DICTIONARY_MAGIC) is a serialized + /// dictionary, and anything else is raw content, which is why any file can + /// be handed to `zstd -D`. Raw content has no id, so the frame records none + /// and the decoder must be given the same bytes explicitly. /// - /// Parses with the encoder-only path (skips the FSE/HUF decode lookup-table - /// build the encoder never reads); the entropy ENCODER tables — and thus - /// the emitted frame — are identical to a full parse. + /// A serialized blob parses through the encoder-only path (skips the + /// FSE/HUF decode lookup-table build the encoder never reads); the entropy + /// ENCODER tables — and thus the emitted frame — are identical to a full + /// parse. To reject anything but a serialized dictionary, parse with + /// [`EncoderDictionary::from_bytes`] (upstream `ZSTD_dct_fullDict`) and + /// attach the result. pub fn set_dictionary_from_bytes( &mut self, raw_dictionary: &[u8], ) -> Result, crate::decoding::errors::DictionaryDecodeError> { - self.attach_dictionary(EncoderDictionary::from_bytes(raw_dictionary)?) + self.attach_dictionary(EncoderDictionary::from_serialized_or_raw_content( + raw_dictionary, + )?) } /// Attach an already-parsed [`EncoderDictionary`] without reparsing a raw diff --git a/zstd/src/encoding/frame_compressor/tests.rs b/zstd/src/encoding/frame_compressor/tests.rs index ab0787b22..f886d6092 100644 --- a/zstd/src/encoding/frame_compressor/tests.rs +++ b/zstd/src/encoding/frame_compressor/tests.rs @@ -1227,6 +1227,69 @@ fn set_dictionary_accepts_a_dictionary_without_an_id() { ); } +/// Upstream loads a dictionary buffer in `ZSTD_dct_auto` mode +/// (`ZSTD_CCtx_loadDictionary` -> `ZSTD_compress_insertDictionary`, +/// zstd_compress.c:5216-5222): a buffer whose first four bytes are not +/// `ZSTD_MAGIC_DICTIONARY` is raw content, not a malformed dictionary. Any file +/// can therefore be handed to `zstd -D`, and the same has to hold here, or a +/// caller that works against libzstd fails against this one. +#[test] +fn set_dictionary_from_bytes_takes_unmagicked_bytes_as_raw_content() { + // Record-shaped, so the payload below actually matches into it: a blob the + // encoder cannot use would round-trip even with the dictionary silently + // dropped, and the frame-size check at the end would not hold. + let raw_dict = b"tenant=demo table=orders op=put value=aaaaabbbbbcccccdddddeeeee\n".repeat(16); + assert_ne!( + &raw_dict[..4], + &crate::decoding::DICTIONARY_MAGIC, + "the fixture must not start with the dictionary magic", + ); + let payload = b"tenant=demo table=orders op=put value=aaaaabbbbbcccccdddddeeeee\n".repeat(4); + + let mut with_dict = Vec::new(); + let mut compressor = FrameCompressor::new(super::CompressionLevel::Default); + compressor + .set_dictionary_from_bytes(&raw_dict) + .expect("raw content must load the way `zstd -D` loads it"); + compressor.set_source(payload.as_slice()); + compressor.set_drain(&mut with_dict); + compressor.compress(); + + // A raw-content dictionary has no header to carry an id, so the frame + // records none (upstream: `dictID = 0` is not written). + let (header, _) = crate::decoding::frame::read_frame_header(with_dict.as_slice()) + .expect("the frame header should read back"); + assert_eq!( + header.dictionary_id(), + None, + "a raw-content dictionary has no id to advertise", + ); + + let handle = crate::decoding::Dictionary::from_raw_content(0, raw_dict.clone()) + .expect("raw content is a valid dictionary") + .into_handle(); + let mut decoded = vec![0u8; payload.len()]; + let written = FrameDecoder::new() + .decode_all_with_dict_handle(&with_dict, &mut decoded, &handle) + .expect("the frame decodes against the same bytes"); + assert_eq!(&decoded[..written], payload.as_slice()); + + // And the dictionary was worth attaching: the same payload without it is + // larger, which is what proves the content was primed rather than parsed + // and discarded. + let mut without_dict = Vec::new(); + let mut plain = FrameCompressor::new(super::CompressionLevel::Default); + plain.set_source(payload.as_slice()); + plain.set_drain(&mut without_dict); + plain.compress(); + assert!( + with_dict.len() < without_dict.len(), + "the raw content should have been primed: {} vs {} bytes without it", + with_dict.len(), + without_dict.len(), + ); +} + #[test] fn set_dictionary_rejects_zero_repeat_offsets() { let invalid = crate::decoding::Dictionary { diff --git a/zstd/src/encoding/streaming_encoder.rs b/zstd/src/encoding/streaming_encoder.rs index 9a9994522..9f63036c1 100644 --- a/zstd/src/encoding/streaming_encoder.rs +++ b/zstd/src/encoding/streaming_encoder.rs @@ -323,14 +323,18 @@ impl StreamingEncoder { Ok(()) } - /// Attach a serialized dictionary blob to the frame (upstream zstd - /// `ZSTD_CCtx_loadDictionary` on a streaming context). The dictionary primes - /// the match-finder and seeds the first block's entropy tables + repeat - /// offsets, and its ID is written into the frame header. Must be called - /// before the first [`write`](Write::write); the parsed dictionary must have - /// a non-zero ID and non-zero repeat offsets. + /// Attach a dictionary blob to the frame (upstream zstd + /// `ZSTD_CCtx_loadDictionary` on a streaming context, which loads in + /// `ZSTD_dct_auto` mode): a blob prefixed with + /// [`DICTIONARY_MAGIC`](crate::decoding::DICTIONARY_MAGIC) is a serialized + /// dictionary, anything else is raw content. The dictionary primes the + /// match-finder and seeds the first block's entropy tables + repeat + /// offsets; a serialized one's ID is written into the frame header, while + /// raw content has none to write, so the decoder must be given the same + /// bytes explicitly. Must be called before the first + /// [`write`](Write::write); repeat offsets must be non-zero. pub fn set_dictionary_from_bytes(&mut self, raw_dictionary: &[u8]) -> Result<(), Error> { - let dict = EncoderDictionary::from_bytes(raw_dictionary) + let dict = EncoderDictionary::from_serialized_or_raw_content(raw_dictionary) .map_err(|err| invalid_input_error(&alloc::format!("invalid dictionary: {err:?}")))?; self.set_encoder_dictionary(dict) } diff --git a/zstd/src/encoding/streaming_encoder/tests.rs b/zstd/src/encoding/streaming_encoder/tests.rs index 85c49cd13..356ba921e 100644 --- a/zstd/src/encoding/streaming_encoder/tests.rs +++ b/zstd/src/encoding/streaming_encoder/tests.rs @@ -1160,3 +1160,48 @@ fn raw_dictionary_leaves_the_id_out_of_the_streaming_header() { decoder.read_to_end(&mut decoded).unwrap(); assert_eq!(decoded, payload); } + +/// The streaming setter is the same upstream entry point as the one-shot one +/// (`ZSTD_CCtx_loadDictionary` on a streaming context), which loads in +/// `ZSTD_dct_auto` mode: bytes without `ZSTD_MAGIC_DICTIONARY` are raw content. +#[test] +fn set_dictionary_from_bytes_takes_unmagicked_bytes_as_raw_content() { + use crate::decoding::Dictionary; + + let content: Vec = b"tenant=demo region=eu table=orders payload=" + .iter() + .copied() + .cycle() + .take(2048) + .collect(); + assert_ne!( + content[..4], + crate::decoding::DICTIONARY_MAGIC, + "the fixture must not start with the dictionary magic", + ); + let mut payload = Vec::new(); + while payload.len() < 8192 { + payload.extend_from_slice(&content); + } + + let mut encoder = StreamingEncoder::new(Vec::new(), CompressionLevel::Default); + encoder + .set_dictionary_from_bytes(&content) + .expect("raw content must load the way `zstd -D` loads it"); + encoder.write_all(&payload).unwrap(); + let compressed = encoder.finish().unwrap(); + + // No header field to advertise: a raw-content dictionary carries no id. + assert_eq!(compressed[4] & 0b11, 0); + + let mut decoder = StreamingDecoder::new_with_dictionary_handle( + compressed.as_slice(), + &crate::decoding::DictionaryHandle::from_dictionary( + Dictionary::from_raw_content(0, content).expect("a raw dictionary has no id"), + ), + ) + .unwrap(); + let mut decoded = Vec::new(); + decoder.read_to_end(&mut decoded).unwrap(); + assert_eq!(decoded, payload); +} From e28f49b670af5e68142739097996008794063eef Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 17:35:16 +0300 Subject: [PATCH 02/15] refactor(bench): let the dict harness take either dictionary kind The raw-content fallback existed because `set_dictionary_from_bytes` rejected a blob without the magic; it now loads either kind, so the fallback was reachable only for a corrupt serialized dictionary, where re-reading the bytes as raw content with a made-up id is the wrong answer anyway. Part of #470 --- zstd/examples/encode_loop_dict.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/zstd/examples/encode_loop_dict.rs b/zstd/examples/encode_loop_dict.rs index f42ee6ee3..ebcf35f56 100644 --- a/zstd/examples/encode_loop_dict.rs +++ b/zstd/examples/encode_loop_dict.rs @@ -91,16 +91,12 @@ fn main() { let mut cctx: FrameCompressor = FrameCompressor::new(CompressionLevel::from_level(level)); if let Some(path) = dict_path { let dict = std::fs::read(path).expect("read dict file"); - // A finalized dict carries the zstd magic; a non-magic blob is raw - // content (the `ZSTD_createCDict`-on-raw-bytes path the dict_matrix - // bench uses), attached with the id flag off so the frame omits a id. - if cctx.set_dictionary_from_bytes(&dict).is_err() { - let dict_obj = structured_zstd::decoding::Dictionary::from_raw_content(1, dict) - .expect("raw-content dictionary should build"); - cctx.set_dictionary_id_flag(false); - cctx.set_dictionary(dict_obj) - .expect("raw-content dictionary should attach"); - } + // Either kind, the way `zstd -D` takes it: a finalized dict carries + // the zstd magic, a non-magic blob is raw content (the + // `ZSTD_createCDict`-on-raw-bytes path the dict_matrix bench uses) and + // has no id, so the frame omits the field on its own. + cctx.set_dictionary_from_bytes(&dict) + .expect("dictionary should attach"); } // Output buffer reused across iterations (allocated once, replaced in From d5b7819c0bb5a8301574fcf90bd9ac6138770e81 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 17:54:29 +0300 Subject: [PATCH 03/15] perf(dfast): scan a dictionary block with one cursor, as upstream does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dictionary path ran on the no-dictionary loop with the dictionary probes bolted on. Upstream keeps the two apart (`ZSTD_compressBlock_doubleFast_dictMatchState_generic` against `..._noDict_generic`) and the difference is not decoration: the no-dictionary loop carries a second cursor, precomputing the long hash of ip+1 and carrying its index and slot across iterations, which buys a hash per two positions at the price of keeping that cursor's state live. The dictionary loop cannot afford it — it must also keep two table pointers, the dictionary's two hash shifts and its region bound live — and the profile showed exactly that: 93.4% of the dictionary path inside this loop, reloading its own invariants from the stack at each probe. So the dictionary gets its own kernel, scanning one cursor, hashing each position once, and stepping the way upstream's dictionary loop steps (`ip += ((ip - anchor) >> 8) + 1`, accelerating with the distance from the last match). The probe order is upstream's too, and one part of it does real work: the dictionary's short table is consulted only when the live short slot is empty or out of window, an `else if` on the slot rather than on the compare, so with the tables a small dictionary sizes that arm all but stops firing within a few hundred positions — where the old arrangement paid a hash, a load and a tag compare at every position. The two-cursor loop loses its dictionary code and its USE_DICT axis with it; a borrowed window never carries a dictionary, so the new kernel needs no BORROWED axis either. Compressed size across the whole dictionary matrix (93 scenario/level rows): one row moved, and downward — level_3_dfast/small-4k-log-lines 56 -> 55 bytes. Total 15,896,896 -> 15,896,895; the ratio against libzstd is unchanged at 1.01704. Closes #469 --- zstd/src/encoding/dfast/mod.rs | 957 ++++++++++++++++++++++----------- 1 file changed, 643 insertions(+), 314 deletions(-) diff --git a/zstd/src/encoding/dfast/mod.rs b/zstd/src/encoding/dfast/mod.rs index 63b17bbde..0642ae421 100644 --- a/zstd/src/encoding/dfast/mod.rs +++ b/zstd/src/encoding/dfast/mod.rs @@ -51,6 +51,19 @@ const HASH_READ_SIZE: usize = 8; /// rep emissions that upstream produces. const DFAST_REP_MIN_MATCH_LEN: usize = 4; +/// Upstream `kSearchStrength` (`zstd_compress_internal.h`): the dictionary +/// scan loop advances by `((ip - anchor) >> kSearchStrength) + 1`, so the +/// stride grows by one for every `1 << kSearchStrength` bytes travelled since +/// the last match. Expressed as the shift because that loop applies it per +/// position; the two-cursor loop counts the same distance out in +/// `DFAST_SKIP_STEP_GROWTH_INTERVAL`-sized intervals instead, and the two must +/// agree. +const DFAST_SKIP_STEP_SHIFT: usize = DFAST_SKIP_STEP_GROWTH_INTERVAL.trailing_zeros() as usize; +const _: () = assert!( + DFAST_SKIP_STEP_GROWTH_INTERVAL == 1 << DFAST_SKIP_STEP_SHIFT, + "the step-growth interval must be a power of two to express it as a shift", +); + /// Cached `DFTRACE` env flag for the dfast commit-path diagnostic (read once; /// see the `DFTRACE` gate in the fast-loop commit handler). #[cfg(feature = "std")] @@ -2094,7 +2107,7 @@ impl DfastMatchGenerator { /// cpl calls inline under one umbrella and `select_kernel()` is resolved ONCE /// per block in the bare dispatcher, never per cpl call. macro_rules! start_matching_fast_loop_body { - ($self:ident, $current_abs_start:ident, $current_len:ident, $handle_sequence:ident, $cpl:path, $use_dict:expr, $borrowed:expr) => {{ + ($self:ident, $current_abs_start:ident, $current_len:ident, $handle_sequence:ident, $cpl:path, $borrowed:expr) => {{ // Behaviour change vs the pre-refactor `start_matching_general`: // this fast loop deliberately drops the strict-incompressible // early-skip path (the `block_looks_incompressible_strict` short @@ -2193,57 +2206,6 @@ macro_rules! start_matching_fast_loop_body { let mut pos = 1usize; let mut literals_start = 0usize; - // Dict dual-probe (upstream zstd `ZSTD_dictMatchState`): snapshot the immutable - // dict tables ONCE. Unlike the live tables (which `emit_candidate` may - // grow / rebase), the dict tables are never mutated during matching, so - // one snapshot before the outer loop stays valid every iteration. The - // raw pointers hold no borrow, so the per-iter `&$self`/`&mut $self` - // accesses below coexist. `use_dict` gates every probe so the no-dict - // hot path keeps its exact instruction shape. `dict_end` is the - // dict/input boundary as a CONCAT index (history_start-relative); the - // dict is invalidated on any history eviction, so concat indices stay - // valid for the snapshot's lifetime. - // `use_dict` MUST track table presence, NOT `is_attached()`: - // `prime_dict_tables_for_range` records the dict region (so - // `is_attached()` is true) but returns before allocating the - // tables when the hashable region is shorter than the short-hash - // lookahead. Gating on `table().is_some()` keeps the null dict - // pointers out of the probe below, which dereferences them before - // the `dict_end` bound is consulted. - // Dict probe pointers are materialised ONLY on the `USE_DICT` kernel. - // The dispatcher monomorphises a separate no-dict kernel - // (`$use_dict == false`, a compile-time const) in which this block and - // every `if $use_dict` probe below const-fold away — so the hot no-dict - // loop carries zero dict code and zero per-position dict check, instead - // of branching on a loop-invariant flag every position (upstream zstd keeps the - // no-dict and dictMatchState loops as separate functions for the same - // reason). `$use_dict == true` is dispatched only when the table is - // present, so the `expect` never fires. - // The dictionary tables have their own widths (the CDict's), so the - // probes below hash the position at `dict_*_shift`, not the live - // tables' shifts. - let (dict_long_ptr, dict_short_ptr, dict_end, dict_long_shift, dict_short_shift): ( - *const u32, - *const u32, - usize, - usize, - usize, - ) = if $use_dict { - let d = $self - .dict - .table() - .expect("USE_DICT kernel dispatched without a dict table"); - ( - d.long.as_ptr(), - d.short.as_ptr(), - $self.dict.region_len(), - 64 - d.long_bits, - 64 - d.short_bits, - ) - } else { - (core::ptr::null(), core::ptr::null(), 0, 64, 64) - }; - // Advertised window cap = `1 << window_log`. Owned mode evicts, so // `history_abs_start` already bounds candidates to the live window; // borrowed mode keeps the whole input in place (no eviction), so an @@ -2804,81 +2766,6 @@ macro_rules! start_matching_fast_loop_body { } } - // Dict long fallback (upstream zstd `dictMatchState`): the live long - // missed (empty / out-of-window / 8-byte mismatch). Probe the - // immutable dict long table at the SAME `hl0_idx`. Flat model: - // the dict sits in the contiguous history before the input, so - // a dict match is `offset = abs_ip0 - dict_abs` and the forward - // count crosses the dict→input boundary like any in-window - // match (no `dictBase`/`count_2segments`). - if $use_dict { - // SAFETY: when `use_dict`, `dict_long_ptr` is non-null and - // sized `1 << long_hash_bits`; `hl0_idx < 1 << long_hash_bits`. - let dmix0 = v8_0.wrapping_mul(PRIME); - // SAFETY: `dmix0 >> dict_long_shift < 1 << long_bits`, the - // dict long table's length. - let dl = unsafe { *dict_long_ptr.add((dmix0 >> dict_long_shift) as usize) }; - // Tag check first (upstream `dictTagsMatchL`): a - // colliding slot never loads the dictionary bytes. - if dl != DFAST_EMPTY_SLOT - && (dl & DFAST_DICT_TAG_MASK) == dfast_dict_tag(dmix0, dict_long_shift) - { - let dp = ((dl >> DFAST_DICT_TAG_BITS) as usize) - 1; - // Dict long slots were only written for positions with - // 8-byte lookahead, so `dp + 8 <= dict_len <= concat_len`; - // `dp < dict_end` keeps the match inside the dict region. - if dp < dict_end { - debug_assert!( - dp + HASH_READ_SIZE <= concat_len, - "dict long load OOB: dp={dp} concat_len={concat_len}", - ); - // SAFETY: `dp + 8 <= concat_len` (above) ⇒ the 8-byte - // load at concat `dp` is in-bounds for live history. - let dcand_v8 = unsafe { - (history_base_ptr.add(history_start_offset + dp) as *const u64) - .read_unaligned() - }; - if dcand_v8 == v8_0 { - let mut match_len = 8usize; - let max_fwd = block_len - (ip0 + 8); - // SAFETY: both ptrs in the same buffer; `max_fwd` - // caps the scan to the live region. - unsafe { - let lhs = history_base_ptr.add(history_start_offset + dp + 8); - let rhs = block_ptr.add(ip0 + 8); - let ext = - $cpl( - lhs, rhs, max_fwd, - ); - match_len += ext; - } - let cand_pos = history_abs_start + dp; - // SAFETY: `history_base_ptr + history_start_offset` is the live - // source start (owned `history[history_start..]` or the - // borrowed input slice) and `concat_len` its readable byte - // count, both from `scan_source()` at the top of this outer - // iter; `extend_backwards_shared` only indexes within the - // candidate/cursor range it is handed, all `< concat_len`. - let concat = unsafe { - core::slice::from_raw_parts( - history_base_ptr.add(history_start_offset), - concat_len, - ) - }; - let cand = extend_backwards_shared( - concat, - history_abs_start, - cand_pos, - abs_ip0, - match_len, - ip0 - literals_start, - ); - break 'inner InnerExit::Committed(cand, 3, abs_ip0); - } - } - } - } - let idxl1 = unsafe { *long_hash_ptr.add(hl1_idx) }; // Short match check at ip0 with idxs0 — 4-byte gate @@ -2958,7 +2845,6 @@ macro_rules! start_matching_fast_loop_body { // If it produces a strictly longer match, use it. let mut chosen = short_cand; let mut retry_upgraded = false; - let mut live_l1_hit = false; if idxl1 != DFAST_EMPTY_SLOT { let cand_pos_l1 = position_base + (idxl1 as usize) - 1; if cand_pos_l1 >= wlow1 && cand_pos_l1 < abs_ip1 { @@ -2969,7 +2855,6 @@ macro_rules! start_matching_fast_loop_body { .read_unaligned() }; if cand_v8_l1 == v8_1 { - live_l1_hit = true; let mut l1_match_len = 8usize; let max_fwd_l1 = block_len - (ip1 + 8); unsafe { @@ -3015,82 +2900,6 @@ macro_rules! start_matching_fast_loop_body { } } } - // Dict long match at ip1 (upstream zstd `_search_next_long` - // dict arm, zstd_double_fast.c:472-483): probed ONLY - // when the live long+1 missed, mirroring upstream zstd's - // `else if dictTagsMatchL3`. Attach-mode keeps the - // dict in a SEPARATE table, so the live long+1 probe - // above never sees dict positions; without this the - // dict-long upgrade the old dense-reprime path got - // for free (dict positions lived in the live table) - // is lost and the loop emits the shorter short match. - if !live_l1_hit && $use_dict { - // SAFETY: `use_dict` ⇒ `dict_long_ptr` non-null, - // sized `1 << long_hash_bits`; `hl1_idx` is in range. - let dmix1 = v8_1.wrapping_mul(PRIME); - // SAFETY: the index is below the dict long table's length. - let dl1 = unsafe { - *dict_long_ptr.add((dmix1 >> dict_long_shift) as usize) - }; - if dl1 != DFAST_EMPTY_SLOT - && (dl1 & DFAST_DICT_TAG_MASK) - == dfast_dict_tag(dmix1, dict_long_shift) - { - let dp1 = ((dl1 >> DFAST_DICT_TAG_BITS) as usize) - 1; - if dp1 < dict_end { - debug_assert!( - dp1 + HASH_READ_SIZE <= concat_len, - "dict long+1 load OOB: dp1={dp1} concat_len={concat_len}", - ); - // SAFETY: `dp1 + 8 <= concat_len` ⇒ the - // 8-byte load at concat `dp1` is in-bounds. - let dcand_v8_l1 = unsafe { - (history_base_ptr.add(history_start_offset + dp1) - as *const u64) - .read_unaligned() - }; - if dcand_v8_l1 == v8_1 { - let mut dl1_match_len = 8usize; - let max_fwd = block_len - (ip1 + 8); - // SAFETY: same buffer; `max_fwd` caps - // the scan to the live region. - unsafe { - let lhs = history_base_ptr - .add(history_start_offset + dp1 + 8); - let rhs = block_ptr.add(ip1 + 8); - let ext = $cpl( - lhs, rhs, max_fwd, - ); - dl1_match_len += ext; - } - if dl1_match_len > short_cand.match_len { - let cand_pos = history_abs_start + dp1; - // SAFETY: `history_base_ptr + history_start_offset` is the live - // source start (owned `history[history_start..]` or the - // borrowed input slice) and `concat_len` its readable byte - // count, both from `scan_source()` at the top of this outer - // iter; `extend_backwards_shared` only indexes within the - // candidate/cursor range it is handed, all `< concat_len`. - let concat = unsafe { - core::slice::from_raw_parts( - history_base_ptr.add(history_start_offset), - concat_len, - ) - }; - chosen = extend_backwards_shared( - concat, - history_abs_start, - cand_pos, - abs_ip1, - dl1_match_len, - ip1 - literals_start, - ); - retry_upgraded = true; - } - } - } - } - } if short_hit_valid || retry_upgraded { // Upstream zstd `_match_found` (zstd_double_fast.c:287): // `if (step < 4) hashLong[hl1] = ip1`. @@ -3118,76 +2927,6 @@ macro_rules! start_matching_fast_loop_body { } } - // Dict short fallback (upstream zstd `dictMatchState`): the live short - // missed / was below floor. Probe the immutable dict short - // table at the SAME `hs0_idx`, 4-byte gate, forward count, then - // enforce the same `DFAST_MIN_MATCH_LEN` floor the live short - // path uses (a sub-floor non-rep match mints a wire offset that - // costs more than emitting the bytes as literals). No - // `_search_next_long` retry: the dict long fallback already - // covers the long-upgrade case at `ip0`. - if $use_dict { - // SAFETY: `use_dict` ⇒ `dict_short_ptr` non-null, sized - // `1 << short_hash_bits`; `hs0_idx < 1 << short_hash_bits`. - let dsmix0 = (v8_0 << 24).wrapping_mul(PRIME); - // SAFETY: the index is below the dict short table's length. - let ds = unsafe { *dict_short_ptr.add((dsmix0 >> dict_short_shift) as usize) }; - if ds != DFAST_EMPTY_SLOT - && (ds & DFAST_DICT_TAG_MASK) == dfast_dict_tag(dsmix0, dict_short_shift) - { - let dp = ((ds >> DFAST_DICT_TAG_BITS) as usize) - 1; - if dp < dict_end { - debug_assert!( - dp + 4 <= concat_len, - "dict short load OOB: dp={dp} concat_len={concat_len}", - ); - // SAFETY: short slots were only written with 4 bytes - // of lookahead ⇒ `dp + 4 <= dict_len <= concat_len`. - let dcand4 = unsafe { - (history_base_ptr.add(history_start_offset + dp) as *const u32) - .read_unaligned() - }; - if dcand4 == v4_0 as u32 { - let mut s_match_len = 4usize; - let max_fwd = block_len - (ip0 + 4); - unsafe { - let lhs = history_base_ptr.add(history_start_offset + dp + 4); - let rhs = block_ptr.add(ip0 + 4); - let ext = - $cpl( - lhs, rhs, max_fwd, - ); - s_match_len += ext; - } - let cand_pos = history_abs_start + dp; - // SAFETY: `history_base_ptr + history_start_offset` is the live - // source start (owned `history[history_start..]` or the - // borrowed input slice) and `concat_len` its readable byte - // count, both from `scan_source()` at the top of this outer - // iter; `extend_backwards_shared` only indexes within the - // candidate/cursor range it is handed, all `< concat_len`. - let concat = unsafe { - core::slice::from_raw_parts( - history_base_ptr.add(history_start_offset), - concat_len, - ) - }; - let dcand = extend_backwards_shared( - concat, - history_abs_start, - cand_pos, - abs_ip0, - s_match_len, - ip0 - literals_start, - ); - if dcand.match_len >= DFAST_MIN_MATCH_LEN { - break 'inner InnerExit::Committed(dcand, 4, abs_ip0); - } - } - } - } - } - // Step bump on distance (upstream zstd `zstd_double_fast.c:224-228`). // Upstream grows the step unbounded (one per `kStepIncr` travelled); // no cap, so the scan stride matches byte-for-byte. @@ -3270,6 +3009,481 @@ macro_rules! start_matching_fast_loop_body { }}; } +/// The dictionary scan loop, in the shape upstream gives its dictionary +/// variant (`ZSTD_compressBlock_doubleFast_dictMatchState_generic`, +/// zstd_double_fast.c:328-545) rather than the shape of its no-dictionary one. +/// +/// Upstream keeps the two apart, and the difference is not decoration. The +/// no-dictionary loop carries TWO cursors: it scans at `ip0`, precomputes the +/// long hash of `ip1` so the `_search_next_long` retry and the next iteration +/// both find it ready, and carries `hl1`/`idxl1` across iterations. That trade +/// buys a hash per two positions at the price of keeping the second cursor's +/// index, slot, position and window bound live the whole way round. The +/// dictionary loop cannot afford it: it must also keep two table pointers, the +/// dictionary's own two hash shifts and its region bound live, and the machine +/// has no registers left. Bolting the dictionary probes onto the two-cursor +/// loop is what this replaces; the profile of that arrangement had the loop +/// reloading its own invariants from the stack at every probe. +/// +/// So this scans one cursor, hashes each position once, and steps the way the +/// reference's dictionary loop steps — `ip += ((ip - anchor) >> 8) + 1`, +/// accelerating with the distance from the last match rather than from the +/// block start. The probe ORDER is the reference's too, and one part of it +/// matters for more than register pressure: the dictionary's short table is +/// consulted only when the live short slot is EMPTY or out of window +/// (zstd_double_fast.c:437-449, an `else if` on the slot, not on the compare). +/// With the tables a small dictionary sizes, every slot is occupied within a +/// few hundred positions, so that arm all but stops firing — which is the work +/// the two-cursor arrangement was doing on every single position. +/// +/// A borrowed window never carries a dictionary (`borrowed_eligible` rejects +/// `use_dictionary_state`), so this kernel is owned-coordinates only and needs +/// no `BORROWED` axis. +macro_rules! start_matching_dict_loop_body { + ($self:ident, $current_abs_start:ident, $current_len:ident, $handle_sequence:ident, $cpl:path) => {{ + debug_assert!($current_len > 0, "dict_loop precondition: $current_len > 0"); + $self.ensure_room_for($current_abs_start + $current_len - 1); + const PRIME: u64 = 0xCF1BBCDCB7A56463_u64; + let short_shift = 64 - $self.short_hash_bits; + let long_shift = 64 - $self.long_hash_bits; + let mut pos = 1usize; + let mut literals_start = 0usize; + + // The immutable dictionary tables, snapshotted once: nothing mutates + // them while matching, so unlike the live tables they need no + // re-snapshot per outer iteration. + let (dict_long_ptr, dict_short_ptr, dict_end, dict_long_shift, dict_short_shift) = { + let d = $self + .dict + .table() + .expect("dict kernel dispatched without a dict table"); + ( + d.long.as_ptr(), + d.short.as_ptr(), + $self.dict.region_len(), + 64 - d.long_bits, + 64 - d.short_bits, + ) + }; + + 'outer: loop { + if pos + HASH_READ_SIZE > $current_len { + break 'outer; + } + // Re-read every per-frame-mutable cursor: `emit_candidate` in the + // previous outer iteration may have rebased or grown history. + let ( + history_base_ptr, + history_start_offset, + history_abs_start, + position_base, + concat_len, + ) = $self.owned_scan_descriptor(); + let short_hash_ptr = $self.short_mut_ptr(); + let long_hash_ptr = $self.long_mut_ptr(); + let block_bias = $current_abs_start - history_abs_start; + // SAFETY: the block is part of live history, so its first byte is + // in bounds of the buffer. + let block_ptr = unsafe { history_base_ptr.add(history_start_offset + block_bias) }; + let block_len = concat_len - block_bias; + debug_assert_eq!(block_len, $current_len); + let scan_limit = $current_len - HASH_READ_SIZE; + let packed_bias = (($current_abs_start - position_base) as u32) + 1; + // Slot payload to candidate bytes in one add, as in the two-cursor + // kernel: see the note there for why the four coordinate constants + // are folded into a pointer rather than kept live. + let slot_base_ptr = history_base_ptr + .wrapping_add(history_start_offset) + .wrapping_offset(position_base as isize - history_abs_start as isize - 1); + let min_slot = ((history_abs_start - position_base) as u32) + 1; + + let mut ip = pos; + let inner_exit: DfastInnerExit = 'inner: loop { + let abs_ip = $current_abs_start + ip; + let packed_curr = (ip as u32) + packed_bias; + // SAFETY: the loop guard keeps `ip + 8 <= block_len`. + let v8 = unsafe { (block_ptr.add(ip) as *const u64).read_unaligned() }; + let v4 = v8 as u32; + let hl_idx = (v8.wrapping_mul(PRIME) >> long_shift) as usize; + let hs_idx = ((v8 << 24).wrapping_mul(PRIME) >> short_shift) as usize; + // SAFETY: both indices are below their table's length. + let (idxl, idxs) = + unsafe { (*long_hash_ptr.add(hl_idx), *short_hash_ptr.add(hs_idx)) }; + // Both tables updated at the cursor BEFORE the checks, as the + // reference does (zstd_double_fast.c:404): a self-collision on + // the `+1` long retry then resolves to a real match rather than + // to the slot's previous occupant. + // SAFETY: as above. + unsafe { + *long_hash_ptr.add(hl_idx) = packed_curr; + *short_hash_ptr.add(hs_idx) = packed_curr; + } + + // Repcode at ip+1, 4-byte gate (zstd_double_fast.c:407-415). + // `ip + 8 <= block_len` from the loop guard covers the read. + let rep1 = $self.offset_hist[0] as usize; + let idx_rep = ip + 1 + block_bias; + if rep1 != 0 && rep1 <= idx_rep { + let cand_idx_r = idx_rep - rep1; + // SAFETY: `cand_idx_r < idx_rep < concat_len`, and the + // 4-byte read at the cursor is inside the block. + let (cand4, cur4) = unsafe { + ( + (history_base_ptr.add(history_start_offset + cand_idx_r) as *const u32) + .read_unaligned(), + (block_ptr.add(ip + 1) as *const u32).read_unaligned(), + ) + }; + if cand4 == cur4 { + let mut match_len = 4usize; + let max_fwd = block_len - (ip + 1 + 4); + // SAFETY: same buffer; `max_fwd` caps the scan to the + // live region. + unsafe { + let lhs = history_base_ptr.add(history_start_offset + cand_idx_r + 4); + let rhs = block_ptr.add(ip + 1 + 4); + match_len += $cpl(lhs, rhs, max_fwd); + } + // Rep coding mints no offset, so the reference accepts + // a 4-byte hit here where a hash match needs 5. + if match_len >= DFAST_REP_MIN_MATCH_LEN { + // SAFETY: the source start and its readable length + // both come from `owned_scan_descriptor()` above; + // `extend_backwards_shared` indexes only within the + // candidate/cursor range it is handed. + let concat = unsafe { + core::slice::from_raw_parts( + history_base_ptr.add(history_start_offset), + concat_len, + ) + }; + let cand = extend_backwards_shared( + concat, + history_abs_start, + history_abs_start + cand_idx_r, + abs_ip + 1, + match_len, + ip + 1 - literals_start, + ); + break 'inner DfastInnerExit::Committed(cand, 0, abs_ip); + } + } + } + + // Long match at the cursor: live table first, dictionary only + // if that missed (zstd_double_fast.c:417-433, `else if + // dictTagsMatchL`). + if idxl >= min_slot && idxl < packed_curr { + // SAFETY: the gates admit only slots naming a position at + // or after the window floor and before the cursor, so this + // lands inside live history. + let cand_v8 = unsafe { + (slot_base_ptr.wrapping_add(idxl as usize) as *const u64).read_unaligned() + }; + if cand_v8 == v8 { + let cand_idx = position_base + (idxl as usize) - 1 - history_abs_start; + let mut match_len = 8usize; + let max_fwd = block_len - (ip + 8); + // SAFETY: same buffer; `max_fwd` caps the scan. + unsafe { + let lhs = history_base_ptr.add(history_start_offset + cand_idx + 8); + let rhs = block_ptr.add(ip + 8); + match_len += $cpl(lhs, rhs, max_fwd); + } + // SAFETY: as at the rep commit above. + let concat = unsafe { + core::slice::from_raw_parts( + history_base_ptr.add(history_start_offset), + concat_len, + ) + }; + let cand = extend_backwards_shared( + concat, + history_abs_start, + history_abs_start + cand_idx, + abs_ip, + match_len, + ip - literals_start, + ); + break 'inner DfastInnerExit::Committed(cand, 1, abs_ip); + } + } + { + let dmix = v8.wrapping_mul(PRIME); + // SAFETY: the index is below the dict long table's length. + let dl = unsafe { *dict_long_ptr.add((dmix >> dict_long_shift) as usize) }; + // The tag rejects a colliding slot without touching the + // dictionary bytes (upstream `ZSTD_comparePackedTags`). + if dl != DFAST_EMPTY_SLOT + && (dl & DFAST_DICT_TAG_MASK) == dfast_dict_tag(dmix, dict_long_shift) + { + let dp = ((dl >> DFAST_DICT_TAG_BITS) as usize) - 1; + if dp < dict_end { + debug_assert!(dp + HASH_READ_SIZE <= concat_len); + // SAFETY: dict long slots were written only for + // positions with 8 bytes of lookahead inside the + // dictionary region, so `dp + 8 <= concat_len`. + let dcand_v8 = unsafe { + (history_base_ptr.add(history_start_offset + dp) as *const u64) + .read_unaligned() + }; + if dcand_v8 == v8 { + let mut match_len = 8usize; + let max_fwd = block_len - (ip + 8); + // SAFETY: same buffer; `max_fwd` caps the scan. + // The dictionary sits contiguously before the + // input, so the count crosses the boundary like + // any in-window match (no `count_2segments`). + unsafe { + let lhs = history_base_ptr.add(history_start_offset + dp + 8); + let rhs = block_ptr.add(ip + 8); + match_len += $cpl(lhs, rhs, max_fwd); + } + // SAFETY: as at the rep commit above. + let concat = unsafe { + core::slice::from_raw_parts( + history_base_ptr.add(history_start_offset), + concat_len, + ) + }; + let cand = extend_backwards_shared( + concat, + history_abs_start, + history_abs_start + dp, + abs_ip, + match_len, + ip - literals_start, + ); + break 'inner DfastInnerExit::Committed(cand, 3, abs_ip); + } + } + } + } + + // Short match at the cursor. The dictionary's short table is + // consulted only when the live slot is EMPTY or out of window + // — the reference branches on the slot, not on the compare + // (zstd_double_fast.c:437-449) — so with a warm live table this + // arm all but stops firing. + let short_cand_idx: usize; + if idxs >= min_slot && idxs < packed_curr { + // SAFETY: as for the long slot above. + let cand4 = unsafe { + (slot_base_ptr.wrapping_add(idxs as usize) as *const u32).read_unaligned() + }; + if cand4 != v4 { + ip += ((ip - literals_start) >> DFAST_SKIP_STEP_SHIFT) + 1; + if ip > scan_limit { + break 'inner DfastInnerExit::Tail(ip); + } + continue 'inner; + } + short_cand_idx = position_base + (idxs as usize) - 1 - history_abs_start; + } else { + let dsmix = (v8 << 24).wrapping_mul(PRIME); + // SAFETY: the index is below the dict short table's length. + let ds = unsafe { *dict_short_ptr.add((dsmix >> dict_short_shift) as usize) }; + let mut found = usize::MAX; + if ds != DFAST_EMPTY_SLOT + && (ds & DFAST_DICT_TAG_MASK) == dfast_dict_tag(dsmix, dict_short_shift) + { + let dp = ((ds >> DFAST_DICT_TAG_BITS) as usize) - 1; + if dp < dict_end { + debug_assert!(dp + 4 <= concat_len); + // SAFETY: dict short slots were written only for + // positions with 4 bytes of lookahead inside the + // dictionary region. + let dcand4 = unsafe { + (history_base_ptr.add(history_start_offset + dp) as *const u32) + .read_unaligned() + }; + if dcand4 == v4 { + found = dp; + } + } + } + if found == usize::MAX { + ip += ((ip - literals_start) >> DFAST_SKIP_STEP_SHIFT) + 1; + if ip > scan_limit { + break 'inner DfastInnerExit::Tail(ip); + } + continue 'inner; + } + short_cand_idx = found; + } + + // `_search_next_long` (zstd_double_fast.c:453-483): a short hit + // is held while the long tables are asked about `ip+1`, and a + // strictly longer answer there wins. Guarded on the lookahead + // the `+1` probe needs; the reference gets the same guard from + // its strict `ip < ilimit`. + let mut s_match_len = 4usize; + let max_fwd = block_len - (ip + 4); + // SAFETY: same buffer; `max_fwd` caps the scan. + unsafe { + let lhs = history_base_ptr.add(history_start_offset + short_cand_idx + 4); + let rhs = block_ptr.add(ip + 4); + s_match_len += $cpl(lhs, rhs, max_fwd); + } + // SAFETY: as at the rep commit above. + let concat = unsafe { + core::slice::from_raw_parts( + history_base_ptr.add(history_start_offset), + concat_len, + ) + }; + let short_cand = extend_backwards_shared( + concat, + history_abs_start, + history_abs_start + short_cand_idx, + abs_ip, + s_match_len, + ip - literals_start, + ); + // A bare 4-byte hash hit mints a wire offset that costs more + // than the four bytes buy, so it is only taken from 5 up; the + // `+1` upgrade below starts at 8 and is always above the floor. + let mut chosen = short_cand; + let mut upgraded = false; + if ip + 1 + HASH_READ_SIZE <= block_len { + // SAFETY: guarded directly above. + let v8_1 = unsafe { (block_ptr.add(ip + 1) as *const u64).read_unaligned() }; + let hl1_idx = (v8_1.wrapping_mul(PRIME) >> long_shift) as usize; + // SAFETY: the index is below the long table's length. + let idxl1 = unsafe { *long_hash_ptr.add(hl1_idx) }; + let packed_next = packed_curr + 1; + let mut live_hit = false; + if idxl1 >= min_slot && idxl1 < packed_next { + // SAFETY: as for the long slot above. + let cand_v8 = unsafe { + (slot_base_ptr.wrapping_add(idxl1 as usize) as *const u64) + .read_unaligned() + }; + if cand_v8 == v8_1 { + live_hit = true; + let cand_idx = position_base + (idxl1 as usize) - 1 - history_abs_start; + let mut l1_len = 8usize; + let max_fwd = block_len - (ip + 1 + 8); + // SAFETY: same buffer; `max_fwd` caps the scan. + unsafe { + let lhs = history_base_ptr.add(history_start_offset + cand_idx + 8); + let rhs = block_ptr.add(ip + 1 + 8); + l1_len += $cpl(lhs, rhs, max_fwd); + } + if l1_len > short_cand.match_len { + chosen = extend_backwards_shared( + concat, + history_abs_start, + history_abs_start + cand_idx, + abs_ip + 1, + l1_len, + ip + 1 - literals_start, + ); + upgraded = true; + } + } + } + if !live_hit { + let dmix1 = v8_1.wrapping_mul(PRIME); + // SAFETY: the index is below the dict long table's length. + let dl1 = + unsafe { *dict_long_ptr.add((dmix1 >> dict_long_shift) as usize) }; + if dl1 != DFAST_EMPTY_SLOT + && (dl1 & DFAST_DICT_TAG_MASK) == dfast_dict_tag(dmix1, dict_long_shift) + { + let dp1 = ((dl1 >> DFAST_DICT_TAG_BITS) as usize) - 1; + if dp1 < dict_end { + debug_assert!(dp1 + HASH_READ_SIZE <= concat_len); + // SAFETY: as for the dict long probe above. + let dcand_v8 = unsafe { + (history_base_ptr.add(history_start_offset + dp1) as *const u64) + .read_unaligned() + }; + if dcand_v8 == v8_1 { + let mut dl1_len = 8usize; + let max_fwd = block_len - (ip + 1 + 8); + // SAFETY: same buffer; `max_fwd` caps the scan. + unsafe { + let lhs = + history_base_ptr.add(history_start_offset + dp1 + 8); + let rhs = block_ptr.add(ip + 1 + 8); + dl1_len += $cpl(lhs, rhs, max_fwd); + } + if dl1_len > short_cand.match_len { + chosen = extend_backwards_shared( + concat, + history_abs_start, + history_abs_start + dp1, + abs_ip + 1, + dl1_len, + ip + 1 - literals_start, + ); + upgraded = true; + } + } + } + } + } + } + if upgraded || short_cand.match_len >= DFAST_MIN_MATCH_LEN { + break 'inner DfastInnerExit::Committed(chosen, 2, abs_ip); + } + // A below-floor short hit with no upgrade: keep scanning. + ip += ((ip - literals_start) >> DFAST_SKIP_STEP_SHIFT) + 1; + if ip > scan_limit { + break 'inner DfastInnerExit::Tail(ip); + } + }; + + match inner_exit { + DfastInnerExit::Committed(candidate, _path_tag, scan_pos) => { + #[cfg(feature = "std")] + if *DFTRACE_ENABLED.get_or_init(|| std::env::var_os("DFTRACE").is_some()) { + std::eprintln!( + "DFT path={} off={} ml={} ll={}", + _path_tag, + candidate.offset, + candidate.match_len, + candidate.start - $current_abs_start - literals_start, + ); + } + let start = $self.emit_candidate( + $current_abs_start, + &mut literals_start, + candidate, + scan_pos, + $handle_sequence, + ); + pos = start + candidate.match_len; + pos = $self.extend_with_repcode_after_match( + $current_abs_start, + $current_len, + pos, + &mut literals_start, + $handle_sequence, + ); + } + DfastInnerExit::Tail(seed) => { + pos = seed; + break 'outer; + } + } + } + + $self.seed_remaining_hashable_starts($current_abs_start, $current_len, pos); + $self.emit_trailing_literals($current_abs_start, literals_start, $handle_sequence); + }}; +} + +/// How the dictionary scan loop left its inner loop: with a match to emit +/// (candidate, a path tag the `DFTRACE` gate prints, and the SCAN position the +/// complementary insertion anchors on), or out of scan room at the first +/// position it did not pack into the tables. +enum DfastInnerExit { + Committed(MatchCandidate, u8, usize), + Tail(usize), +} + impl DfastMatchGenerator { /// Dispatcher for the per-kernel dfast fast loop: resolve the tier ONCE /// per block via `select_kernel()` and call the matching @@ -3283,33 +3497,31 @@ impl DfastMatchGenerator { current_len: usize, handle_sequence: &mut impl for<'a> FnMut(Sequence<'a>), ) { - // Resolve dict presence ONCE here, off the hot path, and select a - // const-monomorphised kernel (`USE_DICT` true/false) so the per-position - // dict probe is compiled in or out at the call shape — never a - // loop-invariant runtime check inside the scan. The no-dict kernel - // carries zero dict code (upstream zstd keeps the noDict / dictMatchState loops - // as separate functions for exactly this reason). - // Two orthogonal axes resolved ONCE here, off the hot path, into a - // const-monomorphised kernel: - // * `USE_DICT` — dict probe compiled in or out (upstream zstd keeps noDict / - // dictMatchState as separate functions for the same reason). - // * `BORROWED` — borrowed-window scan vs owned history concat. The - // borrowed kernel folds the rebase coordinates to literal `0`, - // erasing the per-position abstraction arithmetic the owned path - // needs (upstream zstd `base + index` shape). - // A borrowed block never carries a dict (`borrowed_eligible` rejects - // `use_dictionary_state`), so only three of the four combinations are - // ever instantiated; the `` arm is unreachable. + // Which loop this block scans with is settled ONCE here, off the hot + // path, so nothing inside the scan branches on a block-invariant. + // + // A dictionary sends the block to its OWN loop, the way upstream keeps + // `noDict` and `dictMatchState` as separate functions: the shapes + // differ (one cursor against two) because the dictionary's tables, + // shifts and region bound leave no registers for a second cursor's + // state. See `start_matching_dict_loop_body!`. + // + // Without one, `BORROWED` picks between a borrowed-window scan and the + // owned history concat: the borrowed kernel folds the rebase + // coordinates to a literal `0`, erasing the per-position arithmetic the + // owned path needs (upstream `base + index` shape). A borrowed block + // never carries a dictionary (`borrowed_eligible` rejects + // `use_dictionary_state`), so the two axes never meet. let use_dict = self.dict.table().is_some(); let borrowed = self.borrowed_block.is_some(); macro_rules! dispatch_dict { - ($kernel:ident) => { - if borrowed { - self.$kernel::(current_abs_start, current_len, handle_sequence) - } else if use_dict { - self.$kernel::(current_abs_start, current_len, handle_sequence) + ($kernel:ident, $dict_kernel:ident) => { + if use_dict { + self.$dict_kernel(current_abs_start, current_len, handle_sequence) + } else if borrowed { + self.$kernel::(current_abs_start, current_len, handle_sequence) } else { - self.$kernel::(current_abs_start, current_len, handle_sequence) + self.$kernel::(current_abs_start, current_len, handle_sequence) } }; } @@ -3319,7 +3531,7 @@ impl DfastMatchGenerator { feature = "kernel-neon" ))] unsafe { - dispatch_dict!(start_matching_fast_loop_neon) + dispatch_dict!(start_matching_fast_loop_neon, start_matching_dict_loop_neon) } #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] { @@ -3330,13 +3542,19 @@ impl DfastMatchGenerator { match self.kernel { #[cfg(feature = "kernel-avx2")] FastpathKernel::Avx2Bmi2 => unsafe { - dispatch_dict!(start_matching_fast_loop_avx2_bmi2) + dispatch_dict!( + start_matching_fast_loop_avx2_bmi2, + start_matching_dict_loop_avx2_bmi2 + ) }, #[cfg(feature = "kernel-sse")] FastpathKernel::Sse2 | FastpathKernel::Sse42 => unsafe { - dispatch_dict!(start_matching_fast_loop_sse2) + dispatch_dict!(start_matching_fast_loop_sse2, start_matching_dict_loop_sse2) }, - FastpathKernel::Scalar => dispatch_dict!(start_matching_fast_loop_scalar), + FastpathKernel::Scalar => dispatch_dict!( + start_matching_fast_loop_scalar, + start_matching_dict_loop_scalar + ), } } #[cfg(all( @@ -3345,7 +3563,10 @@ impl DfastMatchGenerator { feature = "kernel-simd128" ))] unsafe { - dispatch_dict!(start_matching_fast_loop_simd128) + dispatch_dict!( + start_matching_fast_loop_simd128, + start_matching_dict_loop_simd128 + ) } #[cfg(not(any( all( @@ -3362,7 +3583,10 @@ impl DfastMatchGenerator { ) )))] { - dispatch_dict!(start_matching_fast_loop_scalar) + dispatch_dict!( + start_matching_fast_loop_scalar, + start_matching_dict_loop_scalar + ) } } @@ -3372,7 +3596,7 @@ impl DfastMatchGenerator { feature = "kernel-neon" ))] #[target_feature(enable = "neon")] - unsafe fn start_matching_fast_loop_neon( + unsafe fn start_matching_fast_loop_neon( &mut self, current_abs_start: usize, current_len: usize, @@ -3384,7 +3608,6 @@ impl DfastMatchGenerator { current_len, handle_sequence, crate::encoding::fastpath::neon::common_prefix_len_ptr, - USE_DICT, BORROWED ) } @@ -3394,7 +3617,7 @@ impl DfastMatchGenerator { feature = "kernel-sse" ))] #[target_feature(enable = "sse2")] - unsafe fn start_matching_fast_loop_sse2( + unsafe fn start_matching_fast_loop_sse2( &mut self, current_abs_start: usize, current_len: usize, @@ -3406,7 +3629,6 @@ impl DfastMatchGenerator { current_len, handle_sequence, crate::encoding::fastpath::sse2::common_prefix_len_ptr, - USE_DICT, BORROWED ) } @@ -3416,7 +3638,7 @@ impl DfastMatchGenerator { feature = "kernel-avx2" ))] #[target_feature(enable = "avx2,bmi2")] - unsafe fn start_matching_fast_loop_avx2_bmi2( + unsafe fn start_matching_fast_loop_avx2_bmi2( &mut self, current_abs_start: usize, current_len: usize, @@ -3428,7 +3650,6 @@ impl DfastMatchGenerator { current_len, handle_sequence, crate::encoding::fastpath::avx2_bmi2::common_prefix_len_ptr, - USE_DICT, BORROWED ) } @@ -3439,7 +3660,7 @@ impl DfastMatchGenerator { feature = "kernel-simd128" ))] #[target_feature(enable = "simd128")] - unsafe fn start_matching_fast_loop_simd128( + unsafe fn start_matching_fast_loop_simd128( &mut self, current_abs_start: usize, current_len: usize, @@ -3451,7 +3672,6 @@ impl DfastMatchGenerator { current_len, handle_sequence, crate::encoding::fastpath::simd128::common_prefix_len_ptr, - USE_DICT, BORROWED ) } @@ -3469,7 +3689,7 @@ impl DfastMatchGenerator { ) )))] #[allow(unused_unsafe)] - fn start_matching_fast_loop_scalar( + fn start_matching_fast_loop_scalar( &mut self, current_abs_start: usize, current_len: usize, @@ -3481,10 +3701,119 @@ impl DfastMatchGenerator { current_len, handle_sequence, crate::encoding::fastpath::scalar::common_prefix_len_ptr, - USE_DICT, BORROWED ) } + + #[cfg(all( + target_arch = "aarch64", + target_endian = "little", + feature = "kernel-neon" + ))] + #[target_feature(enable = "neon")] + unsafe fn start_matching_dict_loop_neon( + &mut self, + current_abs_start: usize, + current_len: usize, + handle_sequence: &mut impl for<'a> FnMut(Sequence<'a>), + ) { + start_matching_dict_loop_body!( + self, + current_abs_start, + current_len, + handle_sequence, + crate::encoding::fastpath::neon::common_prefix_len_ptr + ) + } + + #[cfg(all( + any(target_arch = "x86", target_arch = "x86_64"), + feature = "kernel-sse" + ))] + #[target_feature(enable = "sse2")] + unsafe fn start_matching_dict_loop_sse2( + &mut self, + current_abs_start: usize, + current_len: usize, + handle_sequence: &mut impl for<'a> FnMut(Sequence<'a>), + ) { + start_matching_dict_loop_body!( + self, + current_abs_start, + current_len, + handle_sequence, + crate::encoding::fastpath::sse2::common_prefix_len_ptr + ) + } + + #[cfg(all( + any(target_arch = "x86", target_arch = "x86_64"), + feature = "kernel-avx2" + ))] + #[target_feature(enable = "avx2,bmi2")] + unsafe fn start_matching_dict_loop_avx2_bmi2( + &mut self, + current_abs_start: usize, + current_len: usize, + handle_sequence: &mut impl for<'a> FnMut(Sequence<'a>), + ) { + start_matching_dict_loop_body!( + self, + current_abs_start, + current_len, + handle_sequence, + crate::encoding::fastpath::avx2_bmi2::common_prefix_len_ptr + ) + } + + #[cfg(all( + target_arch = "wasm32", + target_feature = "simd128", + feature = "kernel-simd128" + ))] + #[target_feature(enable = "simd128")] + unsafe fn start_matching_dict_loop_simd128( + &mut self, + current_abs_start: usize, + current_len: usize, + handle_sequence: &mut impl for<'a> FnMut(Sequence<'a>), + ) { + start_matching_dict_loop_body!( + self, + current_abs_start, + current_len, + handle_sequence, + crate::encoding::fastpath::simd128::common_prefix_len_ptr + ) + } + + #[cfg(not(any( + all( + target_arch = "aarch64", + target_endian = "little", + feature = "kernel-neon" + ), + all( + target_arch = "wasm32", + target_feature = "simd128", + feature = "kernel-simd128" + ) + )))] + #[allow(unused_unsafe)] + fn start_matching_dict_loop_scalar( + &mut self, + current_abs_start: usize, + current_len: usize, + handle_sequence: &mut impl for<'a> FnMut(Sequence<'a>), + ) { + start_matching_dict_loop_body!( + self, + current_abs_start, + current_len, + handle_sequence, + crate::encoding::fastpath::scalar::common_prefix_len_ptr + ) + } } #[cfg(test)] From b0815e5722ffbfb289601bc301a0e94e825e7aa8 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 18:04:54 +0300 Subject: [PATCH 04/15] perf(fast): decide a dictionary candidate on four bytes before counting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The borrowed dictionary kernel called the segmented dict/input match counter for every occupied dictionary slot and only then asked whether what came back reached four. On input the dictionary does not describe, almost none of those candidates are matches, so the counter ran from byte zero for each of them: a fifth of the whole path's time sat in it (`count_forward_dict_2segment`, 21.2% of a level-1 profile on 10 KiB of random input with a trained dictionary). Compare four bytes first, as upstream does (`MEM_read32(dictMatch) == MEM_read32(ip0)` before `ZSTD_count_2segments`, zstd_fast.c:578-584), on both dictionary-candidate arms: the main dict probe and the repcode probe's dict-side candidate. The gate is exactly the condition the caller already tested — a count reaches four only when the first four bytes agree — so nothing that was accepted is rejected now. The counter then RESUMES past those four rather than re-reading them, which is why it takes what the caller established as a parameter. Doing the gate without that resume made the fixtures a dictionary actually describes 2.3% slower, since there the count would have succeeded anyway and the four bytes were simply compared twice. A candidate within three bytes of the dictionary's end has four bytes only by crossing into the input, which upstream reads across for free (one contiguous window) and we cannot (two buffers), so those positions keep the plain counting form. Byte-identical: all 93 scenario/level rows of the dictionary matrix unchanged, total 15,896,895 bytes. Part of #323 --- zstd/src/encoding/simple/fast_kernel/count.rs | 35 +++++++++++-- .../simple/fast_kernel/count/tests.rs | 40 +++++++++++++-- .../src/encoding/simple/fast_kernel/kernel.rs | 50 ++++++++++++++++++- 3 files changed, 116 insertions(+), 9 deletions(-) diff --git a/zstd/src/encoding/simple/fast_kernel/count.rs b/zstd/src/encoding/simple/fast_kernel/count.rs index a35775a40..57c3eda9a 100644 --- a/zstd/src/encoding/simple/fast_kernel/count.rs +++ b/zstd/src/encoding/simple/fast_kernel/count.rs @@ -201,6 +201,7 @@ pub(crate) fn count_forward_dict_2segment( cand: usize, inp: &[u8], cur: usize, + known: usize, ) -> usize { // Release assertions: this is a safe `pub(crate)` fn that does raw pointer // math below. `cand >= dict.len()` would make the dict segment read OOB and @@ -218,9 +219,37 @@ pub(crate) fn count_forward_dict_2segment( ); let dict_len = dict.len(); let inp_len = inp.len(); + // `known` bytes at (`dict[cand..]`, `inp[cur..]`) are already established + // equal by the caller's gate, so the count resumes past them instead of + // re-reading what the gate just compared (upstream counts from `+4` after + // its `MEM_read32` for the same reason). A caller with nothing established + // passes 0. + debug_assert!(cur + known <= inp_len, "known runs past the input"); + if cand + known >= dict_len { + // The established bytes already carried the candidate out of the + // dictionary and into the input that follows it in the logical + // `[dict][input]` window, so what remains is a single input segment. + let cand_in = cand + known - dict_len; + let cur2 = cur + known; + if cur2 >= inp_len { + return known; + } + // SAFETY: `cand_in < inp_len` (the candidate precedes the cursor in the + // window) and `cur2 < inp_len`; `count_forward` stops at `iend`. + return known + + unsafe { + count_forward( + inp.as_ptr().add(cur2), + inp.as_ptr().add(cand_in), + inp.as_ptr().add(inp_len), + ) + }; + } + let cand = cand + known; + let cur = cur + known; let cur_avail = inp_len - cur; if cur_avail == 0 { - return 0; + return known; } // Segment 1: candidate reads `dict[cand..dict_len]`, current reads // `inp[cur..]`. Bounded by whichever side runs out first. @@ -238,7 +267,7 @@ pub(crate) fn count_forward_dict_2segment( // Mismatch inside the dict segment, or the current input is exhausted → // the match ends here. if m1 < seg1 || seg1 == cur_avail { - return m1; + return known + m1; } // The candidate exhausted the dict (`m1 == dict_len - cand`) and the current // input still has bytes left. Segment 2: the candidate logically continues @@ -255,7 +284,7 @@ pub(crate) fn count_forward_dict_2segment( inp.as_ptr().add(inp_len), ) }; - m1 + m2 + known + m1 + m2 } #[cfg(test)] diff --git a/zstd/src/encoding/simple/fast_kernel/count/tests.rs b/zstd/src/encoding/simple/fast_kernel/count/tests.rs index d810615c7..f353ab94f 100644 --- a/zstd/src/encoding/simple/fast_kernel/count/tests.rs +++ b/zstd/src/encoding/simple/fast_kernel/count/tests.rs @@ -107,7 +107,7 @@ fn dict_2segment_within_dict_only() { let inp = [30u8, 40, 99]; // cand=2: dict[2]=30 vs inp[0]=30 ✓; dict[3]=40 vs inp[1]=40 ✓; // cand_idx=4 == dict.len() → inp[0]=30 vs inp[2]=99 ✗ → len 2. - assert_eq!(count_forward_dict_2segment(&dict, 2, &inp, 0), 2); + assert_eq!(count_forward_dict_2segment(&dict, 2, &inp, 0, 0), 2); } #[test] @@ -117,7 +117,7 @@ fn dict_2segment_crosses_boundary_into_input() { let dict = [1u8, 2, 3]; let inp = [1u8, 2, 3, 1, 2, 3, 9]; // cur=3 → [1,2,3,9...] // cand=0: dict[0..3] match inp[3..6]; cand_idx=3 → inp[0]=1 vs inp[6]=9 ✗ → 3. - assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 3), 3); + assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 3, 0), 3); } #[test] @@ -128,12 +128,44 @@ fn dict_2segment_continues_word_at_a_time_past_boundary() { let inp = [1u8, 2, 3, 1, 2, 3, 1, 2]; // cur=3 → [1,2,3,1,2] // seg1: dict[0..3]=[1,2,3] vs inp[3..6]=[1,2,3] → m1=3 (dict exhausted). // seg2: inp[0..]=[1,2,...] vs inp[6..]=[1,2] → m2=2. total 5. - assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 3), 5); + assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 3, 0), 5); } #[test] fn dict_2segment_stops_at_input_end() { let dict = [7u8, 7]; let inp = [7u8, 7, 7, 7]; // cur=2 → only 2 bytes left - assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 2), 2); + assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 2, 0), 2); +} + +/// The caller's gate establishes a prefix, and the count must resume past it +/// and still report the TOTAL length — every case a fresh count would report. +#[test] +fn dict_2segment_resumes_past_an_established_prefix() { + // Same three fixtures as above, each answered identically whether the + // caller established nothing or the four bytes its gate compared. + let dict = [1u8, 2, 3, 4, 5, 6]; + let inp = [1u8, 2, 3, 4, 5, 6, 9]; + for known in [0usize, 4] { + assert_eq!( + count_forward_dict_2segment(&dict, 0, &inp, 0, known), + 6, + "established {known} bytes", + ); + } + + // A prefix that carries the candidate out of the dictionary: the rest of + // the match comes from the input that follows it in the window. + let dict = [1u8, 2, 3, 4]; + let inp = [1u8, 2, 3, 4, 1, 2, 3, 4, 1, 2, 7]; + // cand=0 against cur=4: four dict bytes, then the candidate continues at + // inp[0] against inp[8] — [1,2] more, then 3 vs 7 stops it. + assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 4, 0), 6); + assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 4, 4), 6); + + // And when the established prefix reaches exactly the end of the input, + // there is nothing left to count. + let dict = [5u8, 5, 5, 5]; + let inp = [5u8, 5, 5, 5]; + assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 0, 4), 4); } diff --git a/zstd/src/encoding/simple/fast_kernel/kernel.rs b/zstd/src/encoding/simple/fast_kernel/kernel.rs index a99dbd116..7797cc3e9 100644 --- a/zstd/src/encoding/simple/fast_kernel/kernel.rs +++ b/zstd/src/encoding/simple/fast_kernel/kernel.rs @@ -1424,7 +1424,24 @@ unsafe fn borrowed_candidate_len usize>( ) } } else { - let l = count_forward_dict_2segment(dict, cand_abs, inp, cur_off); + // Same four-byte gate as the input-candidate arm above, and for the + // same reason: the count only ever survives when the first four bytes + // agree, so comparing them first keeps the segmented count off every + // rejected candidate. Only a candidate within three bytes of the + // dictionary's end lacks four bytes to compare on its own side (the + // rest of its match would come from the input, a different buffer + // here); those keep the counting form. + let known = if cand_abs + 4 <= dict_end { + // SAFETY: `cand_abs + 4 <= dict_end == dict.len()`, and the caller + // guarantees `cur_off + 4 <= block_end`. + if unsafe { read32(dict.as_ptr().add(cand_abs)) != read32(inp_base.add(cur_off)) } { + return 0; + } + 4 + } else { + 0 + }; + let l = count_forward_dict_2segment(dict, cand_abs, inp, cur_off, known); if l >= 4 { l } else { 0 } } } @@ -1609,7 +1626,36 @@ fn compress_block_fast_dict_borrowed_impl< if main_idx < prefix_start_index { let dpos = dict_idx as usize; if dict_idx >= 1 && dpos < dict_end && dpos >= window_low { - let m0 = count_forward_dict_2segment(dict, dpos, inp, curr); + // Four bytes decide it before anything counts, the way + // upstream does (`MEM_read32(dictMatch) == MEM_read32(ip0)`, + // zstd_fast.c:578). A tag hit is only a candidate, and on + // input the dictionary does not describe most of them are + // not matches at all: counting first ran the segmented + // count from byte zero for every one of them, and it showed + // — a fifth of this path's time sat in that counter. + // + // The compare needs four bytes on the dictionary side. A + // candidate within three bytes of the dictionary's end has + // them only by crossing into the input, which is a + // different buffer here (upstream's window is contiguous, + // so its `MEM_read32` reads across for free), so those few + // positions keep the counting form. + let m0 = if dpos + 4 <= dict_end { + // SAFETY: `dpos + 4 <= dict_end == dict.len()`, and + // `curr <= ilimit` leaves 8 readable bytes at `curr`. + let (cand4, cur4) = unsafe { + (read32(dict.as_ptr().add(dpos)), read32(inp_base.add(curr))) + }; + if cand4 == cur4 { + // Those four are established, so the count resumes + // past them rather than re-reading them. + count_forward_dict_2segment(dict, dpos, inp, curr, 4) + } else { + 0 + } + } else { + count_forward_dict_2segment(dict, dpos, inp, curr, 0) + }; if m0 >= 4 { let mut match_ip = curr; let mut match_pos = dpos; From 6d0446b0dbe7f556979a7b525455d91661cba481 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 18:35:54 +0300 Subject: [PATCH 05/15] fix(cli): reduce an unnamed ultra level instead of refusing it `zstd -22` warns and compresses at 19 ("Warning : compression level higher than max, reduced to 19", exit 0). We failed the run instead, so a script that works against upstream broke against us. Verified against zstd 1.5.7 on the same host: with --ultra it compresses at 22; without it warns, reduces and exits 0. The benchmark range reduces the same way, since `-b20` reaches an ultra level as surely as `-20` does. Found while evaluating both encoders over a real 32 MiB access log: the sweep's level-22 row came back empty against ours. Part of #128 --- zstd/src/bin/structured-zstd/main.rs | 17 ++++++++++++-- zstd/src/bin/structured-zstd/tests.rs | 34 +++++++++++++++++++-------- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/zstd/src/bin/structured-zstd/main.rs b/zstd/src/bin/structured-zstd/main.rs index 5a1375c15..82164517e 100644 --- a/zstd/src/bin/structured-zstd/main.rs +++ b/zstd/src/bin/structured-zstd/main.rs @@ -66,6 +66,11 @@ macro_rules! info { const ZSTD_SUFFIX: &str = ".zst"; +/// Highest level the CLI compresses at when `--ultra` was not given (upstream +/// `ZSTDCLI_CLEVEL_MAX`). Asking for more without naming `--ultra` reduces to +/// this with a warning rather than failing. +const CLI_MAX_LEVEL_WITHOUT_ULTRA: i32 = 19; + /// Operation selected by mode flags / `argv[0]`. #[derive(Clone, Copy, PartialEq, Eq, Debug)] enum Mode { @@ -781,8 +786,16 @@ fn parse_args( // read every input before the first pass refused it. validate_level(lowest_level)?; validate_level(highest_level)?; - if !ultra && highest_level > 19 { - bail!("level {highest_level} requires --ultra (levels 20-22)"); + // Unnamed, an ultra level is not refused but reduced, with a warning, the + // way upstream reduces it — a script that runs `zstd -22` compresses at 19 + // rather than failing, and refusing here is what would break it. + if !ultra && highest_level > CLI_MAX_LEVEL_WITHOUT_ULTRA { + info!( + "Warning : compression level higher than max, reduced to {CLI_MAX_LEVEL_WITHOUT_ULTRA} " + ); + opts.level = opts.level.min(CLI_MAX_LEVEL_WITHOUT_ULTRA); + opts.bench_start = opts.bench_start.min(CLI_MAX_LEVEL_WITHOUT_ULTRA); + opts.bench_end = opts.bench_end.min(CLI_MAX_LEVEL_WITHOUT_ULTRA); } // Long-distance matching runs on the optimal parser here, so below it the // flag would widen the window and never run the matcher it names. Settled diff --git a/zstd/src/bin/structured-zstd/tests.rs b/zstd/src/bin/structured-zstd/tests.rs index 119eec737..56aa4ce75 100644 --- a/zstd/src/bin/structured-zstd/tests.rs +++ b/zstd/src/bin/structured-zstd/tests.rs @@ -1340,16 +1340,30 @@ fn bare_numeric_flag_is_a_level() { assert_eq!(opts.inputs, vec![PathBuf::from("in.txt")]); } -#[test] -fn levels_above_19_require_ultra() { - assert!(parse(&["-22", "in.txt"]).is_err()); - let opts = parse(&["--ultra", "-22", "in.txt"]).unwrap(); - assert_eq!(opts.level, 22); - // Benchmarking compresses the range `-b`/`-e` name, so that is the range - // the gate has to read: `-b20` runs an ultra level as surely as `-20` does. - assert!(parse(&["-b20", "in.txt"]).is_err()); - assert!(parse(&["-b3", "-e22", "in.txt"]).is_err()); - assert!(parse(&["--ultra", "-b20", "in.txt"]).is_ok()); +/// Levels 20-22 are expensive enough that they have to be asked for by name, +/// but asking without `--ultra` is not an error: upstream warns and compresses +/// at 19 ("Warning : compression level higher than max, reduced to 19", exit +/// 0), so a script that runs `zstd -22` keeps working. Refusing instead breaks +/// it against us. +#[test] +fn levels_above_19_without_ultra_fall_back_to_19() { + assert_eq!(parse(&["-22", "in.txt"]).unwrap().level, 19); + assert_eq!(parse(&["-20", "in.txt"]).unwrap().level, 19); + // Named, they run as asked. + assert_eq!(parse(&["--ultra", "-22", "in.txt"]).unwrap().level, 22); + // Benchmarking compresses the range `-b`/`-e` name rather than the level + // `-N` sets, so the range is what gets clamped: `-b20` reaches an ultra + // level as surely as `-20` does. + let opts = parse(&["-b20", "in.txt"]).unwrap(); + assert_eq!((opts.bench_start, opts.bench_end), (19, 19)); + let opts = parse(&["-b3", "-e22", "in.txt"]).unwrap(); + assert_eq!((opts.bench_start, opts.bench_end), (3, 19)); + assert_eq!( + parse(&["--ultra", "-b20", "in.txt"]).unwrap().bench_start, + 20 + ); + // Below the ultra band nothing moves. + assert_eq!(parse(&["-19", "in.txt"]).unwrap().level, 19); } #[test] From 77079f1b3cb256a227f4804f7b136077d4a48dd9 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 18:49:38 +0300 Subject: [PATCH 06/15] test(encode): cover the dictionary paths the dispatch leaves cold The dictionary scan loop has one monomorph per CPU tier and the runtime dispatch runs exactly one of them, so on any given machine the others never execute. Forcing the cached tier runs each in turn over the same dictionary-primed block and pins what the dispatch assumes: every tier emits the same sequences, so the scalar fallback and the SIMD kernels agree bit for bit. Mirrors the binary-tree tiers' existing test. Also covered, all of them boundaries the changed code introduced: - a Fast dictionary candidate in the last three bytes of the dictionary, which cannot be judged on four bytes of its own and keeps the counting form. The match crosses into the input and must still be found, which is what the four-byte gate must not cost. - the counter resuming past an established prefix that reaches exactly the end of the input, with and without dictionary left of its own. - its two contract assertions: it does raw pointer math from a safe signature, so a caller that broke either bound would read outside the buffers. - a magic-prefixed blob that does not parse, which is a corrupt dictionary and must be refused rather than re-read as raw content. --- zstd/src/encoding/frame_compressor/tests.rs | 20 ++++ zstd/src/encoding/match_generator/tests.rs | 89 ++++++++++++++++++ .../simple/fast_kernel/count/tests.rs | 25 ++++- .../simple/fast_kernel/kernel/tests.rs | 91 +++++++++++++++++++ 4 files changed, 224 insertions(+), 1 deletion(-) diff --git a/zstd/src/encoding/frame_compressor/tests.rs b/zstd/src/encoding/frame_compressor/tests.rs index f886d6092..76cf4b097 100644 --- a/zstd/src/encoding/frame_compressor/tests.rs +++ b/zstd/src/encoding/frame_compressor/tests.rs @@ -1290,6 +1290,26 @@ fn set_dictionary_from_bytes_takes_unmagicked_bytes_as_raw_content() { ); } +/// Taking either kind is not taking anything: a blob that claims to be a +/// serialized dictionary by carrying the magic, and then does not parse, is a +/// corrupt dictionary and must be refused rather than quietly re-read as raw +/// content (upstream classifies on the magic alone and then fails the parse). +#[test] +fn set_dictionary_from_bytes_rejects_a_corrupt_serialized_dictionary() { + let mut corrupt = crate::decoding::DICTIONARY_MAGIC.to_vec(); + corrupt.extend_from_slice(&[0xFF; 60]); + + let mut compressor: FrameCompressor< + &[u8], + Vec, + crate::encoding::match_generator::MatchGeneratorDriver, + > = FrameCompressor::new(super::CompressionLevel::Fastest); + assert!( + compressor.set_dictionary_from_bytes(&corrupt).is_err(), + "a magic-prefixed blob that does not parse is corrupt, not raw content", + ); +} + #[test] fn set_dictionary_rejects_zero_repeat_offsets() { let invalid = crate::decoding::Dictionary { diff --git a/zstd/src/encoding/match_generator/tests.rs b/zstd/src/encoding/match_generator/tests.rs index 9a53e0edf..a22337646 100644 --- a/zstd/src/encoding/match_generator/tests.rs +++ b/zstd/src/encoding/match_generator/tests.rs @@ -776,6 +776,95 @@ fn bt_optimal_all_kernel_tiers_emit_identical_sequences() { } } +/// The dictionary scan loop has one monomorph per CPU tier and the runtime +/// dispatch runs exactly one of them, so the rest never execute on this +/// machine unless a test asks for them. Forcing the cached tier runs each in +/// turn over the same dictionary-primed block and pins what the dispatch +/// assumes: every tier emits the same sequences, so the scalar fallback and +/// the SIMD kernels agree bit for bit. +/// +/// x86-only, as with the binary-tree tiers above: the aarch64 dispatch is +/// unconditional NEON and never reads the cached field. +#[cfg(any(target_arch = "x86", target_arch = "x86_64"))] +#[test] +fn dfast_dictionary_all_kernel_tiers_emit_identical_sequences() { + use crate::encoding::fastpath::FastpathKernel; + + // Only tiers the running CPU may legally execute: each dispatch arm is + // `unsafe` and assumes its target feature is present. + #[allow(unused_mut)] + let mut tiers = alloc::vec![FastpathKernel::Scalar]; + #[cfg(feature = "kernel-sse")] + if std::is_x86_feature_detected!("sse2") { + tiers.push(FastpathKernel::Sse2); + } + #[cfg(feature = "kernel-avx2")] + if std::is_x86_feature_detected!("avx2") && std::is_x86_feature_detected!("bmi2") { + tiers.push(FastpathKernel::Avx2Bmi2); + } + + let dict: Vec = (0..20 * 1024u32) + .map(|i| (i.wrapping_mul(2_654_435_761) >> 13) as u8) + .collect(); + // Dictionary slices for the dictionary probes, a repeat of one of them for + // the repcode path, and fresh bytes in between so the live tables fill and + // the live probes take over — all four commit paths of the loop. + let mut block = dict[1000..1200].to_vec(); + block.extend_from_slice(&(0..300u32).map(|i| (i % 251) as u8).collect::>()); + block.extend_from_slice(&dict[5000..5300]); + block.extend_from_slice(&dict[1000..1200]); + block.extend_from_slice(&(0..400u32).map(|i| (i % 241) as u8).collect::>()); + block.extend_from_slice(&dict[5000..5300]); + + let run = |tier: FastpathKernel| -> Vec<(usize, usize, usize)> { + let mut driver = MatchGeneratorDriver::new(32, 2); + driver.set_source_size_hint(block.len() as u64); + driver.set_dictionary_size_hint(crate::encoding::DictionarySizes::raw_content(dict.len())); + driver.reset(CompressionLevel::Level(3)); + driver.prime_with_dictionary(&dict, [1, 4, 8]); + assert_eq!( + driver.active_backend(), + super::super::strategy::BackendTag::Dfast, + "level 3 with a dictionary must run the dfast backend", + ); + driver.dfast_matcher_mut().kernel = tier; + let mut space = driver.get_next_space(); + space.clear(); + space.extend_from_slice(&block); + driver.commit_space(space); + let mut seqs = Vec::new(); + driver.start_matching(|seq| match seq { + Sequence::Triple { + literals, + offset, + match_len, + } => seqs.push((literals.len(), offset, match_len)), + Sequence::Literals { literals } => seqs.push((literals.len(), 0, 0)), + }); + seqs + }; + + let reference = run(tiers[0]); + // The block is built from dictionary slices, so matches reaching past it + // must exist — otherwise every tier would agree on nothing at all. + let dict_matches = reference + .iter() + .filter(|(_, offset, len)| *offset > block.len() && *len > 0) + .count(); + assert!( + dict_matches >= 2, + "the primed dictionary should be found (got {dict_matches} in {reference:?})", + ); + for &tier in &tiers[1..] { + assert_eq!( + run(tier), + reference, + "kernel tier {tier:?} diverged from {:?} on the dictionary loop", + tiers[0], + ); + } +} + /// Resolving positive levels across the source-size tiers drives every /// strategy arm of the cParams -> `LevelParams` derivation, and each resolved /// strategy must pair with the matching search method (Fast -> Fast, diff --git a/zstd/src/encoding/simple/fast_kernel/count/tests.rs b/zstd/src/encoding/simple/fast_kernel/count/tests.rs index f353ab94f..cef62f7dd 100644 --- a/zstd/src/encoding/simple/fast_kernel/count/tests.rs +++ b/zstd/src/encoding/simple/fast_kernel/count/tests.rs @@ -164,8 +164,31 @@ fn dict_2segment_resumes_past_an_established_prefix() { assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 4, 4), 6); // And when the established prefix reaches exactly the end of the input, - // there is nothing left to count. + // there is nothing left to count — whether or not the candidate still has + // dictionary left of its own. let dict = [5u8, 5, 5, 5]; let inp = [5u8, 5, 5, 5]; assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 0, 4), 4); + let dict = [5u8, 5, 5, 5, 5, 5]; + let inp = [5u8, 5, 5, 5]; + assert_eq!(count_forward_dict_2segment(&dict, 0, &inp, 0, 4), 4); +} + +/// The counter does raw pointer math from a safe signature, so it enforces its +/// contract rather than trusting it. Both bounds are release assertions; a +/// caller that broke one would otherwise read outside the buffers. +#[test] +#[should_panic(expected = "cand")] +fn dict_2segment_rejects_a_candidate_outside_the_dict() { + let dict = [1u8, 2, 3]; + let inp = [1u8, 2, 3]; + let _ = count_forward_dict_2segment(&dict, 3, &inp, 0, 0); +} + +#[test] +#[should_panic(expected = "cur")] +fn dict_2segment_rejects_a_cursor_past_the_input() { + let dict = [1u8, 2, 3]; + let inp = [1u8, 2, 3]; + let _ = count_forward_dict_2segment(&dict, 0, &inp, 4, 0); } diff --git a/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs b/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs index fa0de32e8..42cad2dc3 100644 --- a/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs +++ b/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs @@ -714,3 +714,94 @@ fn borrowed_dict_kernel_reconstructs_via_dual_base() { "expected at least one match reading from the dictionary region (dual-base path)", ); } + +/// A candidate in the last three bytes of the dictionary cannot be judged on +/// four bytes of its own — the fourth would come from the input, a separate +/// buffer here — so it keeps the counting form instead of the four-byte gate. +/// The match is real (it continues into the input across the boundary) and +/// must still be found, which is what the gate must not cost. +#[test] +fn borrowed_dict_kernel_finds_a_match_starting_in_the_dictionary_tail() { + use crate::encoding::fastpath::FastpathKernel; + + let hash_log = 12u32; + const MLS: u32 = 4; + // The dictionary ends with "AB"; the input is "ABAB…", so a candidate at + // `dict.len() - 2` matches two bytes of dictionary and then continues into + // the input itself. + let mut dict: Vec = (0u8..30).collect(); + dict.extend_from_slice(b"AB"); + let dict_end = dict.len(); + let inp: Vec = b"AB".repeat(24); + + let mut main_table = FastHashTable::new(hash_log, MLS); + let mut dict_table = FastHashTable::new(hash_log, MLS); + // Point the slot the input's first key looks up at that tail position. The + // ordinary fill stops `HASH_READ_SIZE` short of the end, so the only way to + // reach the tail case is to seed it — which is exactly the state a + // dictionary whose own tail was hashed elsewhere would leave. + let dpos = dict_end - 2; + // SAFETY: the input has more than 8 readable bytes; MLS matches the table. + let hat = unsafe { hash_ptr_raw::(inp.as_ptr(), hash_log + DICT_TAG_BITS) }; + unsafe { + dict_table.put( + hat >> DICT_TAG_BITS, + ((dpos as u32) << DICT_TAG_BITS) | (hat & DICT_TAG_MASK), + ) + }; + + let mut tuples: Vec<(Vec, usize, usize)> = Vec::new(); + let mut handle = |seq: Sequence<'_>| match seq { + Sequence::Triple { + literals, + offset, + match_len, + } => tuples.push((literals.to_vec(), offset, match_len)), + Sequence::Literals { literals } => tuples.push((literals.to_vec(), 0, 0)), + }; + + let result = compress_block_fast_dict_borrowed::( + &inp, + &dict, + 0, + inp.len(), + &mut main_table, + &dict_table, + PrefixBounds { + prefix_start_index: 1, + window_low: 0, + }, + [0, 0], + 2, + &mut handle, + FastpathKernel::Scalar, + ); + + let mut window = dict.clone(); + let mut saw_dict_tail_match = false; + for (literals, offset, match_len) in &tuples { + window.extend_from_slice(literals); + if *match_len > 0 { + let start = window.len() - offset; + if start >= dpos && start < dict_end { + saw_dict_tail_match = true; + } + for i in 0..*match_len { + let b = window[start + i]; + window.push(b); + } + } + } + let tail_start = inp.len() - result.tail_literals_len; + window.extend_from_slice(&inp[tail_start..]); + + assert_eq!( + &window[dict_end..], + &inp[..], + "a dictionary-tail match must still reconstruct the input exactly", + ); + assert!( + saw_dict_tail_match, + "expected the match to start in the dictionary's last bytes: {tuples:?}", + ); +} From 13076b15e5e608f0c26078f0e64a57dac6e78664 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 18:53:43 +0300 Subject: [PATCH 07/15] test(fast): cover a repcode whose candidate is in the dictionary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repeat offset can point into the dictionary rather than the input — what a dictionary's own repeat offsets are for on the first block — and the probe then reads its candidate from the other buffer, through the four-byte gate the previous commit added there. Empty dictionary table in the fixture, so the emitted sequence can only have come through that arm. --- .../simple/fast_kernel/kernel/tests.rs | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs b/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs index 42cad2dc3..ecac77542 100644 --- a/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs +++ b/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs @@ -715,6 +715,89 @@ fn borrowed_dict_kernel_reconstructs_via_dual_base() { ); } +/// A repeat offset can point into the dictionary rather than the input — that +/// is what a dictionary's own repeat offsets are for on the first block — and +/// then the repcode probe reads its candidate from the other buffer. The +/// four-byte gate has to hold there too: the match is real and must be found. +#[test] +fn borrowed_dict_kernel_takes_a_repcode_whose_candidate_is_in_the_dictionary() { + use crate::encoding::fastpath::FastpathKernel; + + let hash_log = 12u32; + const MLS: u32 = 4; + let dict: Vec = (0u8..40).collect(); + let dict_end = dict.len(); + // The repcode is probed at `curr + 1`, so the byte that lines up with the + // dictionary's start is the input's SECOND one. + let mut inp: Vec = alloc::vec![0xFF]; + inp.extend_from_slice(&dict[0..24]); + inp.extend_from_slice(b"tail"); + + // `rep_abs = (dict_end + curr) + 1 - offset_1`, so this offset puts the + // candidate at dictionary position 0 for the very first probe. + let offset_1 = (dict_end + 1) as u32; + + let mut main_table = FastHashTable::new(hash_log, MLS); + // Empty dictionary table: nothing but the repcode may match, so the + // sequence below can only have come through the repcode's dictionary arm. + let dict_table = FastHashTable::new(hash_log, MLS); + + let mut tuples: Vec<(Vec, usize, usize)> = Vec::new(); + let mut handle = |seq: Sequence<'_>| match seq { + Sequence::Triple { + literals, + offset, + match_len, + } => tuples.push((literals.to_vec(), offset, match_len)), + Sequence::Literals { literals } => tuples.push((literals.to_vec(), 0, 0)), + }; + + let result = compress_block_fast_dict_borrowed::( + &inp, + &dict, + 0, + inp.len(), + &mut main_table, + &dict_table, + PrefixBounds { + prefix_start_index: 1, + window_low: 0, + }, + [offset_1, 0], + 2, + &mut handle, + FastpathKernel::Scalar, + ); + + let mut window = dict.clone(); + let mut saw_dict_repcode = false; + for (literals, offset, match_len) in &tuples { + window.extend_from_slice(literals); + if *match_len > 0 { + let start = window.len() - offset; + if start < dict_end { + saw_dict_repcode = true; + } + for i in 0..*match_len { + let b = window[start + i]; + window.push(b); + } + } + } + let tail_start = inp.len() - result.tail_literals_len; + window.extend_from_slice(&inp[tail_start..]); + + assert_eq!( + &window[dict_end..], + &inp[..], + "a repcode reading from the dictionary must reconstruct the input exactly", + ); + assert!( + saw_dict_repcode, + "expected the repcode to be taken against the dictionary: {tuples:?}", + ); +} + /// A candidate in the last three bytes of the dictionary cannot be judged on /// four bytes of its own — the fourth would come from the input, a separate /// buffer here — so it keeps the counting form instead of the four-byte gate. From bc639a8d84572093d6f80d7ae3f2a66b78d440ee Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 19:00:50 +0300 Subject: [PATCH 08/15] fix(dictionary): take an empty buffer as a dictionary with no content `ZSTD_createDDict(NULL, 0)` builds a usable DDict that references no content (zstd_ddict.c:123-140), and an empty buffer is how `ZSTD_CCtx_loadDictionary` is told there is no dictionary. Loading either kind answered `DictionaryTooSmall` for that input, so a caller handed an empty file got an error where upstream gives them nothing. The constructor that names raw content still refuses it: there the emptiness is the caller asking for a dictionary that cannot exist. Also bounds the counter's established-prefix argument in release, for the same reason the two arguments beside it are bounded there: it feeds a subtraction and a raw-pointer add from a safe signature. --- zstd/src/decoding/dictionary.rs | 15 +++++++++++++ zstd/src/decoding/dictionary/tests.rs | 22 +++++++++++++++++++ zstd/src/encoding/simple/fast_kernel/count.rs | 9 +++++++- 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/zstd/src/decoding/dictionary.rs b/zstd/src/decoding/dictionary.rs index 62a17d7b8..a24092c36 100644 --- a/zstd/src/decoding/dictionary.rs +++ b/zstd/src/decoding/dictionary.rs @@ -121,6 +121,21 @@ impl Dictionary { pub fn from_serialized_or_raw_content(raw: &[u8]) -> Result { if raw.starts_with(&MAGIC_NUM) { Self::decode_dict(raw) + } else if raw.is_empty() { + // A zero-sized buffer is a dictionary with nothing in it rather + // than a malformed one: `ZSTD_createDDict(NULL, 0)` builds a + // usable `DDict` referencing no content, and + // `ZSTD_CCtx_loadDictionary` with an empty buffer is how a caller + // says "no dictionary". [`Self::from_raw_content`] still refuses + // it, because naming raw content and handing over none is the + // caller asking for a dictionary that cannot exist. + Ok(Dictionary { + id: 0, + fse: FSEScratch::new(), + huf: HuffmanScratch::new(), + dict_content: Vec::new(), + offset_hist: [1, 4, 8], + }) } else { Self::from_raw_content(0, raw.to_vec()) } diff --git a/zstd/src/decoding/dictionary/tests.rs b/zstd/src/decoding/dictionary/tests.rs index 24d26a3f2..7fcca2538 100644 --- a/zstd/src/decoding/dictionary/tests.rs +++ b/zstd/src/decoding/dictionary/tests.rs @@ -123,6 +123,28 @@ fn dictionary_handle_takes_serialized_or_raw_content() { assert_eq!(handle.as_dict().dict_content.as_slice(), raw.as_slice()); } +/// A zero-sized buffer is a dictionary with nothing in it, not a malformed +/// one: `ZSTD_createDDict(NULL, 0)` builds a usable `DDict` that references no +/// content (zstd_ddict.c:123-140), and a caller handed an empty file gets the +/// same nothing here rather than an error. +#[test] +fn an_empty_buffer_is_a_dictionary_with_no_content() { + let dict = Dictionary::from_serialized_or_raw_content(&[]) + .expect("an empty buffer is a dictionary with no content"); + assert_eq!(dict.id, 0); + assert!(dict.dict_content.is_empty()); + assert_eq!(dict.offset_hist, [1, 4, 8]); + + let handle = DictionaryHandle::from_serialized_or_raw_content(&[]) + .expect("the handle takes it the same way"); + assert_eq!(handle.id(), 0); + assert!(handle.as_dict().dict_content.is_empty()); + + // The constructor that names raw content still refuses it: there its + // emptiness is the caller asking for a dictionary that cannot exist. + assert!(Dictionary::from_raw_content(1, Vec::new()).is_err()); +} + #[test] fn dictionary_handle_clones_share_inner() { let raw = include_bytes!("../../../dict_tests/dictionary"); diff --git a/zstd/src/encoding/simple/fast_kernel/count.rs b/zstd/src/encoding/simple/fast_kernel/count.rs index 57c3eda9a..e87261ebd 100644 --- a/zstd/src/encoding/simple/fast_kernel/count.rs +++ b/zstd/src/encoding/simple/fast_kernel/count.rs @@ -224,7 +224,14 @@ pub(crate) fn count_forward_dict_2segment( // re-reading what the gate just compared (upstream counts from `+4` after // its `MEM_read32` for the same reason). A caller with nothing established // passes 0. - debug_assert!(cur + known <= inp_len, "known runs past the input"); + // + // Bounded in release for the same reason as the two above: `cur + known` + // feeds a subtraction and a raw-pointer `add`, so a caller that overstated + // what it established would read outside the input. + assert!( + cur + known <= inp_len, + "count_forward_dict_2segment requires cur ({cur}) + known ({known}) <= inp.len() ({inp_len})", + ); if cand + known >= dict_len { // The established bytes already carried the candidate out of the // dictionary and into the input that follows it in the logical From 2e1a532cc198c1cbbc84b5096fb6ba7544595638 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 19:03:18 +0300 Subject: [PATCH 09/15] perf(dfast): index the probed ip+1 position, as upstream does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dictionary loop probes the long table at ip+1 and, unlike upstream, never wrote that position back (`hashLong[hl3] = curr + 1`, zstd_double_fast.c:459). Nothing else covers it: the complementary insertion after a match writes `curr + 2` and the two positions before the match end, so the probed position was searched and then forgotten. Compressed size across the dictionary matrix: 15,896,895 -> 15,896,884, all of it on the one row where we were losing to libzstd — level_3_dfast/small-4k-log-lines 55 -> 44 bytes, which is exactly what libzstd emits there. --- zstd/src/encoding/dfast/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/zstd/src/encoding/dfast/mod.rs b/zstd/src/encoding/dfast/mod.rs index 0642ae421..a59c96f37 100644 --- a/zstd/src/encoding/dfast/mod.rs +++ b/zstd/src/encoding/dfast/mod.rs @@ -3352,6 +3352,13 @@ macro_rules! start_matching_dict_loop_body { // SAFETY: the index is below the long table's length. let idxl1 = unsafe { *long_hash_ptr.add(hl1_idx) }; let packed_next = packed_curr + 1; + // The probed position is indexed as upstream indexes it + // (`hashLong[hl3] = curr + 1`, zstd_double_fast.c:459): + // nothing else writes it, since the complementary + // insertion after a match covers `curr + 2` and the two + // positions before the match end. + // SAFETY: as for the read above. + unsafe { *long_hash_ptr.add(hl1_idx) = packed_next }; let mut live_hit = false; if idxl1 >= min_slot && idxl1 < packed_next { // SAFETY: as for the long slot above. From 2ee35cccf28cbccc2a23f9895640b75da2ef09bf Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 19:07:18 +0300 Subject: [PATCH 10/15] docs(dfast): record what the dictionary loop's shape measured The commit that introduced it reported microseconds, wall clock and instructions; the cycle figures behind it now sit at the code they justify, with the control arm that makes them attributable. --- zstd/src/encoding/dfast/mod.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/zstd/src/encoding/dfast/mod.rs b/zstd/src/encoding/dfast/mod.rs index a59c96f37..30a7a23b2 100644 --- a/zstd/src/encoding/dfast/mod.rs +++ b/zstd/src/encoding/dfast/mod.rs @@ -3039,6 +3039,13 @@ macro_rules! start_matching_fast_loop_body { /// A borrowed window never carries a dictionary (`borrowed_eligible` rejects /// `use_dictionary_state`), so this kernel is owned-coordinates only and needs /// no `BORROWED` axis. +/// +/// What the shape is worth, on 20 000 dictionary frames of a 10 KiB random +/// fixture (i9, three interleaved rounds of two prebuilt binaries, with the +/// same harness minus the dictionary as the control arm): 8168 -> 6561 M +/// cycles, 1.95 -> 1.60 s wall clock, 11,980 -> 9,171 M instructions. The +/// control arm's instruction count is unchanged to the digit, so the delta is +/// this loop's. macro_rules! start_matching_dict_loop_body { ($self:ident, $current_abs_start:ident, $current_len:ident, $handle_sequence:ident, $cpl:path) => {{ debug_assert!($current_len > 0, "dict_loop precondition: $current_len > 0"); From 6f5781b91a81f72329192a50315e79d20944b47d Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 19:47:52 +0300 Subject: [PATCH 11/15] test(dfast): check the NEON dictionary loop against the scalar one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tier-parity test was x86-only, and aarch64 is where it was needed most: the dispatch there resolves NEON at compile time, so the SIMD kernel always wins and nothing ever ran the scalar loop to compare it against — on that target the scalar dictionary wrapper was not even built. It is now built in test builds there, and the dispatcher keeps a `cfg(test)` branch that asks for it, which is the only way the target can run both. The test covers every tier the running CPU may legally execute, on whichever architecture it runs. Proven to bite: with the scalar loop stubbed out to emit nothing, the test fails on aarch64 with an empty sequence list rather than passing. --- zstd/src/encoding/dfast/mod.rs | 49 +++++++++++++++------- zstd/src/encoding/match_generator/tests.rs | 24 ++++++++--- 2 files changed, 54 insertions(+), 19 deletions(-) diff --git a/zstd/src/encoding/dfast/mod.rs b/zstd/src/encoding/dfast/mod.rs index 30a7a23b2..e1c851a73 100644 --- a/zstd/src/encoding/dfast/mod.rs +++ b/zstd/src/encoding/dfast/mod.rs @@ -3544,8 +3544,21 @@ impl DfastMatchGenerator { target_endian = "little", feature = "kernel-neon" ))] - unsafe { - dispatch_dict!(start_matching_fast_loop_neon, start_matching_dict_loop_neon) + { + // NEON is resolved at compile time here, so the cached tier is not + // read and a release build carries no choice at all. Tests ask for + // the scalar dictionary loop through it, which is the only way this + // target can check the two against each other — the branch is + // `cfg(test)` and does not exist in a shipped build. + #[cfg(test)] + if use_dict && self.kernel == crate::encoding::fastpath::FastpathKernel::Scalar { + return self.start_matching_dict_loop_scalar( + current_abs_start, + current_len, + handle_sequence, + ); + } + unsafe { dispatch_dict!(start_matching_fast_loop_neon, start_matching_dict_loop_neon) } } #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] { @@ -3801,18 +3814,26 @@ impl DfastMatchGenerator { ) } - #[cfg(not(any( - all( - target_arch = "aarch64", - target_endian = "little", - feature = "kernel-neon" - ), - all( - target_arch = "wasm32", - target_feature = "simd128", - feature = "kernel-simd128" - ) - )))] + /// Also built in test builds on the targets whose dispatch is + /// unconditional (aarch64+NEON, wasm+simd128), where nothing would + /// otherwise compile it: without it those targets have no second kernel to + /// check the first against, and the scalar/SIMD agreement the dispatch + /// assumes would go untested exactly where the SIMD one always wins. + #[cfg(any( + not(any( + all( + target_arch = "aarch64", + target_endian = "little", + feature = "kernel-neon" + ), + all( + target_arch = "wasm32", + target_feature = "simd128", + feature = "kernel-simd128" + ) + )), + test + ))] #[allow(unused_unsafe)] fn start_matching_dict_loop_scalar( &mut self, diff --git a/zstd/src/encoding/match_generator/tests.rs b/zstd/src/encoding/match_generator/tests.rs index a22337646..466a74eb0 100644 --- a/zstd/src/encoding/match_generator/tests.rs +++ b/zstd/src/encoding/match_generator/tests.rs @@ -783,9 +783,11 @@ fn bt_optimal_all_kernel_tiers_emit_identical_sequences() { /// assumes: every tier emits the same sequences, so the scalar fallback and /// the SIMD kernels agree bit for bit. /// -/// x86-only, as with the binary-tree tiers above: the aarch64 dispatch is -/// unconditional NEON and never reads the cached field. -#[cfg(any(target_arch = "x86", target_arch = "x86_64"))] +/// Runs on every target that has a second kernel to compare against, aarch64 +/// included: there the dispatch resolves NEON at compile time, so the scalar +/// loop is reachable only through the `cfg(test)` branch the dispatcher keeps +/// for exactly this — otherwise the target where the SIMD kernel always wins +/// would be the one target that never checks it. #[test] fn dfast_dictionary_all_kernel_tiers_emit_identical_sequences() { use crate::encoding::fastpath::FastpathKernel; @@ -794,14 +796,26 @@ fn dfast_dictionary_all_kernel_tiers_emit_identical_sequences() { // `unsafe` and assumes its target feature is present. #[allow(unused_mut)] let mut tiers = alloc::vec![FastpathKernel::Scalar]; - #[cfg(feature = "kernel-sse")] + #[cfg(all( + any(target_arch = "x86", target_arch = "x86_64"), + feature = "kernel-sse" + ))] if std::is_x86_feature_detected!("sse2") { tiers.push(FastpathKernel::Sse2); } - #[cfg(feature = "kernel-avx2")] + #[cfg(all( + any(target_arch = "x86", target_arch = "x86_64"), + feature = "kernel-avx2" + ))] if std::is_x86_feature_detected!("avx2") && std::is_x86_feature_detected!("bmi2") { tiers.push(FastpathKernel::Avx2Bmi2); } + #[cfg(all( + target_arch = "aarch64", + target_endian = "little", + feature = "kernel-neon" + ))] + tiers.push(FastpathKernel::Neon); let dict: Vec = (0..20 * 1024u32) .map(|i| (i.wrapping_mul(2_654_435_761) >> 13) as u8) From 01e61bfd372caa5637c5aacf14685f788787d168 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 19:50:18 +0300 Subject: [PATCH 12/15] docs(fast): record what the dictionary gate measured in cycles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The commit that introduced it reported wall clock and instructions for the Fast rows; the cycle figures now sit at the code they justify, with the control arm that makes them attributable — 5387/5463/5373 -> 3935/3939/3943 M on the fixture the gate is for, against 1.4% of drift on a level that cannot run it. --- zstd/src/encoding/simple/fast_kernel/kernel.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/zstd/src/encoding/simple/fast_kernel/kernel.rs b/zstd/src/encoding/simple/fast_kernel/kernel.rs index 7797cc3e9..0d4f8eb4f 100644 --- a/zstd/src/encoding/simple/fast_kernel/kernel.rs +++ b/zstd/src/encoding/simple/fast_kernel/kernel.rs @@ -1634,6 +1634,17 @@ fn compress_block_fast_dict_borrowed_impl< // count from byte zero for every one of them, and it showed // — a fifth of this path's time sat in that counter. // + // What it is worth, on 20 000 dictionary frames of a 10 KiB + // random fixture (i9, three interleaved rounds of two + // prebuilt binaries, with the dfast dictionary loop — which + // cannot execute this code — as the control arm): 5387 / + // 5463 / 5373 -> 3935 / 3939 / 3943 M cycles, a 27% cut + // with no overlap, against 1.4% of drift on the control. + // Structured input moves less (339 / 345 / 351 -> 333 / + // 333 / 327 M) and input the dictionary describes is flat, + // which is the expected shape: there the count would have + // succeeded anyway. + // // The compare needs four bytes on the dictionary side. A // candidate within three bytes of the dictionary's end has // them only by crossing into the input, which is a From 14810b19a6678d79f288867b07e1979906197926 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 20:11:24 +0300 Subject: [PATCH 13/15] fix(fast): bound the established prefix without a sum that can wrap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `cur + known <= inp_len` computes the sum before comparing it, and that sum wraps in a release build: `known = usize::MAX` with any cursor passes a bound it should have failed, after which the pointer arithmetic below leaves the input. `cur <= inp_len` is established directly above, so the same bound expressed as `known <= inp_len - cur` cannot wrap and cannot underflow. Carries the regression test: a one-byte input with `cur = 1` and `known = usize::MAX` must fail the contract, and before the fix it failed the ADD instead — a different panic in a debug build, and none at all in a release one. Also, an empty buffer now clears the encoder's dictionary instead of being reported as one too small to use, on both the one-shot and the streaming setter. That is what the upstream entry point they mirror does (`ZSTD_clearAllDicts` then `return 0`, zstd_compress.c:1293-1295), so a caller could previously neither say "no dictionary" nor undo an earlier one through the setter. The decode side was fixed a commit earlier; the encoder kept its own copy of the branch. And the tier-parity test reaches wasm: simd128 is resolved at compile time there, so without the same `cfg(test)` door the NEON arm has, that target ran its SIMD kernel under every tier name and compared it with itself. (The test does not build for wasm32 today — a dev-dependency fails to compile for that target — but the door and the tier are what make it meaningful when it does.) --- zstd/src/encoding/dfast/mod.rs | 23 +++++++--- zstd/src/encoding/frame_compressor.rs | 7 +++ zstd/src/encoding/frame_compressor/tests.rs | 45 +++++++++++++++++++ zstd/src/encoding/match_generator/tests.rs | 6 +++ zstd/src/encoding/simple/fast_kernel/count.rs | 7 ++- .../simple/fast_kernel/count/tests.rs | 13 ++++++ zstd/src/encoding/streaming_encoder.rs | 13 ++++++ zstd/src/encoding/streaming_encoder/tests.rs | 33 ++++++++++++++ 8 files changed, 141 insertions(+), 6 deletions(-) diff --git a/zstd/src/encoding/dfast/mod.rs b/zstd/src/encoding/dfast/mod.rs index e1c851a73..26bc3dc28 100644 --- a/zstd/src/encoding/dfast/mod.rs +++ b/zstd/src/encoding/dfast/mod.rs @@ -3589,11 +3589,24 @@ impl DfastMatchGenerator { target_feature = "simd128", feature = "kernel-simd128" ))] - unsafe { - dispatch_dict!( - start_matching_fast_loop_simd128, - start_matching_dict_loop_simd128 - ) + { + // Same `cfg(test)` door as the NEON arm: simd128 is resolved at + // compile time here, so without it this target could never run the + // scalar dictionary loop to check the SIMD one against. + #[cfg(test)] + if use_dict && self.kernel == crate::encoding::fastpath::FastpathKernel::Scalar { + return self.start_matching_dict_loop_scalar( + current_abs_start, + current_len, + handle_sequence, + ); + } + unsafe { + dispatch_dict!( + start_matching_fast_loop_simd128, + start_matching_dict_loop_simd128 + ) + } } #[cfg(not(any( all( diff --git a/zstd/src/encoding/frame_compressor.rs b/zstd/src/encoding/frame_compressor.rs index 88cebac1a..1c1918cfd 100644 --- a/zstd/src/encoding/frame_compressor.rs +++ b/zstd/src/encoding/frame_compressor.rs @@ -3126,10 +3126,17 @@ impl FrameCompressor { /// parse. To reject anything but a serialized dictionary, parse with /// [`EncoderDictionary::from_bytes`] (upstream `ZSTD_dct_fullDict`) and /// attach the result. + /// + /// An empty buffer is how the same upstream entry point is told there is + /// no dictionary: it clears whatever was attached and succeeds, returning + /// it, rather than reporting a dictionary too small to use. pub fn set_dictionary_from_bytes( &mut self, raw_dictionary: &[u8], ) -> Result, crate::decoding::errors::DictionaryDecodeError> { + if raw_dictionary.is_empty() { + return Ok(self.clear_dictionary()); + } self.attach_dictionary(EncoderDictionary::from_serialized_or_raw_content( raw_dictionary, )?) diff --git a/zstd/src/encoding/frame_compressor/tests.rs b/zstd/src/encoding/frame_compressor/tests.rs index 76cf4b097..8659cebd4 100644 --- a/zstd/src/encoding/frame_compressor/tests.rs +++ b/zstd/src/encoding/frame_compressor/tests.rs @@ -1290,6 +1290,51 @@ fn set_dictionary_from_bytes_takes_unmagicked_bytes_as_raw_content() { ); } +/// An empty buffer is how `ZSTD_CCtx_loadDictionary` is told there is no +/// dictionary: it clears whatever was attached and succeeds +/// (`ZSTD_clearAllDicts` then `return 0`, zstd_compress.c:1293-1295). Ours +/// refused it, so a caller could neither say "no dictionary" nor undo an +/// earlier one through the setter. +#[test] +fn set_dictionary_from_bytes_with_an_empty_buffer_clears_the_dictionary() { + let raw_dict = b"tenant=demo table=orders op=put value=aaaaabbbbbcccccdddddeeeee\n".repeat(16); + let payload = b"tenant=demo table=orders op=put value=aaaaabbbbbcccccdddddeeeee\n".repeat(4); + + let mut compressor: FrameCompressor<&[u8], Vec> = + FrameCompressor::new(super::CompressionLevel::Default); + compressor + .set_dictionary_from_bytes(&raw_dict) + .expect("the dictionary attaches"); + let cleared = compressor + .set_dictionary_from_bytes(&[]) + .expect("an empty buffer is how a caller says there is no dictionary"); + assert!( + cleared.is_some(), + "clearing hands back the dictionary that was attached", + ); + + // What it compresses is now what a compressor that never saw a dictionary + // compresses — the attach is gone, not merely emptied. + let mut after_clear = Vec::new(); + let mut c = FrameCompressor::new(super::CompressionLevel::Default); + c.set_dictionary_from_bytes(&raw_dict).expect("attach"); + c.set_dictionary_from_bytes(&[]).expect("clear"); + c.set_source(payload.as_slice()); + c.set_drain(&mut after_clear); + c.compress(); + + let mut never_had_one = Vec::new(); + let mut plain = FrameCompressor::new(super::CompressionLevel::Default); + plain.set_source(payload.as_slice()); + plain.set_drain(&mut never_had_one); + plain.compress(); + + assert_eq!( + after_clear, never_had_one, + "a cleared dictionary must leave the frame a plain one", + ); +} + /// Taking either kind is not taking anything: a blob that claims to be a /// serialized dictionary by carrying the magic, and then does not parse, is a /// corrupt dictionary and must be refused rather than quietly re-read as raw diff --git a/zstd/src/encoding/match_generator/tests.rs b/zstd/src/encoding/match_generator/tests.rs index 466a74eb0..3777dfe5f 100644 --- a/zstd/src/encoding/match_generator/tests.rs +++ b/zstd/src/encoding/match_generator/tests.rs @@ -816,6 +816,12 @@ fn dfast_dictionary_all_kernel_tiers_emit_identical_sequences() { feature = "kernel-neon" ))] tiers.push(FastpathKernel::Neon); + #[cfg(all( + target_arch = "wasm32", + target_feature = "simd128", + feature = "kernel-simd128" + ))] + tiers.push(FastpathKernel::Simd128); let dict: Vec = (0..20 * 1024u32) .map(|i| (i.wrapping_mul(2_654_435_761) >> 13) as u8) diff --git a/zstd/src/encoding/simple/fast_kernel/count.rs b/zstd/src/encoding/simple/fast_kernel/count.rs index e87261ebd..2e36cb2ba 100644 --- a/zstd/src/encoding/simple/fast_kernel/count.rs +++ b/zstd/src/encoding/simple/fast_kernel/count.rs @@ -228,8 +228,13 @@ pub(crate) fn count_forward_dict_2segment( // Bounded in release for the same reason as the two above: `cur + known` // feeds a subtraction and a raw-pointer `add`, so a caller that overstated // what it established would read outside the input. + // + // Phrased as a subtraction, not as `cur + known <= inp_len`: that sum wraps + // in a release build, and a wrapped sum passes the bound it should have + // failed. `cur <= inp_len` is established directly above, so the difference + // cannot underflow. assert!( - cur + known <= inp_len, + known <= inp_len - cur, "count_forward_dict_2segment requires cur ({cur}) + known ({known}) <= inp.len() ({inp_len})", ); if cand + known >= dict_len { diff --git a/zstd/src/encoding/simple/fast_kernel/count/tests.rs b/zstd/src/encoding/simple/fast_kernel/count/tests.rs index cef62f7dd..2ee0f42df 100644 --- a/zstd/src/encoding/simple/fast_kernel/count/tests.rs +++ b/zstd/src/encoding/simple/fast_kernel/count/tests.rs @@ -192,3 +192,16 @@ fn dict_2segment_rejects_a_cursor_past_the_input() { let inp = [1u8, 2, 3]; let _ = count_forward_dict_2segment(&dict, 0, &inp, 4, 0); } + +/// The established-prefix bound has to hold without adding to the cursor: +/// `cur + known` wraps in a release build, and a wrapped sum passes a bound it +/// should have failed, after which the pointer arithmetic below leaves the +/// input. Expressed as a subtraction from a length already known to be at +/// least the cursor, there is nothing to wrap. +#[test] +#[should_panic(expected = "count_forward_dict_2segment requires")] +fn dict_2segment_rejects_a_known_prefix_that_would_wrap() { + let dict = [1u8, 2, 3]; + let inp = [1u8]; + let _ = count_forward_dict_2segment(&dict, 0, &inp, 1, usize::MAX); +} diff --git a/zstd/src/encoding/streaming_encoder.rs b/zstd/src/encoding/streaming_encoder.rs index 9f63036c1..e960ff454 100644 --- a/zstd/src/encoding/streaming_encoder.rs +++ b/zstd/src/encoding/streaming_encoder.rs @@ -334,6 +334,19 @@ impl StreamingEncoder { /// bytes explicitly. Must be called before the first /// [`write`](Write::write); repeat offsets must be non-zero. pub fn set_dictionary_from_bytes(&mut self, raw_dictionary: &[u8]) -> Result<(), Error> { + if raw_dictionary.is_empty() { + // An empty buffer is how the same upstream entry point is told + // there is no dictionary: it clears and succeeds. Still refused + // once the frame is open, like any other attach. + self.ensure_open()?; + if self.frame_started { + return Err(invalid_input_error( + "dictionary must be attached before the first write", + )); + } + self.dictionary = None; + return Ok(()); + } let dict = EncoderDictionary::from_serialized_or_raw_content(raw_dictionary) .map_err(|err| invalid_input_error(&alloc::format!("invalid dictionary: {err:?}")))?; self.set_encoder_dictionary(dict) diff --git a/zstd/src/encoding/streaming_encoder/tests.rs b/zstd/src/encoding/streaming_encoder/tests.rs index 356ba921e..d19f7bb1c 100644 --- a/zstd/src/encoding/streaming_encoder/tests.rs +++ b/zstd/src/encoding/streaming_encoder/tests.rs @@ -1161,6 +1161,39 @@ fn raw_dictionary_leaves_the_id_out_of_the_streaming_header() { assert_eq!(decoded, payload); } +/// The same entry point takes an empty buffer as "no dictionary" and clears, +/// rather than reporting one too small to use. +#[test] +fn set_dictionary_from_bytes_with_an_empty_buffer_clears_the_dictionary() { + let content: Vec = b"tenant=demo region=eu table=orders payload=" + .iter() + .copied() + .cycle() + .take(2048) + .collect(); + let mut payload = Vec::new(); + while payload.len() < 8192 { + payload.extend_from_slice(&content); + } + + let mut encoder = StreamingEncoder::new(Vec::new(), CompressionLevel::Default); + encoder + .set_dictionary_from_bytes(&content) + .expect("the dictionary attaches"); + encoder + .set_dictionary_from_bytes(&[]) + .expect("an empty buffer is how a caller says there is no dictionary"); + encoder.write_all(&payload).unwrap(); + let compressed = encoder.finish().unwrap(); + + // Decodes with no dictionary supplied, which it could not do had the + // earlier attach survived. + let mut decoder = StreamingDecoder::new(compressed.as_slice()).unwrap(); + let mut decoded = Vec::new(); + decoder.read_to_end(&mut decoded).unwrap(); + assert_eq!(decoded, payload); +} + /// The streaming setter is the same upstream entry point as the one-shot one /// (`ZSTD_CCtx_loadDictionary` on a streaming context), which loads in /// `ZSTD_dct_auto` mode: bytes without `ZSTD_MAGIC_DICTIONARY` are raw content. From d02ceacccfcbc1bf0d54c4345d1ea487d5ae9263 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 20:17:01 +0300 Subject: [PATCH 14/15] docs(encode): record the dictionary kernels against upstream's counters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The measurements so far compared our own before and after; the numbers now sit beside libzstd's, taken in the same runs on the same fixture with the same counters (a CDict parsed once, then 20 000 frames through a reused context on each side). Level 1 (Fast): 5675/5663/5689 -> 3883/3908/3901 M cycles against its 2105/2103/2108, and 15,936 -> 9,936 M instructions against its 4,996 — the gap goes 2.70x -> 1.85x in cycles. Level 3 (dfast): 8161/8125/8412 -> 6594/6574/6626 M cycles against its 5035/5044/5033, and 11,980 -> 9,292 M instructions against its 9,750 — 1.62x -> 1.31x in cycles, and slightly FEWER instructions than it runs. Also records what the Fast gate emits rather than only how much: frame md5 over three dictionary fixture shapes x five Fast levels x (with dictionary, without) is unchanged, 30 rows. Equal compressed lengths would not have shown that. --- zstd/src/encoding/dfast/mod.rs | 12 +++++++----- zstd/src/encoding/simple/fast_kernel/kernel.rs | 10 +++++++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/zstd/src/encoding/dfast/mod.rs b/zstd/src/encoding/dfast/mod.rs index 26bc3dc28..368b3ed53 100644 --- a/zstd/src/encoding/dfast/mod.rs +++ b/zstd/src/encoding/dfast/mod.rs @@ -3041,11 +3041,13 @@ macro_rules! start_matching_fast_loop_body { /// no `BORROWED` axis. /// /// What the shape is worth, on 20 000 dictionary frames of a 10 KiB random -/// fixture (i9, three interleaved rounds of two prebuilt binaries, with the -/// same harness minus the dictionary as the control arm): 8168 -> 6561 M -/// cycles, 1.95 -> 1.60 s wall clock, 11,980 -> 9,171 M instructions. The -/// control arm's instruction count is unchanged to the digit, so the delta is -/// this loop's. +/// fixture (i9, three interleaved rounds of prebuilt binaries, with the same +/// harness minus the dictionary as the control arm): 8161 / 8125 / 8412 -> +/// 6594 / 6574 / 6626 M cycles and 11,980 -> 9,292 M instructions, against +/// libzstd's 5035 / 5044 / 5033 M and 9,750 M measured in the same runs. The +/// gap to it goes 1.62x -> 1.31x in cycles, and in instructions we now run +/// slightly FEWER than it does — what is left there is execution density, not +/// work. macro_rules! start_matching_dict_loop_body { ($self:ident, $current_abs_start:ident, $current_len:ident, $handle_sequence:ident, $cpl:path) => {{ debug_assert!($current_len > 0, "dict_loop precondition: $current_len > 0"); diff --git a/zstd/src/encoding/simple/fast_kernel/kernel.rs b/zstd/src/encoding/simple/fast_kernel/kernel.rs index 0d4f8eb4f..eaf218599 100644 --- a/zstd/src/encoding/simple/fast_kernel/kernel.rs +++ b/zstd/src/encoding/simple/fast_kernel/kernel.rs @@ -1643,7 +1643,15 @@ fn compress_block_fast_dict_borrowed_impl< // Structured input moves less (339 / 345 / 351 -> 333 / // 333 / 327 M) and input the dictionary describes is flat, // which is the expected shape: there the count would have - // succeeded anyway. + // succeeded anyway. Against libzstd on the same fixture + // and the same counters, the level-1 gap went 2.70x -> 1.85x + // in cycles and 3.19x -> 1.99x in instructions. + // + // The gate emits the same bytes it did without it: frame + // md5 is unchanged over three dictionary fixture shapes x + // five Fast levels x (with dictionary, without), 30 rows. + // Equal compressed LENGTHS would not have shown that — two + // different parses can weigh the same. // // The compare needs four bytes on the dictionary side. A // candidate within three bytes of the dictionary's end has From 6dedb5fd26a99a29e614479c5fd0f2e96870019c Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 7 Sep 2026 20:55:06 +0300 Subject: [PATCH 15/15] fix(stream): give back the dictionary's entropy tables when clearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clearing the stream dictionary dropped the dictionary and kept the tables built from it: the Huffman and FSE allocations stayed alive, and in `heap_size`, for as long as the encoder lived, though nothing could reach them again. Measured by the regression test that carries this: a serialized dictionary left 37,912 bytes behind a clear that should have returned to zero. The match-finder snapshot needs no matching call, and the code says why: a dictionary is primed at the first write, which is also the point after which this setter refuses to run, so there is never one to drop. Also exercises the paths this round's code introduced: - clearing after the frame is open, and on a stream whose write failed — the two refusals a clear inherits from the attach it is. - a repcode candidate in the dictionary's last three bytes whose match falls short of four bytes: no four bytes of its own to be gated on, so it is counted, and the count rejects it. Both arms of that decision are now exercised. The three lines left in the dfast loop are the `DFTRACE` diagnostic, gated on an environment variable read into a process-wide latch: a test that enabled it would make every other test in the binary write to stderr, so it stays off deliberately. --- .../simple/fast_kernel/kernel/tests.rs | 58 +++++++++++++++++ zstd/src/encoding/streaming_encoder.rs | 8 +++ zstd/src/encoding/streaming_encoder/tests.rs | 65 +++++++++++++++++++ 3 files changed, 131 insertions(+) diff --git a/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs b/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs index ecac77542..f3416c391 100644 --- a/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs +++ b/zstd/src/encoding/simple/fast_kernel/kernel/tests.rs @@ -798,6 +798,64 @@ fn borrowed_dict_kernel_takes_a_repcode_whose_candidate_is_in_the_dictionary() { ); } +/// The other side of the dictionary-tail case: a repcode candidate there has +/// no four bytes of its own to be judged on, so it is counted instead — and +/// when that count falls short of four the candidate is rejected, exactly as a +/// failed four-byte gate would have rejected it. +#[test] +fn borrowed_dict_kernel_rejects_a_short_repcode_in_the_dictionary_tail() { + use crate::encoding::fastpath::FastpathKernel; + + let hash_log = 12u32; + const MLS: u32 = 4; + let mut dict: Vec = (0u8..30).collect(); + dict.extend_from_slice(b"AB"); + // The probe is at `curr + 1`, and `rep_abs = dict_end + curr + 1 - + // offset_1`, so offset 3 puts it two bytes from the dictionary's end. + // `A` lines up, `Z` does not: one byte, short of the four a match needs. + let inp: Vec = b"\xffAZ-then-plain-literals-with-nothing-to-match".to_vec(); + + let mut main_table = FastHashTable::new(hash_log, MLS); + let dict_table = FastHashTable::new(hash_log, MLS); + + let mut tuples: Vec<(Vec, usize, usize)> = Vec::new(); + let mut handle = |seq: Sequence<'_>| match seq { + Sequence::Triple { + literals, + offset, + match_len, + } => tuples.push((literals.to_vec(), offset, match_len)), + Sequence::Literals { literals } => tuples.push((literals.to_vec(), 0, 0)), + }; + + let result = compress_block_fast_dict_borrowed::( + &inp, + &dict, + 0, + inp.len(), + &mut main_table, + &dict_table, + PrefixBounds { + prefix_start_index: 1, + window_low: 0, + }, + [3, 0], + 2, + &mut handle, + FastpathKernel::Scalar, + ); + + assert!( + tuples.iter().all(|(_, _, m)| *m == 0), + "a one-byte dictionary-tail candidate is not a match: {tuples:?}", + ); + assert_eq!( + result.tail_literals_len, + inp.len(), + "with nothing matched the whole block is literals", + ); +} + /// A candidate in the last three bytes of the dictionary cannot be judged on /// four bytes of its own — the fourth would come from the input, a separate /// buffer here — so it keeps the counting form instead of the four-byte gate. diff --git a/zstd/src/encoding/streaming_encoder.rs b/zstd/src/encoding/streaming_encoder.rs index e960ff454..474a1bc70 100644 --- a/zstd/src/encoding/streaming_encoder.rs +++ b/zstd/src/encoding/streaming_encoder.rs @@ -344,6 +344,14 @@ impl StreamingEncoder { "dictionary must be attached before the first write", )); } + // The entropy tables were built at attach time and go with it: + // holding them past the clear keeps Huffman and FSE allocations + // the encoder can no longer reach, for as long as it lives, and + // reports them in `heap_size`. The primed match-finder snapshot + // needs no such call — priming happens at the first write, which + // is also the point after which this setter refuses to run, so + // there is never one to drop here. + self.dictionary_entropy_cache = None; self.dictionary = None; return Ok(()); } diff --git a/zstd/src/encoding/streaming_encoder/tests.rs b/zstd/src/encoding/streaming_encoder/tests.rs index d19f7bb1c..2064533f3 100644 --- a/zstd/src/encoding/streaming_encoder/tests.rs +++ b/zstd/src/encoding/streaming_encoder/tests.rs @@ -1194,6 +1194,71 @@ fn set_dictionary_from_bytes_with_an_empty_buffer_clears_the_dictionary() { assert_eq!(decoded, payload); } +/// Clearing is an attach like any other, and both of the attach's refusals +/// hold for it: a frame already open has its dictionary decided, and a stream +/// that failed a write answers with the failure it kept rather than pretending +/// the change took. +#[test] +fn clearing_the_stream_dictionary_is_refused_where_attaching_is() { + let content: Vec = b"tenant=demo region=eu table=orders payload=" + .iter() + .copied() + .cycle() + .take(1024) + .collect(); + + // Once the frame is open. + let mut enc = StreamingEncoder::new(Vec::new(), CompressionLevel::Default); + enc.set_dictionary_from_bytes(&content).expect("attach"); + enc.write_all(b"the frame starts here").unwrap(); + let err = enc + .set_dictionary_from_bytes(&[]) + .expect_err("the frame's dictionary is already decided"); + assert!( + alloc::format!("{err:?}").contains("before the first write"), + "unexpected error: {err:?}", + ); + + // On a stream whose write failed, the kept failure comes back instead. + let mut enc = StreamingEncoder::new(FailingWriteOnce::new(1), CompressionLevel::Fastest); + let big = vec![b'x'; 512 * 1024]; + let _ = enc.write_all(&big); + let _ = enc.flush(); + assert!( + enc.set_dictionary_from_bytes(&[]).is_err(), + "a poisoned stream answers with its failure, not with success", + ); +} + +/// Clearing has to give back what the attach took. The dictionary's entropy +/// tables are built at attach time and reported by `heap_size`; dropping only +/// the dictionary would leave the encoder holding Huffman and FSE allocations +/// it can no longer reach, for as long as it lives. +#[test] +fn clearing_the_stream_dictionary_gives_back_what_it_allocated() { + // A SERIALIZED dictionary, not raw content: the entropy tables are what + // the attach allocates, and raw content has none — with it the cache is + // empty and this test could not tell a leak from a clean clear. + let content = include_bytes!("../../../dict_tests/dictionary").to_vec(); + + let mut enc = StreamingEncoder::new(Vec::new(), CompressionLevel::Default); + let empty = enc.heap_size(); + enc.set_dictionary_from_bytes(&content) + .expect("the dictionary attaches"); + let attached = enc.heap_size(); + assert!( + attached > empty, + "attaching should have allocated something to give back: {empty} -> {attached}", + ); + + enc.set_dictionary_from_bytes(&[]).expect("clear"); + assert_eq!( + enc.heap_size(), + empty, + "a cleared dictionary must leave the encoder holding no more than it did before", + ); +} + /// The streaming setter is the same upstream entry point as the one-shot one /// (`ZSTD_CCtx_loadDictionary` on a streaming context), which loads in /// `ZSTD_dct_auto` mode: bytes without `ZSTD_MAGIC_DICTIONARY` are raw content.