From fa4d8842a52a940a195557c2220c45f81bd9640f Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 26 Oct 2026 10:53:15 +0200 Subject: [PATCH 1/8] test(bench): time the LDM matrix cells in the encode loops `encode_loop_oneshot_z000033` and `ffi_encode_loop_z000033` take an optional `ldm` argument that times what the benchmark's `level__*_ldm` cells time: a fresh context per frame, long-distance matching, content checksum and content size on. --- zstd/examples/encode_loop_oneshot_z000033.rs | 42 +++++++++++++- zstd/examples/ffi_encode_loop_z000033.rs | 60 +++++++++++++++++++- 2 files changed, 99 insertions(+), 3 deletions(-) diff --git a/zstd/examples/encode_loop_oneshot_z000033.rs b/zstd/examples/encode_loop_oneshot_z000033.rs index 91bdca8dd..47613d581 100644 --- a/zstd/examples/encode_loop_oneshot_z000033.rs +++ b/zstd/examples/encode_loop_oneshot_z000033.rs @@ -11,14 +11,21 @@ //! Output buffer allocated once and reused, as in the sibling binaries. //! //! Build: `cargo build --release -p ffi-bench --example encode_loop_oneshot_z000033 --features dict-builder` -//! Run: `perf stat -e cycles,instructions ./encode_loop_oneshot_z000033 ` +//! Run: `perf stat -e cycles,instructions ./encode_loop_oneshot_z000033 [ldm]` +//! +//! With `ldm` it times what the benchmark's `level__*_ldm` cells time: a +//! fresh compressor per frame, long-distance matching on, content checksum +//! on; `ffi_encode_loop_z000033 ... ldm` is the C side of the same cell. use std::env; use std::fs; -use structured_zstd::encoding::{CompressionLevel, FrameCompressor}; +use structured_zstd::encoding::{CompressionLevel, CompressionParameters, FrameCompressor}; fn main() { + if env::args().nth(4).as_deref() == Some("ldm") { + return ldm_loop(); + } let level: i32 = env::args().nth(1).and_then(|s| s.parse().ok()).unwrap_or(3); let iters: usize = env::args() .nth(2) @@ -46,3 +53,34 @@ fn main() { sum ); } + +fn ldm_loop() { + let level: i32 = env::args().nth(1).and_then(|s| s.parse().ok()).unwrap_or(1); + let iters: usize = env::args() + .nth(2) + .and_then(|s| s.parse().ok()) + .unwrap_or(40); + let corpus_path = env::args().nth(3).expect("corpus path"); + let bytes = fs::read(&corpus_path).expect("read corpus"); + // `CompressionLevel::Level`, as the benchmark's level inventory uses. + let params = CompressionParameters::builder(CompressionLevel::Level(level)) + .enable_long_distance_matching(true) + .build() + .expect("LDM-only parameters"); + let mut out: Vec = Vec::new(); + let mut sum = 0usize; + for _ in 0..iters { + let mut enc: FrameCompressor = FrameCompressor::new(CompressionLevel::Level(level)); + enc.set_parameters(¶ms); + enc.set_content_checksum(true); + enc.compress_independent_frame_into(&bytes[..], &mut out); + sum += out.len(); + } + println!( + "encoded {} bytes x {} iters at level {} with ldm; last-out-sum={}", + bytes.len(), + iters, + level, + sum + ); +} diff --git a/zstd/examples/ffi_encode_loop_z000033.rs b/zstd/examples/ffi_encode_loop_z000033.rs index d04ee8577..dc49083ff 100644 --- a/zstd/examples/ffi_encode_loop_z000033.rs +++ b/zstd/examples/ffi_encode_loop_z000033.rs @@ -9,7 +9,11 @@ //! overhead. //! //! Build: `cargo build --release -p ffi-bench --example ffi_encode_loop_z000033` -//! Run: `perf stat -e cycles,instructions ./ffi_encode_loop_z000033 ` +//! Run: `perf stat -e cycles,instructions ./ffi_encode_loop_z000033 [ldm]` +//! +//! With `ldm` it times the benchmark's `level__*_ldm` C arm: a fresh +//! context per frame through `ZSTD_compress2`, long-distance matching on, +//! content checksum and content size on. use std::env; use std::fs; @@ -17,6 +21,9 @@ use std::fs; use zstd::zstd_safe::zstd_sys; fn main() { + if env::args().nth(4).as_deref() == Some("ldm") { + return ldm_loop(); + } let level: i32 = env::args().nth(1).and_then(|s| s.parse().ok()).unwrap_or(3); let iters: usize = env::args() .nth(2) @@ -57,3 +64,54 @@ fn main() { sum ); } + +fn ldm_loop() { + use zstd_sys::ZSTD_cParameter as P; + let level: i32 = env::args().nth(1).and_then(|s| s.parse().ok()).unwrap_or(1); + let iters: usize = env::args() + .nth(2) + .and_then(|s| s.parse().ok()) + .unwrap_or(40); + let corpus_path = env::args().nth(3).expect("corpus path"); + let bytes = fs::read(&corpus_path).expect("read corpus"); + let bound = unsafe { zstd_sys::ZSTD_compressBound(bytes.len()) }; + let mut out = vec![0u8; bound]; + let mut sum = 0usize; + for _ in 0..iters { + // SAFETY: every call operates on the context created here, freed at + // the end of the iteration; errors are asserted. + unsafe { + let cctx = zstd_sys::ZSTD_createCCtx(); + assert!(!cctx.is_null(), "ZSTD_createCCtx returned null"); + for (param, value) in [ + (P::ZSTD_c_compressionLevel, level), + (P::ZSTD_c_checksumFlag, 1), + (P::ZSTD_c_contentSizeFlag, 1), + (P::ZSTD_c_enableLongDistanceMatching, 1), + ] { + let rc = zstd_sys::ZSTD_CCtx_setParameter(cctx, param, value); + assert!(zstd_sys::ZSTD_isError(rc) == 0, "setParameter failed"); + } + let written = zstd_sys::ZSTD_compress2( + cctx, + out.as_mut_ptr().cast(), + out.len(), + bytes.as_ptr().cast(), + bytes.len(), + ); + assert!( + zstd_sys::ZSTD_isError(written) == 0, + "ZSTD_compress2 failed" + ); + sum += written; + zstd_sys::ZSTD_freeCCtx(cctx); + } + } + println!( + "encoded {} bytes x {} iters at level {} with ldm; last-out-sum={}", + bytes.len(), + iters, + level, + sum + ); +} From 83f5ac43475294c452098fcf85225ae8f81c08e9 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 26 Oct 2026 11:18:00 +0200 Subject: [PATCH 2/8] perf(ldm): one cycle per byte on the gear hash chain The long-distance matcher's rolling hash, `(hash << 1) + GEAR_TAB[b]`, is one serial chain through every byte of the input. Written plainly, x86_64 codegen folds the table load into an `add` behind `add hash, hash`, two cycles a byte on that chain; upstream's compiled `ZSTD_ldm_gear_feed` takes one `lea`. The step is now that `lea` on x86_64; elsewhere the shift already folds into the add. --- zstd/src/encoding/ldm/gear_hash.rs | 49 ++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/zstd/src/encoding/ldm/gear_hash.rs b/zstd/src/encoding/ldm/gear_hash.rs index 98c0fa19f..7ee6a106a 100644 --- a/zstd/src/encoding/ldm/gear_hash.rs +++ b/zstd/src/encoding/ldm/gear_hash.rs @@ -128,6 +128,35 @@ impl GearHashState { } } +/// One byte through the rolling hash: `(hash << 1) + GEAR_TAB[byte]`. +#[inline(always)] +fn gear_step(hash: u64, byte: u8) -> u64 { + let entry = GEAR_TAB[byte as usize]; + #[cfg(target_arch = "x86_64")] + { + // One `lea`, a single cycle on the hash's dependency chain, as + // upstream's compiled `ZSTD_ldm_gear_feed` has it. Written plainly, + // the table load folds into an `add` behind `add hash, hash`: two + // cycles a byte on the chain the whole scan waits on. + let mut hash = hash; + // SAFETY: register arithmetic only; no memory, stack or flags. + unsafe { + core::arch::asm!( + "lea {h}, [{e} + {h}*2]", + h = inout(reg) hash, + e = in(reg) entry, + options(pure, nomem, nostack, preserves_flags), + ); + } + hash + } + // Elsewhere the shift folds into the add (aarch64 `add x, t, h, lsl #1`). + #[cfg(not(target_arch = "x86_64"))] + { + (hash << 1).wrapping_add(entry) + } +} + /// Feed `min_match_length` bytes through the rolling hash WITHOUT /// emitting any splits. /// @@ -148,14 +177,14 @@ pub(crate) fn reset(state: &mut GearHashState, data: &[u8]) { // arithmetic monomorphises to the same instruction sequence on // every backend. while n + 3 < data.len() { - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n] as usize]); - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n + 1] as usize]); - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n + 2] as usize]); - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n + 3] as usize]); + hash = gear_step(hash, data[n]); + hash = gear_step(hash, data[n + 1]); + hash = gear_step(hash, data[n + 2]); + hash = gear_step(hash, data[n + 3]); n += 4; } while n < data.len() { - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n] as usize]); + hash = gear_step(hash, data[n]); n += 1; } state.rolling = hash; @@ -198,7 +227,7 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) // matches `splits[*numSplits] = n` in upstream zstd (n already // incremented). while n + 3 < data.len() { - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n] as usize]); + hash = gear_step(hash, data[n]); n += 1; if hash & mask == 0 { splits[num_splits] = n; @@ -208,7 +237,7 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) return (n, num_splits); } } - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n] as usize]); + hash = gear_step(hash, data[n]); n += 1; if hash & mask == 0 { splits[num_splits] = n; @@ -218,7 +247,7 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) return (n, num_splits); } } - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n] as usize]); + hash = gear_step(hash, data[n]); n += 1; if hash & mask == 0 { splits[num_splits] = n; @@ -228,7 +257,7 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) return (n, num_splits); } } - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n] as usize]); + hash = gear_step(hash, data[n]); n += 1; if hash & mask == 0 { splits[num_splits] = n; @@ -241,7 +270,7 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) } // Tail loop for the remaining 0..3 bytes. Upstream zstd `zstd_ldm.c:124-126`. while n < data.len() { - hash = (hash << 1).wrapping_add(GEAR_TAB[data[n] as usize]); + hash = gear_step(hash, data[n]); n += 1; if hash & mask == 0 { splits[num_splits] = n; From c64b254f3d51a5bd05a139b152a0ec078b92ec99 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 26 Oct 2026 11:43:33 +0200 Subject: [PATCH 3/8] perf(ldm): walk the gear feed in four-byte chunks Each chunk's length is known, so its loads carry no bounds check, and the loop's only counter is the chunk pointer, as upstream's is its index. Split positions are derived from the pointer on the rare split only. --- zstd/src/encoding/ldm/gear_hash.rs | 93 +++++++++++------------------- 1 file changed, 34 insertions(+), 59 deletions(-) diff --git a/zstd/src/encoding/ldm/gear_hash.rs b/zstd/src/encoding/ldm/gear_hash.rs index 7ee6a106a..f4cb20ad8 100644 --- a/zstd/src/encoding/ldm/gear_hash.rs +++ b/zstd/src/encoding/ldm/gear_hash.rs @@ -218,71 +218,46 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) ); let mut hash = state.rolling; let mask = state.stop_mask; - let mut n: usize = 0; let mut num_splits: usize = 0; - // 4-wide unrolled head. Upstream zstd `zstd_ldm.c:118-123`. Each iteration - // increments `n` first, then tests the post-update hash so the - // split index points to the byte AFTER the matched window — this - // matches `splits[*numSplits] = n` in upstream zstd (n already - // incremented). - while n + 3 < data.len() { - hash = gear_step(hash, data[n]); - n += 1; - if hash & mask == 0 { - splits[num_splits] = n; - num_splits += 1; - if num_splits == LDM_BATCH_SIZE { - state.rolling = hash; - return (n, num_splits); - } - } - hash = gear_step(hash, data[n]); - n += 1; - if hash & mask == 0 { - splits[num_splits] = n; - num_splits += 1; - if num_splits == LDM_BATCH_SIZE { - state.rolling = hash; - return (n, num_splits); - } - } - hash = gear_step(hash, data[n]); - n += 1; - if hash & mask == 0 { - splits[num_splits] = n; - num_splits += 1; - if num_splits == LDM_BATCH_SIZE { - state.rolling = hash; - return (n, num_splits); + // One byte at `$at`: the split index is the count of bytes consumed, + // `$at + 1`, the byte AFTER the matched window, as upstream's + // `splits[*numSplits] = n` with `n` already incremented. Positions are + // derived only here, on the rare split, so the scan carries a pointer + // and nothing else. + macro_rules! step { + ($byte:expr, $at:expr) => {{ + hash = gear_step(hash, $byte); + if hash & mask == 0 { + let consumed = $at + 1; + splits[num_splits] = consumed; + num_splits += 1; + if num_splits == LDM_BATCH_SIZE { + state.rolling = hash; + return (consumed, num_splits); + } } - } - hash = gear_step(hash, data[n]); - n += 1; - if hash & mask == 0 { - splits[num_splits] = n; - num_splits += 1; - if num_splits == LDM_BATCH_SIZE { - state.rolling = hash; - return (n, num_splits); - } - } + }}; } - // Tail loop for the remaining 0..3 bytes. Upstream zstd `zstd_ldm.c:124-126`. - while n < data.len() { - hash = gear_step(hash, data[n]); - n += 1; - if hash & mask == 0 { - splits[num_splits] = n; - num_splits += 1; - if num_splits == LDM_BATCH_SIZE { - state.rolling = hash; - return (n, num_splits); - } - } + // Four bytes a round, upstream zstd `zstd_ldm.c:118-123`; a chunk's length + // is known, so its loads carry no bounds check, and the loop's only + // counter is the chunk pointer, as upstream's is its index. + let base = data.as_ptr() as usize; + let mut quads = data.chunks_exact(4); + for quad in &mut quads { + let at = quad.as_ptr() as usize - base; + step!(quad[0], at); + step!(quad[1], at + 1); + step!(quad[2], at + 2); + step!(quad[3], at + 3); + } + // The remaining 0..3 bytes, upstream zstd `zstd_ldm.c:124-126`. + let tail_at = data.len() - quads.remainder().len(); + for (i, &byte) in quads.remainder().iter().enumerate() { + step!(byte, tail_at + i); } state.rolling = hash; - (n, num_splits) + (data.len(), num_splits) } /// 256-entry random-permutation table from upstream zstd From e709d7c671fe17dca72af454c05feb536d03101f Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 26 Oct 2026 11:53:52 +0200 Subject: [PATCH 4/8] perf(ldm): take the gear feed's chunks as arrays `as_chunks::<4>` instead of `chunks_exact(4)`: the chunk is a `[u8; 4]` by type. Measured with `chunks_exact` on runner4 against the lea step, 3 interleaved rounds, frames identical: L1 ldm low1m cycles -3.5%, high1m -2%, no overlap; instructions equal. --- zstd/src/encoding/ldm/gear_hash.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zstd/src/encoding/ldm/gear_hash.rs b/zstd/src/encoding/ldm/gear_hash.rs index f4cb20ad8..6567cc165 100644 --- a/zstd/src/encoding/ldm/gear_hash.rs +++ b/zstd/src/encoding/ldm/gear_hash.rs @@ -243,8 +243,8 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) // is known, so its loads carry no bounds check, and the loop's only // counter is the chunk pointer, as upstream's is its index. let base = data.as_ptr() as usize; - let mut quads = data.chunks_exact(4); - for quad in &mut quads { + let (quads, rest) = data.as_chunks::<4>(); + for quad in quads { let at = quad.as_ptr() as usize - base; step!(quad[0], at); step!(quad[1], at + 1); @@ -252,8 +252,8 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) step!(quad[3], at + 3); } // The remaining 0..3 bytes, upstream zstd `zstd_ldm.c:124-126`. - let tail_at = data.len() - quads.remainder().len(); - for (i, &byte) in quads.remainder().iter().enumerate() { + let tail_at = data.len() - rest.len(); + for (i, &byte) in rest.iter().enumerate() { step!(byte, tail_at + i); } state.rolling = hash; From 7bc77d2cc62eacf7041126d4e00206447b0d8103 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 26 Oct 2026 11:54:54 +0200 Subject: [PATCH 5/8] test(bench): decode C-encoded LDM frames in the decode loop `decode_loop_z000033` takes comma-separated encode options; `ldm` turns on long-distance matching, the frames the benchmark's `level__*_ldm` decode cells read. --- zstd/examples/decode_loop_z000033.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/zstd/examples/decode_loop_z000033.rs b/zstd/examples/decode_loop_z000033.rs index 73e4d6505..04b46559c 100644 --- a/zstd/examples/decode_loop_z000033.rs +++ b/zstd/examples/decode_loop_z000033.rs @@ -22,10 +22,13 @@ fn main() { let iters: u32 = args.get(2).and_then(|s| s.parse().ok()).unwrap_or(50_000); let mode: &str = args.get(3).map(|s| s.as_str()).unwrap_or("rust"); let corpus_path: Option<&str> = args.get(4).map(|s| s.as_str()); - // 6th arg: "checksum" → encode with content_checksum_flag = 1 so the - // decode loop exercises the post-decode XXH64 verify pass. Lets us - // isolate the checksum cost (flag-on vs flag-off) in one harness. - let checksum: bool = args.get(5).map(|s| s == "checksum").unwrap_or(false); + // 6th arg, comma-separated encode options: "checksum" sets + // content_checksum_flag = 1 so the decode loop exercises the post-decode + // XXH64 verify pass; "ldm" turns on long-distance matching, the frames the + // benchmark's `level__*_ldm` decode cells read. + let options: Vec<&str> = args.get(5).map_or(Vec::new(), |s| s.split(',').collect()); + let checksum = options.contains(&"checksum"); + let ldm = options.contains(&"ldm"); // Source resolution. This example is named after the `z000033` corpus, // so an explicit path or the in-tree corpus is the contract. The LCG @@ -94,8 +97,8 @@ fn main() { // FFI encode once at requested level. let dst_cap = unsafe { zstd_sys::ZSTD_compressBound(src.len()) }; let mut compressed: Vec = vec![0u8; dst_cap]; - let written = if checksum { - // ZSTD_compress2 honours CCtx params, including the checksum flag. + let written = if checksum || ldm { + // ZSTD_compress2 honours CCtx params, the checksum and LDM flags. let cctx = unsafe { zstd_sys::ZSTD_createCCtx() }; assert!(!cctx.is_null(), "ZSTD_createCCtx failed"); unsafe { @@ -107,7 +110,12 @@ fn main() { zstd_sys::ZSTD_CCtx_setParameter( cctx, zstd_sys::ZSTD_cParameter::ZSTD_c_checksumFlag, - 1, + i32::from(checksum), + ); + zstd_sys::ZSTD_CCtx_setParameter( + cctx, + zstd_sys::ZSTD_cParameter::ZSTD_c_enableLongDistanceMatching, + i32::from(ldm), ); } let w = unsafe { From 10a61d35a00f845b479dde1a8b7413f5e637f5e2 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 26 Oct 2026 12:18:03 +0200 Subject: [PATCH 6/8] perf(ldm): record a gear split out of line A split comes about once per `2 ^ hash_rate_log` bytes. Without a hint the record was laid out on the fall-through path, so every other byte took a branch around it; under `target-cpu=x86-64-v3` that layout cost the lea-step loop 17% of the frame on runner4. The record is now a cold function the scan branches out to, upstream's compiled shape. --- zstd/src/encoding/ldm/gear_hash.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/zstd/src/encoding/ldm/gear_hash.rs b/zstd/src/encoding/ldm/gear_hash.rs index 6567cc165..514254e8e 100644 --- a/zstd/src/encoding/ldm/gear_hash.rs +++ b/zstd/src/encoding/ldm/gear_hash.rs @@ -230,9 +230,7 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) hash = gear_step(hash, $byte); if hash & mask == 0 { let consumed = $at + 1; - splits[num_splits] = consumed; - num_splits += 1; - if num_splits == LDM_BATCH_SIZE { + if push_split(splits, &mut num_splits, consumed) { state.rolling = hash; return (consumed, num_splits); } @@ -260,6 +258,20 @@ pub(crate) fn feed(state: &mut GearHashState, data: &[u8], splits: &mut [usize]) (data.len(), num_splits) } +/// Record a split at `consumed`; whether the batch is now full. +/// +/// Out of line and cold: a split comes about once per `2 ^ hash_rate_log` +/// bytes, and without the hint the record was laid out on the fall-through +/// path, so every other byte took a branch around it. Upstream's compiled +/// loop branches out to its record, as this one now does. +#[cold] +#[inline(never)] +fn push_split(splits: &mut [usize], num_splits: &mut usize, consumed: usize) -> bool { + splits[*num_splits] = consumed; + *num_splits += 1; + *num_splits == LDM_BATCH_SIZE +} + /// 256-entry random-permutation table from upstream zstd /// `lib/compress/zstd_ldm_geartab.h`. Reproduced verbatim — DO NOT /// REGENERATE: byte-parity of split points (and therefore LDM ratio From 31dfe46f43c8284693ff1691d418e50e7039b581 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 26 Oct 2026 12:39:09 +0200 Subject: [PATCH 7/8] perf(ldm): scan a Fast-level long-distance frame in place A one-shot frame under long-distance matching copied its whole input into the owned history, because the long-distance block is matched in pieces by re-committing it there. Upstream's one-shot frame reads its input where it lies. The Fast backend now does the same: the long-distance search reads the input slice, each gap between long matches is scanned in place ending where the owned window would end, and each match body's tail is indexed up to the match's end. A block cut is moved out of a long match as the owned loop moves it. The borrowed block path also keeps every block on the search under long-distance matching, as the owned path does: a sample cannot see a match from outside the block. Frames are byte-identical to the owned loop's (regression test a_long_distance_frame_scanned_in_place_is_the_owned_frame, including an over-window input). Dictionary frames and the other backends keep the owned history. --- zstd/src/encoding/frame_compressor.rs | 11 +++ zstd/src/encoding/levels/fastest.rs | 7 +- .../encoding/match_generator/long_distance.rs | 68 ++++++++++++++++- zstd/src/encoding/match_generator/mod.rs | 43 ++++++++++- zstd/src/encoding/simple/fast_matcher.rs | 17 +++++ zstd/src/tests/roundtrip_integrity.rs | 74 +++++++++++++++++++ 6 files changed, 212 insertions(+), 8 deletions(-) diff --git a/zstd/src/encoding/frame_compressor.rs b/zstd/src/encoding/frame_compressor.rs index bb1fe9447..d0568662b 100644 --- a/zstd/src/encoding/frame_compressor.rs +++ b/zstd/src/encoding/frame_compressor.rs @@ -2281,6 +2281,17 @@ impl FrameCompressor { block_capacity, savings, ); + // A cut the splitter proposes inside a long match is moved out of + // it, as the owned loop does for the full block it has read. + let available = (input.len() - start).min(block_capacity); + let block_len = if block_len < available && available == block_capacity { + guard + .0 + .matcher + .settle_block_cut_with::(available, block_len) + } else { + block_len + }; let end = (start + block_len).min(input.len()); let block = &input[start..end]; let last_block = end == input.len(); diff --git a/zstd/src/encoding/levels/fastest.rs b/zstd/src/encoding/levels/fastest.rs index 53672895e..dc14aafdd 100644 --- a/zstd/src/encoding/levels/fastest.rs +++ b/zstd/src/encoding/levels/fastest.rs @@ -358,9 +358,10 @@ pub(crate) fn compress_block_encoded_borrowed( // incompressible. let is_rle = !block.is_empty() && block.iter().all(|x| block[0].eq(x)); let window_size = state.matcher.window_size(); - // As on the owned path: an attached dictionary keeps the block on the - // search rather than trusting a sample to say the dictionary is irrelevant. - let dict_rejects_raw = dict_active; + // As on the owned path: an attached dictionary, or long-distance matching, + // keeps the block on the search rather than trusting a sample to say + // matches from outside the block are irrelevant. + let dict_rejects_raw = dict_active || state.matcher.matches_beyond_window_tables(); // As on the owned path: where no block may go out raw, the grid has nothing // to answer and is left alone. let raw_skip_reachable = compression_level_allows_raw_fast_path(compression_level, window_size); diff --git a/zstd/src/encoding/match_generator/long_distance.rs b/zstd/src/encoding/match_generator/long_distance.rs index 58cccbe2a..98519f301 100644 --- a/zstd/src/encoding/match_generator/long_distance.rs +++ b/zstd/src/encoding/match_generator/long_distance.rs @@ -32,8 +32,15 @@ impl MatchGeneratorDriver { } else { to.saturating_sub(window) }; - let live_start = self.ldm_committed_end - self.storage.committed_window().len(); - let live = self.storage.window_with_pending(); + // A frame scanned in place holds the whole input from stream position + // 0; an owned history holds the window and the read-ahead behind it. + let (live, live_start) = match self.storage.borrowed_window() { + Some(input) => (input, 0), + None => ( + self.storage.window_with_pending(), + self.ldm_committed_end - self.storage.committed_window().len(), + ), + }; let floor = reach_floor.max(live_start); debug_assert!(floor <= from, "a search range wider than the window"); self.ldm_found.clear(); @@ -139,6 +146,63 @@ impl MatchGeneratorDriver { true } + /// [`Self::start_matching_long_distance`] for the Fast backend scanning the + /// frame in place: the block `[block_start, block_end)` is never copied, + /// each gap between long matches is scanned where it lies, ending where + /// the owned history would end once the gap is committed, and each match + /// body's tail is indexed up to the match's end. The frame it writes is + /// the owned one's. + pub(super) fn start_matching_long_distance_borrowed( + &mut self, + block_start: usize, + block_end: usize, + handle_sequence: &mut impl FnMut(Sequence), + ) -> bool { + let block_len = self.take_long_distance_block::(); + debug_assert_eq!(block_len, block_end - block_start); + if self.ldm_block.is_empty() { + return false; + } + let sequences = core::mem::take(&mut self.ldm_block); + let simple = self.simple_mut(); + let mut pos = block_start; + for seq in &sequences { + // The gap's trailing literals become the long match's own. + let mut gap = Some(0usize); + if seq.lit_length != 0 { + let gap_end = pos + seq.lit_length; + simple.start_matching_borrowed( + pos, + gap_end, + &mut owned_sink(handle_sequence, &mut gap), + ); + pos = gap_end; + } + let gap_literals = gap.expect("the gap sink only ever writes Some"); + simple.skip_long_match_borrowed(pos, pos + seq.match_length); + pos += seq.match_length; + handle_sequence(Sequence::Triple { + literal_len: gap_literals, + offset: seq.offset, + match_len: seq.match_length, + }); + simple.note_long_match(seq.offset, gap_literals); + } + if pos != block_end { + let mut forward = None; + simple.start_matching_borrowed( + pos, + block_end, + &mut owned_sink(handle_sequence, &mut forward), + ); + } + // The pieces leave the last one as the block the encoder reads back; + // it is the whole block. + simple.stage_borrowed_block(block_start, block_end); + self.ldm_block = sequences; + true + } + /// Match the piece just committed between long matches: by the producer /// when `produced` and it gives a parse, by the backend otherwise. fn match_gap(&mut self, produced: bool, sink: &mut impl FnMut(Sequence)) { diff --git a/zstd/src/encoding/match_generator/mod.rs b/zstd/src/encoding/match_generator/mod.rs index caa1587d7..bb36643e1 100644 --- a/zstd/src/encoding/match_generator/mod.rs +++ b/zstd/src/encoding/match_generator/mod.rs @@ -258,6 +258,16 @@ impl MatcherStorage { } } + /// The whole input of a frame scanned in place, which only the Fast + /// backend does under long-distance matching. + #[cfg(feature = "ldm")] + fn borrowed_window(&self) -> Option<&[u8]> { + match self { + Self::Simple(m) => m.borrowed_window(), + Self::Dfast(_) | Self::Row(_) | Self::HashChain(_) => None, + } + } + /// The committed window followed by the input read but not committed. #[cfg(feature = "ldm")] fn window_with_pending(&self) -> &[u8] { @@ -1044,6 +1054,15 @@ impl MatchGeneratorDriver { "borrowed block range must satisfy start <= end (start={block_start} end={block_end})", ); self.borrowed_pending = Some((block_start, block_end)); + // The long-distance search tracks the stream end the way + // `commit_filled` does for an owned block; a frame scanned in place has + // no dictionary ahead of it, so stream positions are input offsets. + #[cfg(feature = "ldm")] + if self.ldm.is_some() { + debug_assert_eq!(self.ldm_committed_end, block_start); + self.ldm_committed_end = block_end; + self.ldm_split_block = None; + } // Make the range visible to `get_last_space()` immediately: the // emit pipeline reads `get_last_space().len()` in // `collect_block_parts` BEFORE `start_matching` consumes the @@ -1697,16 +1716,21 @@ impl Matcher for MatchGeneratorDriver { // taking it again. The kernels keep positions in `u32`, counting a // dictionary ahead of the slice, so a longer one is copied. let dict_len = dict_hint.map_or(0, |sizes| sizes.content); - // The long-distance matcher splits a block into pieces through the - // owned history, so its frames keep the input there. + // The long-distance matcher matches a block in pieces. The Fast backend + // scans those pieces in place (`start_matching_long_distance_borrowed`), + // as upstream's one-shot frame reads its input where it lies; the other + // backends, and a dictionary frame, split the block through the owned + // history, so their frames keep the input there. #[cfg(feature = "ldm")] let long_distance = self.param_overrides.is_some_and(|ov| ov.ldm.is_some()); #[cfg(not(feature = "ldm"))] let long_distance = false; + let long_distance_in_place = + matches!(next_backend, super::strategy::BackendTag::Simple) && dict_hint.is_none(); self.frame_in_place = match workspace.ingest() { crate::encoding::workspace::IngestPlan::Slice(len) => { (dict_hint.is_none() || fast_attach) - && !long_distance + && (!long_distance || long_distance_in_place) && self.borrowed_supported() && len .checked_add(dict_len) @@ -2379,6 +2403,19 @@ impl MatchGeneratorDriver { // the Simple backend is instrumented (the gate guarantees it), // and the stage is consumed so the next block re-stages. if let Some((block_start, block_end)) = self.borrowed_pending.take() { + // Long-distance matches are taken outright below the optimal + // parser; only the Fast backend reaches here with them (the reset + // keeps every other backend's long-distance frame owned). + #[cfg(feature = "ldm")] + if self.ldm.is_some() + && self.start_matching_long_distance_borrowed::( + block_start, + block_end, + &mut handle_sequence, + ) + { + return; + } // The borrowed scans share their kernels with the owned ones, so // they report through the same sink type. let mut forward = None; diff --git a/zstd/src/encoding/simple/fast_matcher.rs b/zstd/src/encoding/simple/fast_matcher.rs index 612f8f134..13f90b8e0 100644 --- a/zstd/src/encoding/simple/fast_matcher.rs +++ b/zstd/src/encoding/simple/fast_matcher.rs @@ -1687,6 +1687,23 @@ impl FastKernelMatcher { self.prime_hash_table_for_range_stepped(from, FILL_STEP); } + /// [`Self::skip_long_match`] for a match body `[start, end)` of the + /// borrowed window: the same tail, indexed only up to the match's end, as + /// the owned window ends there when the body is committed. + #[cfg(feature = "ldm")] + pub(crate) fn skip_long_match_borrowed(&mut self, start: usize, end: usize) { + const FILL_STEP: usize = 3; + self.table_pos_high_water = self.table_pos_high_water.max(end); + let from = end - (end - start).min(crate::encoding::ldm::LONG_MATCH_INDEXED_TAIL); + self.prime_hash_table_for_range_borrowed(from, end, FILL_STEP); + } + + /// The registered borrowed window, when the frame is scanned in place. + #[cfg(feature = "ldm")] + pub(crate) fn borrowed_window(&self) -> Option<&[u8]> { + self.borrowed.map(|_| self.history_bytes()) + } + /// Carry a long-distance match the driver emitted into the repeat /// offsets: the kernel's pair shifts it in as the newest (upstream /// `ZSTD_ldm_blockCompress`), the encoder-side history takes it as the diff --git a/zstd/src/tests/roundtrip_integrity.rs b/zstd/src/tests/roundtrip_integrity.rs index b0e1c092a..308edd398 100644 --- a/zstd/src/tests/roundtrip_integrity.rs +++ b/zstd/src/tests/roundtrip_integrity.rs @@ -1036,3 +1036,77 @@ fn borrowed_oneshot_matches_owned_and_roundtrips() { } } } + +/// A Fast-level long-distance frame scanned in place must be the frame the +/// owned loop writes, which re-commits each block in pieces around its long +/// matches. The input repeats random runs far apart so the long-distance +/// search finds matches inside blocks, across blocks, and past a block cut; +/// the 3 MiB case runs past the window. Strategies that keep the owned loop +/// under long-distance matching are compared too, as a no-op. +#[test] +fn a_long_distance_frame_scanned_in_place_is_the_owned_frame() { + use crate::encoding::CompressionParameters; + + fn far_repeats(seed: u64, len: usize) -> Vec { + let a = generate_data(seed, 70_000); + let b = generate_data(seed + 1, 50_000); + let mut data = Vec::with_capacity(len); + let mut turn = 0u64; + while data.len() < len { + match turn % 4 { + 0 | 2 => data.extend_from_slice(&a), + 1 => data.extend_from_slice(&b), + _ => data.extend_from_slice(&generate_compressible(seed + turn, 9_000)), + } + turn += 1; + } + data.truncate(len); + data + } + + for level in [ + CompressionLevel::Level(1), + CompressionLevel::Level(-5), + CompressionLevel::Default, + CompressionLevel::Level(5), + ] { + let params = CompressionParameters::builder(level) + .enable_long_distance_matching(true) + .build() + .unwrap(); + for &(seed, len) in &[(1u64, 400_000usize), (2, 131_072), (3, 3_000_000)] { + let data = far_repeats(seed, len); + let borrowed = { + let mut fc: FrameCompressor = FrameCompressor::new(level); + fc.set_parameters(¶ms); + fc.compress_independent_frame(&data) + }; + let owned = { + let mut out = Vec::new(); + let mut fc = FrameCompressor::new(level); + fc.set_parameters(¶ms); + fc.set_source_size_hint(data.len() as u64); + fc.set_source(data.as_slice()); + fc.set_drain(&mut out); + fc.compress(); + out + }; + // The first differing byte, not the frames: they run to megabytes. + let first_difference = borrowed.iter().zip(&owned).position(|(b, o)| b != o); + assert!( + borrowed.len() == owned.len() && first_difference.is_none(), + "long-distance frame scanned in place differs from owned at {level:?} \ + seed={seed} len={len}: {} vs {} bytes, first difference at {first_difference:?}", + borrowed.len(), + owned.len(), + ); + let mut decoder = StreamingDecoder::new(borrowed.as_slice()); + let mut result = Vec::new(); + decoder.read_to_end(&mut result).unwrap(); + assert!( + result == data, + "long-distance roundtrip mismatch at {level:?}" + ); + } + } +} From bc06519a056a8d433f1d4fb79312c90a4e20d1bf Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Mon, 26 Oct 2026 14:32:43 +0200 Subject: [PATCH 8/8] fix(ldm): keep in-place long-distance frames the owned frames - A block written out unsearched (one byte repeated over the block) was skipped by the in-place loop without taking its long-distance matches, so the store was reset at the next block and the search ahead of the block was lost; the owned loop takes them past. The borrowed skip now does too. Regression test: a_long_distance_frame_with_an_unsearched_block_is_the_owned_frame. - An attached Fast dictionary with long-distance matching: the reset keeps such a frame owned, but the dictionary's in-place check still admitted it, tripping the reset's prediction (and in release running the long-distance search without the dictionary ahead of the input). The check now refuses long-distance frames. Regression test: a_long_distance_dictionary_frame_is_the_owned_frame. - The borrowed-vs-owned comparison is one helper shared by the tests. - The decode loop's options say the benchmark's LDM cells need `checksum,ldm`. --- zstd/examples/decode_loop_z000033.rs | 5 +- zstd/src/encoding/match_generator/mod.rs | 12 +- zstd/src/tests/roundtrip_integrity.rs | 143 +++++++++++++++++------ 3 files changed, 119 insertions(+), 41 deletions(-) diff --git a/zstd/examples/decode_loop_z000033.rs b/zstd/examples/decode_loop_z000033.rs index 04b46559c..cc5e0dd45 100644 --- a/zstd/examples/decode_loop_z000033.rs +++ b/zstd/examples/decode_loop_z000033.rs @@ -24,8 +24,9 @@ fn main() { let corpus_path: Option<&str> = args.get(4).map(|s| s.as_str()); // 6th arg, comma-separated encode options: "checksum" sets // content_checksum_flag = 1 so the decode loop exercises the post-decode - // XXH64 verify pass; "ldm" turns on long-distance matching, the frames the - // benchmark's `level__*_ldm` decode cells read. + // XXH64 verify pass; "ldm" turns on long-distance matching. The + // benchmark's `level__*_ldm` decode cells read frames with both, so + // reproduce one with `checksum,ldm`. let options: Vec<&str> = args.get(5).map_or(Vec::new(), |s| s.split(',').collect()); let checksum = options.contains(&"checksum"); let ldm = options.contains(&"ldm"); diff --git a/zstd/src/encoding/match_generator/mod.rs b/zstd/src/encoding/match_generator/mod.rs index bb36643e1..80e093117 100644 --- a/zstd/src/encoding/match_generator/mod.rs +++ b/zstd/src/encoding/match_generator/mod.rs @@ -983,8 +983,12 @@ impl MatchGeneratorDriver { /// path. Checked AFTER priming, so `is_attached()` reflects the resolved /// attach-vs-copy decision. pub(crate) fn borrowed_dict_supported(&self) -> bool { - // As in `borrowed_supported`: a producer reads the committed block. + // As in `borrowed_supported`: a producer reads the committed block. A + // long-distance search counts the dictionary ahead of the input, which + // only the owned history lays out, so the reset keeps such a frame + // owned and this must agree. self.producer.is_none() + && !self.matches_beyond_window_tables() && matches!( &self.storage, MatcherStorage::Simple(m) if m.dict_is_attached() @@ -2307,6 +2311,12 @@ impl MatchGeneratorDriver { // hashes on the dict-priming hint) with no owned-history append // and nothing to recycle. Stage is consumed. if let Some((block_start, block_end)) = self.borrowed_pending.take() { + // As below for an owned block: the long-distance store moves past + // this block, keeping what was searched ahead of it. + #[cfg(feature = "ldm")] + if self.ldm.is_some() { + self.take_long_distance_block::(); + } match self.active_backend() { super::strategy::BackendTag::Simple => self.simple_mut().skip_matching_borrowed( block_start, diff --git a/zstd/src/tests/roundtrip_integrity.rs b/zstd/src/tests/roundtrip_integrity.rs index 308edd398..e8569ccf5 100644 --- a/zstd/src/tests/roundtrip_integrity.rs +++ b/zstd/src/tests/roundtrip_integrity.rs @@ -1045,8 +1045,6 @@ fn borrowed_oneshot_matches_owned_and_roundtrips() { /// under long-distance matching are compared too, as a no-op. #[test] fn a_long_distance_frame_scanned_in_place_is_the_owned_frame() { - use crate::encoding::CompressionParameters; - fn far_repeats(seed: u64, len: usize) -> Vec { let a = generate_data(seed, 70_000); let b = generate_data(seed + 1, 50_000); @@ -1070,43 +1068,112 @@ fn a_long_distance_frame_scanned_in_place_is_the_owned_frame() { CompressionLevel::Default, CompressionLevel::Level(5), ] { - let params = CompressionParameters::builder(level) - .enable_long_distance_matching(true) - .build() - .unwrap(); for &(seed, len) in &[(1u64, 400_000usize), (2, 131_072), (3, 3_000_000)] { - let data = far_repeats(seed, len); - let borrowed = { - let mut fc: FrameCompressor = FrameCompressor::new(level); - fc.set_parameters(¶ms); - fc.compress_independent_frame(&data) - }; - let owned = { - let mut out = Vec::new(); - let mut fc = FrameCompressor::new(level); - fc.set_parameters(¶ms); - fc.set_source_size_hint(data.len() as u64); - fc.set_source(data.as_slice()); - fc.set_drain(&mut out); - fc.compress(); - out - }; - // The first differing byte, not the frames: they run to megabytes. - let first_difference = borrowed.iter().zip(&owned).position(|(b, o)| b != o); - assert!( - borrowed.len() == owned.len() && first_difference.is_none(), - "long-distance frame scanned in place differs from owned at {level:?} \ - seed={seed} len={len}: {} vs {} bytes, first difference at {first_difference:?}", - borrowed.len(), - owned.len(), - ); - let mut decoder = StreamingDecoder::new(borrowed.as_slice()); - let mut result = Vec::new(); - decoder.read_to_end(&mut result).unwrap(); - assert!( - result == data, - "long-distance roundtrip mismatch at {level:?}" - ); + assert_one_shot_long_distance_is_owned(level, &far_repeats(seed, len), None); + } + } +} + +/// A long-distance frame with a block written out unsearched (one byte +/// repeated over a whole block) must still be the owned frame: the owned loop +/// searches that block for long matches and takes them past, so the in-place +/// loop has to as well, or the matches searched ahead of the block and the +/// block's own entries in the long-distance table are lost to it. A later +/// block repeats the run in part, so it reads those entries. +#[test] +fn a_long_distance_frame_with_an_unsearched_block_is_the_owned_frame() { + // A compressible lead-in so the block splitter runs, a random run seen + // twice so the search ahead of a cut block finds a long match, then whole + // blocks of one byte right after the repeat. + for (seed, run) in [(21u64, 3 * 131_072), (22, 200_000), (23, 131_072 + 40_000)] { + let r = generate_data(seed, 60_000); + let mut data = generate_compressible(seed, 300_000); + data.extend_from_slice(&r); + data.extend_from_slice(&generate_compressible(seed + 100, 150_000)); + data.extend_from_slice(&r); + data.extend(core::iter::repeat_n(0xAB, run)); + data.extend_from_slice(&generate_data(seed + 200, 50_000)); + data.extend_from_slice(&r); + for level in [CompressionLevel::Level(1), CompressionLevel::Level(-5)] { + assert_one_shot_long_distance_is_owned(level, &data, None); } } } + +/// A Fast-level long-distance frame with an attached dictionary keeps the +/// owned history, and the one-shot entry must take the owned loop for it too, +/// not the in-place dictionary scan: the long-distance search counts the +/// dictionary ahead of the input. +#[test] +fn a_long_distance_dictionary_frame_is_the_owned_frame() { + let dict_raw = include_bytes!("../../dict_tests/dictionary"); + let dict = crate::encoding::EncoderDictionary::from_bytes(dict_raw).unwrap(); + // Small enough that the Fast backend attaches the dictionary. + let mut data = + b"tenant=demo table=orders op=put key=1 value=aaaaabbbbbcccccdddddeeeee\n".repeat(20); + data.extend_from_slice(&generate_data(31, 1_500)); + data.extend_from_slice(&data.clone()); + assert_one_shot_long_distance_is_owned(CompressionLevel::Level(1), &data, Some(&dict)); +} + +/// Compress `data` with long-distance matching at `level` (and `dict`) through +/// the one-shot entry and through the owned block loop; the frames must be +/// the same bytes, and the one-shot frame must round-trip. +fn assert_one_shot_long_distance_is_owned( + level: CompressionLevel, + data: &[u8], + dict: Option<&crate::encoding::EncoderDictionary>, +) { + let params = crate::encoding::CompressionParameters::builder(level) + .enable_long_distance_matching(true) + .build() + .unwrap(); + let borrowed = { + let mut fc: FrameCompressor = FrameCompressor::new(level); + if let Some(dict) = dict { + fc.set_encoder_dictionary(dict.clone()).unwrap(); + } + fc.set_parameters(¶ms); + fc.compress_independent_frame(data) + }; + let owned = { + let mut out = Vec::new(); + let mut fc = FrameCompressor::new(level); + if let Some(dict) = dict { + fc.set_encoder_dictionary(dict.clone()).unwrap(); + } + fc.set_parameters(¶ms); + fc.set_source_size_hint(data.len() as u64); + fc.set_source(data); + fc.set_drain(&mut out); + fc.compress(); + out + }; + // The first differing byte, not the frames: they run to megabytes. + let first_difference = borrowed.iter().zip(&owned).position(|(b, o)| b != o); + assert!( + borrowed.len() == owned.len() && first_difference.is_none(), + "long-distance frame scanned in place differs from owned at {level:?} len={}: \ + {} vs {} bytes, first difference at {first_difference:?}", + data.len(), + borrowed.len(), + owned.len(), + ); + let mut decoder = match dict { + Some(_) => { + let raw = include_bytes!("../../dict_tests/dictionary"); + let mut decoder = crate::decoding::FrameDecoder::new(); + decoder + .add_dict(crate::decoding::Dictionary::decode_dict(raw).unwrap()) + .unwrap(); + StreamingDecoder::new_with_decoder(borrowed.as_slice(), decoder) + } + None => StreamingDecoder::new(borrowed.as_slice()), + }; + let mut result = Vec::new(); + decoder.read_to_end(&mut result).unwrap(); + assert!( + result == data, + "long-distance roundtrip mismatch at {level:?}" + ); +}