From 23450d8c3b2dd0f260c0862d88b728ee798912cc Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sat, 18 Jul 2026 15:29:02 +0200 Subject: [PATCH] feat(cli): move patch-from parameter policy to Rust Keep the public ZSTD_getCParams() selection and all DISPLAYLEVEL output in programs/fileio.c, but pass its by-value compression-parameter snapshot to a Rust policy leaf. Rust now preserves the original source-size highbit and window clamping rules, validates and updates the patch memory limit before success outputs, derives cycleLog, updates comprParams->windowLog, and applies automatic LDM without overwriting the explicit-LDM diagnostic semantics. The Rust ABI returns the raw file window and the two diagnostic predicates so C can retain its existing messages and optimal-parser note ordering. C layout assertions cover the seven-word compression-parameter snapshot on both platform-width variants; rejected unknown and oversized inputs leave all outputs unchanged. Test Plan: - Rust clippy pre/post nightly-format matrix for library compression and CLI cli,compression,decompression,benchmark features -- passed. - `cargo test --manifest-path rust/cli/Cargo.toml --no-default-features --features cli,compression,decompression,benchmark` -- 137 passed. - `make -C programs -j2 zstd` -- passed. - `make -C tests -j2 test-cli-tests` -- 41 passed. - `python3 tests/cli-tests/run.py decompression/pass-through.sh` -- passed. - Focused patch-from round trips, stream-size failure ordering, and automatic long-mode diagnostics -- passed. - `make -C tests -j2 test-rust-lib-smoke` -- passed. - No i686 Rust target is installed; 32-bit policy bounds are covered by size_of-based constants and C/Rust ABI assertions. --- programs/fileio.c | 63 +++++----- rust/src/fileio_prefs.rs | 266 +++++++++++++++++++++++++++++++-------- 2 files changed, 252 insertions(+), 77 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 6ce4c32ab..69505379d 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -282,6 +282,12 @@ typedef char FIO_rust_ctx_total_output_offset[ typedef char FIO_rust_ctx_size[ (sizeof(FIO_ctx_t) == offsetof(FIO_ctx_t, totalBytesOutput) + sizeof(size_t)) ? 1 : -1]; +typedef char FIO_rust_compression_params_strategy_offset[ + (offsetof(ZSTD_compressionParameters, strategy) + == 6 * sizeof(unsigned)) ? 1 : -1]; +typedef char FIO_rust_compression_params_size[ + (sizeof(ZSTD_compressionParameters) + == 7 * sizeof(unsigned)) ? 1 : -1]; int FIO_rust_shouldDisplayFileSummary(const FIO_ctx_t* fCtx); int FIO_rust_shouldDisplayMultipleFileSummary(const FIO_ctx_t* fCtx); @@ -328,11 +334,14 @@ static int FIO_shouldDisplayMultipleFileSummary(FIO_ctx_t const* fCtx) int FIO_rust_checkFilenameCollisions(const char** filenameTable, unsigned nbFiles); unsigned FIO_rust_highbit64(unsigned long long v); unsigned long long FIO_rust_getLargestFileSize(const char** inFileNames, unsigned nbFiles); -unsigned FIO_rust_cycleLog(unsigned hashLog, int strategy); -void FIO_rust_patchFromWindowPolicy(unsigned fileWindowLog, - unsigned cycleLog, - unsigned* windowLog, - int* enableLdm); +int FIO_rust_adjustParamsForPatchFromMode(FIO_prefs_t* prefs, + ZSTD_compressionParameters* comprParams, + unsigned long long dictSize, + unsigned long long maxSrcFileSize, + ZSTD_compressionParameters cParams, + unsigned* fileWindowLog, + int* autoLdm, + int* optimalParser); void FIO_rust_setInBuffer(ZSTD_inBuffer* output, const void* buf, size_t s, size_t pos); void FIO_rust_setOutBuffer(ZSTD_outBuffer* output, void* buf, size_t s, size_t pos); const char* FIO_rust_determineCompressedName(const char* srcFileName, const char* outDirName, const char* suffix); @@ -865,38 +874,36 @@ typedef struct { ReadPoolCtx_t *readCtx; } cRess_t; -/** ZSTD_cycleLog() : - * condition for correct operation : hashLog > 1 */ -static U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat) -{ - assert(hashLog > 1); - return FIO_rust_cycleLog(hashLog, (int)strat); -} - static void FIO_adjustParamsForPatchFromMode(FIO_prefs_t* const prefs, ZSTD_compressionParameters* comprParams, unsigned long long const dictSize, unsigned long long const maxSrcFileSize, int cLevel) { - unsigned const fileWindowLog = FIO_highbit64(maxSrcFileSize) + 1; + enum { + FIO_PATCH_MEM_LIMIT_SUCCESS = 0, + FIO_PATCH_MEM_LIMIT_UNKNOWN_SIZE = 1, + FIO_PATCH_MEM_LIMIT_TOO_LARGE = 2 + }; ZSTD_compressionParameters const cParams = ZSTD_getCParams(cLevel, (size_t)maxSrcFileSize, (size_t)dictSize); - unsigned windowLog; - int enableLdm; - FIO_adjustMemLimitForPatchFromMode(prefs, dictSize, maxSrcFileSize); - FIO_rust_patchFromWindowPolicy(fileWindowLog, - ZSTD_cycleLog(cParams.chainLog, cParams.strategy), - &windowLog, - &enableLdm); + unsigned fileWindowLog; + int autoLdm; + int optimalParser; + int const status = FIO_rust_adjustParamsForPatchFromMode( + prefs, comprParams, dictSize, maxSrcFileSize, cParams, + &fileWindowLog, &autoLdm, &optimalParser); + if (status == FIO_PATCH_MEM_LIMIT_UNKNOWN_SIZE) + EXM_THROW(42, "Using --patch-from with stdin requires --stream-size"); + if (status == FIO_PATCH_MEM_LIMIT_TOO_LARGE) { + unsigned const maxWindowSize = (1U << ZSTD_WINDOWLOG_MAX); + EXM_THROW(42, "Can't handle files larger than %u GB\n", maxWindowSize/(1 GB)); + } + assert(status == FIO_PATCH_MEM_LIMIT_SUCCESS); if (fileWindowLog > ZSTD_WINDOWLOG_MAX) DISPLAYLEVEL(1, "Max window log exceeded by file (compression ratio will suffer)\n"); - comprParams->windowLog = windowLog; - if (enableLdm) { - if (!prefs->ldmFlag) - DISPLAYLEVEL(2, "long mode automatically triggered\n"); - FIO_setLdmFlag(prefs, 1); - } - if (cParams.strategy >= ZSTD_btopt) { + if (autoLdm) + DISPLAYLEVEL(2, "long mode automatically triggered\n"); + if (optimalParser) { DISPLAYLEVEL(4, "[Optimal parser notes] Consider the following to improve patch size at the cost of speed:\n"); DISPLAYLEVEL(4, "- Set a larger targetLength (e.g. --zstd=targetLength=4096)\n"); DISPLAYLEVEL(4, "- Set a larger chainLog (e.g. --zstd=chainLog=%u)\n", ZSTD_CHAINLOG_MAX); diff --git a/rust/src/fileio_prefs.rs b/rust/src/fileio_prefs.rs index 42ca020e3..3862b83d8 100644 --- a/rust/src/fileio_prefs.rs +++ b/rust/src/fileio_prefs.rs @@ -32,6 +32,7 @@ const UTIL_FILESIZE_UNKNOWN: u64 = u64::MAX; const ZSTD_WINDOWLOG_MIN: u32 = 10; const ZSTD_WINDOWLOG_MAX: u32 = if size_of::() == 4 { 30 } else { 31 }; const ZSTD_BTLAZY2: c_int = 6; +const ZSTD_BTOPT: c_int = 7; static STDOUT_MARK: &[u8] = b"/*stdout*\\\0"; static mut COMPRESSED_NAME_CAPACITY: usize = 0; @@ -53,6 +54,23 @@ pub struct FIO_outBuffer { pos: usize, } +/// ABI snapshot of C's public `ZSTD_compressionParameters`. +/// +/// `programs/fileio.c` keeps selecting the parameters with `ZSTD_getCParams()` +/// and passes this value by value. Rust only consumes the policy fields; it +/// does not reach into a private compression context. +#[repr(C)] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct FIO_compressionParameters { + windowLog: c_uint, + chainLog: c_uint, + hashLog: c_uint, + searchLog: c_uint, + minMatch: c_uint, + targetLength: c_uint, + strategy: c_int, +} + /// C's private `fileInfo_t` from `programs/fileio.c`. #[repr(C)] pub struct FIO_fileInfo_t { @@ -590,8 +608,8 @@ pub unsafe extern "C" fn FIO_determineHasStdinInput( /// This mirrors `FIO_highbit64()` and deliberately keeps its zero-input /// assertion semantics. In builds where assertions are disabled, zero /// still produces the same result as the original shift loop. -#[no_mangle] -pub extern "C" fn FIO_rust_highbit64(value: u64) -> c_uint { +#[inline] +fn highbit64(value: u64) -> c_uint { debug_assert!(value != 0); if value == 0 { return 0; @@ -599,6 +617,11 @@ pub extern "C" fn FIO_rust_highbit64(value: u64) -> c_uint { (u64::BITS - 1 - value.leading_zeros()) as c_uint } +#[no_mangle] +pub extern "C" fn FIO_rust_highbit64(value: u64) -> c_uint { + highbit64(value) +} + #[inline] fn cycle_log(hash_log: c_uint, strategy: c_int) -> c_uint { hash_log.wrapping_sub(c_uint::from(strategy >= ZSTD_BTLAZY2)) @@ -610,18 +633,60 @@ pub extern "C" fn FIO_rust_cycleLog(hash_log: c_uint, strategy: c_int) -> c_uint cycle_log(hash_log, strategy) } -/// Rust scalar ABI for patch-from window and automatic-LDM policy. +/// Apply the deterministic patch-from parameter policy. +/// +/// C intentionally selects `c_params` with its public `ZSTD_getCParams()` API +/// and keeps all user-facing diagnostics in the wrapper. Rust owns the +/// remaining policy: source-size window calculation, memory-limit validation, +/// cycle/window selection, and automatic LDM mutation. The three output +/// pointers return the predicates needed by C to preserve its diagnostics. +#[allow(clippy::too_many_arguments)] #[no_mangle] -pub unsafe extern "C" fn FIO_rust_patchFromWindowPolicy( - file_window_log: c_uint, - cycle_log: c_uint, - window_log: *mut c_uint, - enable_ldm: *mut c_int, -) { +pub unsafe extern "C" fn FIO_rust_adjustParamsForPatchFromMode( + prefs: *mut FIO_prefs_t, + compr_params: *mut FIO_compressionParameters, + dict_size: u64, + max_src_file_size: u64, + c_params: FIO_compressionParameters, + file_window_log: *mut c_uint, + auto_ldm: *mut c_int, + optimal_parser: *mut c_int, +) -> c_int { + /* Keep this calculation before memory-limit validation: the original C + * helper evaluates FIO_highbit64() before calling its mem-limit helper. + * `debug_assert!` and the zero fallback preserve that helper's debug and + * NDEBUG behavior. */ + let file_window_log_value = highbit64(max_src_file_size).wrapping_add(1); + let current_mem_limit = unsafe { (*prefs).memLimit }; + let mem_limit = match adjusted_patch_mem_limit(current_mem_limit, dict_size, max_src_file_size) + { + Ok(mem_limit) => mem_limit, + Err(status) => return status, + }; + + /* C's ZSTD_cycleLog() asserted this invariant before the scalar policy + * was moved here. Keep the assertion while retaining wrapping arithmetic + * in non-asserting builds. */ + debug_assert!(c_params.chainLog > 1); + let cycle_log_value = cycle_log(c_params.chainLog, c_params.strategy); + let window_log = file_window_log_value.clamp(ZSTD_WINDOWLOG_MIN, ZSTD_WINDOWLOG_MAX); + let enable_ldm = file_window_log_value > cycle_log_value; + let was_ldm_disabled = unsafe { (*prefs).ldmFlag == 0 }; + unsafe { - window_log.write(file_window_log.clamp(ZSTD_WINDOWLOG_MIN, ZSTD_WINDOWLOG_MAX)); - enable_ldm.write(c_int::from(file_window_log > cycle_log)); + ptr::addr_of_mut!((*prefs).memLimit).write(mem_limit); + ptr::addr_of_mut!((*compr_params).windowLog).write(window_log); + if enable_ldm { + /* Match FIO_setLdmFlag(prefs, 1): an automatic trigger always + * normalizes the stored preference to exactly one. */ + ptr::addr_of_mut!((*prefs).ldmFlag).write(1); + } + file_window_log.write(file_window_log_value); + auto_ldm.write(c_int::from(enable_ldm && was_ldm_disabled)); + optimal_parser.write(c_int::from(c_params.strategy >= ZSTD_BTOPT)); } + + FIO_PATCH_MEM_LIMIT_SUCCESS } #[allow(clippy::too_many_arguments)] @@ -1083,6 +1148,50 @@ mod tests { prefs } + fn compression_params(chain_log: c_uint, strategy: c_int) -> FIO_compressionParameters { + FIO_compressionParameters { + windowLog: 19, + chainLog: chain_log, + hashLog: 18, + searchLog: 1, + minMatch: 4, + targetLength: 0, + strategy, + } + } + + fn apply_patch_policy( + prefs: &mut FIO_prefs_t, + dict_size: u64, + max_src_file_size: u64, + c_params: FIO_compressionParameters, + ) -> (c_int, FIO_compressionParameters, c_uint, c_int, c_int) { + let mut compr_params = c_params; + compr_params.windowLog = 99; + let mut file_window_log = c_uint::MAX; + let mut auto_ldm = -1; + let mut optimal_parser = -1; + let status = unsafe { + FIO_rust_adjustParamsForPatchFromMode( + prefs, + &mut compr_params, + dict_size, + max_src_file_size, + c_params, + &mut file_window_log, + &mut auto_ldm, + &mut optimal_parser, + ) + }; + ( + status, + compr_params, + file_window_log, + auto_ldm, + optimal_parser, + ) + } + #[test] fn patch_mem_limit_preserves_existing_limit() { let mut prefs = prefs_with_mem_limit(4096); @@ -1194,52 +1303,111 @@ mod tests { assert_eq!(FIO_rust_cycleLog(20, ZSTD_BTLAZY2 + 3), 19); } - fn patch_from_window_policy(file_window_log: c_uint, cycle_log: c_uint) -> (c_uint, c_int) { - let mut window_log = c_uint::MAX; - let mut enable_ldm = -1; - unsafe { - FIO_rust_patchFromWindowPolicy( - file_window_log, - cycle_log, - &mut window_log, - &mut enable_ldm, - ); - } - (window_log, enable_ldm) - } - #[test] - fn patch_from_window_policy_clamps_to_lower_bound() { + fn patch_policy_matches_cparams_abi_and_window_boundaries() { assert_eq!( - patch_from_window_policy(ZSTD_WINDOWLOG_MIN - 1, ZSTD_WINDOWLOG_MIN), - (ZSTD_WINDOWLOG_MIN, 0) + size_of::(), + 7 * size_of::() ); - } - - #[test] - fn patch_from_window_policy_clamps_to_upper_bound() { assert_eq!( - patch_from_window_policy(ZSTD_WINDOWLOG_MAX + 1, ZSTD_WINDOWLOG_MAX + 1), - (ZSTD_WINDOWLOG_MAX, 0) + offset_of!(FIO_compressionParameters, strategy), + 6 * size_of::() ); - } - #[test] - fn patch_from_window_policy_keeps_equal_cycle_log_out_of_ldm() { - assert_eq!(patch_from_window_policy(20, 20), (20, 0)); - } + let mut prefs = prefs_with_mem_limit(0); + let (status, compr_params, file_window_log, auto_ldm, optimal_parser) = + apply_patch_policy(&mut prefs, 0, 1, compression_params(10, 1)); + assert_eq!(status, FIO_PATCH_MEM_LIMIT_SUCCESS); + assert_eq!(compr_params.windowLog, ZSTD_WINDOWLOG_MIN); + assert_eq!(file_window_log, 1); + assert_eq!(auto_ldm, 0); + assert_eq!(optimal_parser, 0); - #[test] - fn patch_from_window_policy_enables_ldm_one_above_cycle_log() { - assert_eq!(patch_from_window_policy(20, 19), (20, 1)); - } - - #[test] - fn patch_from_window_policy_uses_raw_value_above_upper_bound_for_ldm() { - assert_eq!( - patch_from_window_policy(ZSTD_WINDOWLOG_MAX + 1, ZSTD_WINDOWLOG_MAX), - (ZSTD_WINDOWLOG_MAX, 1) + let mut prefs = prefs_with_mem_limit(0); + let source_size = 1u64 << (ZSTD_WINDOWLOG_MAX - 1); + let (status, compr_params, file_window_log, auto_ldm, optimal_parser) = apply_patch_policy( + &mut prefs, + 0, + source_size, + compression_params(ZSTD_WINDOWLOG_MAX, 1), ); + assert_eq!(status, FIO_PATCH_MEM_LIMIT_SUCCESS); + assert_eq!(compr_params.windowLog, ZSTD_WINDOWLOG_MAX); + assert_eq!(file_window_log, ZSTD_WINDOWLOG_MAX); + assert_eq!(auto_ldm, 0); + assert_eq!(optimal_parser, 0); + + let mut prefs = prefs_with_mem_limit(0); + let (status, compr_params, file_window_log, auto_ldm, optimal_parser) = apply_patch_policy( + &mut prefs, + 0, + 1u64 << ZSTD_WINDOWLOG_MAX, + compression_params(ZSTD_WINDOWLOG_MAX, 1), + ); + assert_eq!(status, FIO_PATCH_MEM_LIMIT_SUCCESS); + assert_eq!(compr_params.windowLog, ZSTD_WINDOWLOG_MAX); + assert_eq!(file_window_log, ZSTD_WINDOWLOG_MAX + 1); + assert_eq!(auto_ldm, 1); + assert_eq!(optimal_parser, 0); + } + + #[test] + fn patch_policy_returns_memory_statuses_before_mutating_outputs() { + let mut prefs = prefs_with_mem_limit(4096); + let c_params = compression_params(20, ZSTD_BTOPT); + let (status, compr_params, file_window_log, auto_ldm, optimal_parser) = + apply_patch_policy(&mut prefs, 0, UTIL_FILESIZE_UNKNOWN, c_params); + assert_eq!(status, FIO_PATCH_MEM_LIMIT_UNKNOWN_SIZE); + assert_eq!(prefs.memLimit, 4096); + assert_eq!(compr_params.windowLog, 99); + assert_eq!(file_window_log, c_uint::MAX); + assert_eq!(auto_ldm, -1); + assert_eq!(optimal_parser, -1); + + let mut prefs = prefs_with_mem_limit(4096); + let (status, compr_params, file_window_log, auto_ldm, optimal_parser) = + apply_patch_policy(&mut prefs, 0, (1u64 << ZSTD_WINDOWLOG_MAX) + 1, c_params); + assert_eq!(status, FIO_PATCH_MEM_LIMIT_TOO_LARGE); + assert_eq!(prefs.memLimit, 4096); + assert_eq!(compr_params.windowLog, 99); + assert_eq!(file_window_log, c_uint::MAX); + assert_eq!(auto_ldm, -1); + assert_eq!(optimal_parser, -1); + } + + #[test] + fn patch_policy_automatically_enables_ldm_only_when_needed() { + let mut prefs = prefs_with_mem_limit(0); + let (status, compr_params, file_window_log, auto_ldm, optimal_parser) = + apply_patch_policy(&mut prefs, 0, 1u64 << 13, compression_params(13, 1)); + assert_eq!(status, FIO_PATCH_MEM_LIMIT_SUCCESS); + assert_eq!(compr_params.windowLog, 14); + assert_eq!(file_window_log, 14); + assert_eq!(prefs.ldmFlag, 1); + assert_eq!(auto_ldm, 1); + assert_eq!(optimal_parser, 0); + + let mut prefs = prefs_with_mem_limit(0); + prefs.ldmFlag = 1; + let (status, _, _, auto_ldm, _) = + apply_patch_policy(&mut prefs, 0, 1u64 << 13, compression_params(13, 1)); + assert_eq!(status, FIO_PATCH_MEM_LIMIT_SUCCESS); + assert_eq!(prefs.ldmFlag, 1); + assert_eq!(auto_ldm, 0); + } + + #[test] + fn patch_policy_reports_optimal_parser_condition_from_cparams() { + let mut prefs = prefs_with_mem_limit(0); + let (status, _, _, auto_ldm, optimal_parser) = apply_patch_policy( + &mut prefs, + 0, + 1u64 << 10, + compression_params(20, ZSTD_BTOPT), + ); + assert_eq!(status, FIO_PATCH_MEM_LIMIT_SUCCESS); + assert_eq!(auto_ldm, 0); + assert_eq!(optimal_parser, 1); } #[test]