From 9fdc27d596f96f7f7cd08451cf79213cec6d51bb Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sun, 19 Jul 2026 15:23:55 +0200 Subject: [PATCH] feat(compress): move CDict reset selection into Rust Move the shared CDict attach-versus-copy policy and callback dispatch into a Rust-owned ABI leaf. Keep both workspace- and match-state-heavy reset implementations in C behind opaque callbacks. Test Plan: - cargo test --manifest-path rust/Cargo.toml --lib - cargo clippy --manifest-path rust/Cargo.toml --all-targets -- -D warnings - make -B -C programs -j1 zstd - make -C tests -j1 test-zstream ZSTREAM_TESTTIME=-T1s - focused reset_using_cdict unit tests --- lib/compress/zstd_compress.c | 106 ++++++++-- rust/src/zstd_compress_dictionary.rs | 293 ++++++++++++++++++++++++++- 2 files changed, 376 insertions(+), 23 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index f1d620a70..960a9f8ef 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1011,9 +1011,6 @@ ZSTD_compressionParameters ZSTD_rust_params_dedicatedDictSearch_getCParams(ZSTD_compressionParameters cParams); ZSTD_compressionParameters ZSTD_rust_params_dedicatedDictSearch_revertCParams(ZSTD_compressionParameters cParams); -int ZSTD_rust_params_shouldAttachDict(int strategy, int dedicatedDictSearch, - U64 pledgedSrcSize, int attachDictPref, - int forceWindow); int ZSTD_rust_params_getCParamMode(int cdict_present, int cdict_strategy, int cdict_dedicated_search, U64 pledgedSrcSize, @@ -1555,6 +1552,52 @@ typedef char ZSTD_rust_compress_begin_state_layout[ && sizeof(ZSTD_rust_compressBeginState) == 18 * sizeof(void*)) ? 1 : -1]; +typedef size_t (*ZSTD_rust_resetCCtxUsingCDictAttach_f)( + void* context, const void* cdict, const void* params, + U64 pledgedSrcSize, int zbuff); +typedef size_t (*ZSTD_rust_resetCCtxUsingCDictCopy_f)( + void* context, const void* cdict, const void* params, + U64 pledgedSrcSize, int zbuff); +typedef struct { + void* callbackContext; + const void* cdict; + const void* params; + const int* cdictStrategy; + const int* dedicatedDictSearch; + const int* attachDictPref; + const int* forceWindow; + const U64* pledgedSrcSize; + const int* zbuff; + ZSTD_rust_resetCCtxUsingCDictAttach_f attach; + ZSTD_rust_resetCCtxUsingCDictCopy_f copy; +} ZSTD_rust_resetCCtxUsingCDictState; +size_t ZSTD_rust_resetCCtxUsingCDict( + const ZSTD_rust_resetCCtxUsingCDictState* state); +typedef char ZSTD_rust_reset_cctx_using_cdict_state_layout[ + (offsetof(ZSTD_rust_resetCCtxUsingCDictState, callbackContext) == 0 + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, cdict) + == sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, params) + == 2 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, cdictStrategy) + == 3 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, dedicatedDictSearch) + == 4 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, attachDictPref) + == 5 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, forceWindow) + == 6 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, pledgedSrcSize) + == 7 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, zbuff) + == 8 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, attach) + == 9 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxUsingCDictState, copy) + == 10 * sizeof(void*) + && sizeof(ZSTD_rust_resetCCtxUsingCDictState) == 11 * sizeof(void*)) + ? 1 : -1]; + /* The sequence-compression loop receives only the state it actually reads or * updates. In particular, neither ZSTD_CCtx nor a C function pointer crosses * the Rust ABI. */ @@ -3164,18 +3207,6 @@ void ZSTD_invalidateRepCodes(ZSTD_CCtx* cctx) { assert(!ZSTD_window_hasExtDict(cctx->blockState.matchState.window)); } -static int ZSTD_shouldAttachDict(const ZSTD_CDict* cdict, - const ZSTD_CCtx_params* params, - U64 pledgedSrcSize) -{ - return ZSTD_rust_params_shouldAttachDict( - cdict->matchState.cParams.strategy, - cdict->matchState.dedicatedDictSearch, - pledgedSrcSize, - (int)params->attachDictPref, - params->forceWindow); -} - static size_t ZSTD_resetCCtx_byAttachingCDict(ZSTD_CCtx* cctx, const ZSTD_CDict* cdict, @@ -3331,6 +3362,26 @@ static size_t ZSTD_resetCCtx_byCopyingCDict(ZSTD_CCtx* cctx, return 0; } +static size_t ZSTD_rust_resetCCtxUsingCDict_attach( + void* context, const void* cdict, const void* params, + U64 pledgedSrcSize, int zbuff) +{ + return ZSTD_resetCCtx_byAttachingCDict( + (ZSTD_CCtx*)context, (const ZSTD_CDict*)cdict, + *(const ZSTD_CCtx_params*)params, pledgedSrcSize, + (ZSTD_buffered_policy_e)zbuff); +} + +static size_t ZSTD_rust_resetCCtxUsingCDict_copy( + void* context, const void* cdict, const void* params, + U64 pledgedSrcSize, int zbuff) +{ + return ZSTD_resetCCtx_byCopyingCDict( + (ZSTD_CCtx*)context, (const ZSTD_CDict*)cdict, + *(const ZSTD_CCtx_params*)params, pledgedSrcSize, + (ZSTD_buffered_policy_e)zbuff); +} + /* We have a choice between copying the dictionary context into the working * context, or referencing the dictionary context from the working context * in-place. We decide here which strategy to use. */ @@ -3340,17 +3391,28 @@ static size_t ZSTD_resetCCtx_usingCDict(ZSTD_CCtx* cctx, U64 pledgedSrcSize, ZSTD_buffered_policy_e zbuff) { + ZSTD_rust_resetCCtxUsingCDictState state; + int const cdictStrategy = (int)cdict->matchState.cParams.strategy; + int const dedicatedDictSearch = (int)cdict->matchState.dedicatedDictSearch; + int const attachDictPref = (int)params->attachDictPref; + int const forceWindow = (int)params->forceWindow; + int const zbuffValue = (int)zbuff; DEBUGLOG(4, "ZSTD_resetCCtx_usingCDict (pledgedSrcSize=%u)", (unsigned)pledgedSrcSize); - if (ZSTD_shouldAttachDict(cdict, params, pledgedSrcSize)) { - return ZSTD_resetCCtx_byAttachingCDict( - cctx, cdict, *params, pledgedSrcSize, zbuff); - } else { - return ZSTD_resetCCtx_byCopyingCDict( - cctx, cdict, *params, pledgedSrcSize, zbuff); - } + state.callbackContext = cctx; + state.cdict = cdict; + state.params = params; + state.cdictStrategy = &cdictStrategy; + state.dedicatedDictSearch = &dedicatedDictSearch; + state.attachDictPref = &attachDictPref; + state.forceWindow = &forceWindow; + state.pledgedSrcSize = &pledgedSrcSize; + state.zbuff = &zbuffValue; + state.attach = ZSTD_rust_resetCCtxUsingCDict_attach; + state.copy = ZSTD_rust_resetCCtxUsingCDict_copy; + return ZSTD_rust_resetCCtxUsingCDict(&state); } /*! ZSTD_copyCCtx_internal() : diff --git a/rust/src/zstd_compress_dictionary.rs b/rust/src/zstd_compress_dictionary.rs index e09cb5c00..ac6f45490 100644 --- a/rust/src/zstd_compress_dictionary.rs +++ b/rust/src/zstd_compress_dictionary.rs @@ -17,7 +17,7 @@ use crate::fse_compress::FSE_buildCTable_wksp; use crate::huf_compress::HUF_readCTable; use crate::zstd_compress_params::{ ZSTD_compressionParameters, ZSTD_frameParameters, ZSTD_parameters, ZSTD_rust_params_getCParams, - ZSTD_CONTENTSIZE_UNKNOWN, + ZSTD_rust_params_shouldAttachDict, ZSTD_CONTENTSIZE_UNKNOWN, }; use crate::zstd_compress_params_api::ZSTD_CCtx_params; use crate::zstd_compress_stats::{ @@ -570,6 +570,105 @@ pub unsafe extern "C" fn ZSTD_rust_compressBegin( 0 } +type ResetCCtxUsingCDictAttachFn = + unsafe extern "C" fn(*mut c_void, *const c_void, *const c_void, u64, c_int) -> usize; +type ResetCCtxUsingCDictCopyFn = + unsafe extern "C" fn(*mut c_void, *const c_void, *const c_void, u64, c_int) -> usize; + +/// Projection for the attach-versus-copy decision in +/// `ZSTD_resetCCtx_usingCDict`. +/// +/// Rust owns the dictionary attachment policy. C retains both reset +/// implementations because they operate on private CCtx, CDict, workspace, +/// and match-state layouts. +#[repr(C)] +pub struct ZSTD_rust_resetCCtxUsingCDictState { + callback_context: *mut c_void, + cdict: *const c_void, + params: *const c_void, + cdict_strategy: *const c_int, + dedicated_dict_search: *const c_int, + attach_dict_pref: *const c_int, + force_window: *const c_int, + pledged_src_size: *const u64, + zbuff: *const c_int, + attach: Option, + copy: Option, +} + +const _: () = { + assert!(size_of::() == size_of::()); + assert!(size_of::() == size_of::()); + assert!(offset_of!(ZSTD_rust_resetCCtxUsingCDictState, callback_context) == 0); + assert!(offset_of!(ZSTD_rust_resetCCtxUsingCDictState, cdict) == size_of::()); + assert!(offset_of!(ZSTD_rust_resetCCtxUsingCDictState, params) == 2 * size_of::()); + assert!( + offset_of!(ZSTD_rust_resetCCtxUsingCDictState, cdict_strategy) == 3 * size_of::() + ); + assert!( + offset_of!(ZSTD_rust_resetCCtxUsingCDictState, dedicated_dict_search) + == 4 * size_of::() + ); + assert!( + offset_of!(ZSTD_rust_resetCCtxUsingCDictState, attach_dict_pref) == 5 * size_of::() + ); + assert!(offset_of!(ZSTD_rust_resetCCtxUsingCDictState, force_window) == 6 * size_of::()); + assert!( + offset_of!(ZSTD_rust_resetCCtxUsingCDictState, pledged_src_size) == 7 * size_of::() + ); + assert!(offset_of!(ZSTD_rust_resetCCtxUsingCDictState, zbuff) == size_of::<[usize; 8]>()); + assert!(offset_of!(ZSTD_rust_resetCCtxUsingCDictState, attach) == 9 * size_of::()); + assert!(offset_of!(ZSTD_rust_resetCCtxUsingCDictState, copy) == 10 * size_of::()); + assert!(size_of::() == size_of::<[usize; 11]>()); +}; + +/// Select the private C reset implementation for an already-built CDict. +#[no_mangle] +pub unsafe extern "C" fn ZSTD_rust_resetCCtxUsingCDict( + state: *const ZSTD_rust_resetCCtxUsingCDictState, +) -> usize { + if state.is_null() { + return ERROR(ZstdErrorCode::Generic); + } + let state = unsafe { &*state }; + let Some(attach) = state.attach else { + return ERROR(ZstdErrorCode::Generic); + }; + let Some(copy) = state.copy else { + return ERROR(ZstdErrorCode::Generic); + }; + if state.callback_context.is_null() + || state.cdict.is_null() + || state.params.is_null() + || state.cdict_strategy.is_null() + || state.dedicated_dict_search.is_null() + || state.attach_dict_pref.is_null() + || state.force_window.is_null() + || state.pledged_src_size.is_null() + || state.zbuff.is_null() + { + return ERROR(ZstdErrorCode::Generic); + } + + let should_attach = ZSTD_rust_params_shouldAttachDict( + unsafe { *state.cdict_strategy }, + unsafe { *state.dedicated_dict_search }, + unsafe { *state.pledged_src_size }, + unsafe { *state.attach_dict_pref }, + unsafe { *state.force_window }, + ) != 0; + let callback = if should_attach { attach } else { copy }; + unsafe { + callback( + state.callback_context, + state.cdict, + state.params, + *state.pledged_src_size, + *state.zbuff, + ) + } +} + /// Scalar projections for the public CDict query helpers. #[repr(C)] pub struct ZSTD_rust_cdictQueryState { @@ -1500,6 +1599,198 @@ mod tests { assert_eq!(probe.attach_zbuff, zbuff); } + struct ResetUsingCDictProbe { + events: Vec<&'static str>, + attach_cdict: *const c_void, + attach_params: *const c_void, + attach_pledged_src_size: u64, + attach_zbuff: c_int, + attach_result: usize, + copy_cdict: *const c_void, + copy_params: *const c_void, + copy_pledged_src_size: u64, + copy_zbuff: c_int, + copy_result: usize, + } + + impl Default for ResetUsingCDictProbe { + fn default() -> Self { + Self { + events: Vec::new(), + attach_cdict: ptr::null(), + attach_params: ptr::null(), + attach_pledged_src_size: 0, + attach_zbuff: 0, + attach_result: 0, + copy_cdict: ptr::null(), + copy_params: ptr::null(), + copy_pledged_src_size: 0, + copy_zbuff: 0, + copy_result: 0, + } + } + } + + unsafe fn reset_using_cdict_probe(context: *mut c_void) -> &'static mut ResetUsingCDictProbe { + unsafe { &mut *context.cast::() } + } + + unsafe extern "C" fn reset_using_cdict_attach( + context: *mut c_void, + cdict: *const c_void, + params: *const c_void, + pledged_src_size: u64, + zbuff: c_int, + ) -> usize { + let probe = unsafe { reset_using_cdict_probe(context) }; + probe.events.push("attach"); + probe.attach_cdict = cdict; + probe.attach_params = params; + probe.attach_pledged_src_size = pledged_src_size; + probe.attach_zbuff = zbuff; + probe.attach_result + } + + unsafe extern "C" fn reset_using_cdict_copy( + context: *mut c_void, + cdict: *const c_void, + params: *const c_void, + pledged_src_size: u64, + zbuff: c_int, + ) -> usize { + let probe = unsafe { reset_using_cdict_probe(context) }; + probe.events.push("copy"); + probe.copy_cdict = cdict; + probe.copy_params = params; + probe.copy_pledged_src_size = pledged_src_size; + probe.copy_zbuff = zbuff; + probe.copy_result + } + + fn reset_using_cdict_test_state( + probe: &mut ResetUsingCDictProbe, + cdict: *const c_void, + params: *const c_void, + strategy: &c_int, + dedicated_dict_search: &c_int, + attach_dict_pref: &c_int, + force_window: &c_int, + pledged_src_size: &u64, + zbuff: &c_int, + ) -> ZSTD_rust_resetCCtxUsingCDictState { + ZSTD_rust_resetCCtxUsingCDictState { + callback_context: (probe as *mut ResetUsingCDictProbe).cast(), + cdict, + params, + cdict_strategy: strategy, + dedicated_dict_search, + attach_dict_pref, + force_window, + pledged_src_size, + zbuff, + attach: Some(reset_using_cdict_attach), + copy: Some(reset_using_cdict_copy), + } + } + + #[test] + fn reset_using_cdict_selects_attach_at_policy_cutoff() { + let mut probe = ResetUsingCDictProbe::default(); + let cdict = 0x4000usize as *const c_void; + let params = 0x3000usize as *const c_void; + let strategy = 1; + let dedicated_dict_search = 0; + let attach_dict_pref = 0; + let force_window = 0; + let pledged_src_size = 8 * 1024; + let zbuff = 9; + let state = reset_using_cdict_test_state( + &mut probe, + cdict, + params, + &strategy, + &dedicated_dict_search, + &attach_dict_pref, + &force_window, + &pledged_src_size, + &zbuff, + ); + + let result = unsafe { ZSTD_rust_resetCCtxUsingCDict(&state) }; + + assert_eq!(result, 0); + assert_eq!(probe.events, ["attach"]); + assert_eq!(probe.attach_cdict, cdict); + assert_eq!(probe.attach_params, params); + assert_eq!(probe.attach_pledged_src_size, pledged_src_size); + assert_eq!(probe.attach_zbuff, zbuff); + assert!(probe.copy_cdict.is_null()); + } + + #[test] + fn reset_using_cdict_selects_copy_and_propagates_its_error() { + let mut probe = ResetUsingCDictProbe { + copy_result: ERROR(ZstdErrorCode::MemoryAllocation), + ..Default::default() + }; + let cdict = 0x4000usize as *const c_void; + let params = 0x3000usize as *const c_void; + let strategy = 1; + let dedicated_dict_search = 0; + let attach_dict_pref = 0; + let force_window = 0; + let pledged_src_size = 8 * 1024 + 1; + let zbuff = 9; + let state = reset_using_cdict_test_state( + &mut probe, + cdict, + params, + &strategy, + &dedicated_dict_search, + &attach_dict_pref, + &force_window, + &pledged_src_size, + &zbuff, + ); + + let result = unsafe { ZSTD_rust_resetCCtxUsingCDict(&state) }; + + assert_eq!(result, probe.copy_result); + assert_eq!(probe.events, ["copy"]); + assert_eq!(probe.copy_cdict, cdict); + assert_eq!(probe.copy_params, params); + assert_eq!(probe.copy_pledged_src_size, pledged_src_size); + assert_eq!(probe.copy_zbuff, zbuff); + assert!(probe.attach_cdict.is_null()); + } + + #[test] + fn reset_using_cdict_dedicated_search_overrides_force_copy() { + let mut probe = ResetUsingCDictProbe::default(); + let strategy = 1; + let dedicated_dict_search = 1; + let attach_dict_pref = 2; + let force_window = 1; + let pledged_src_size = u64::MAX; + let zbuff = 9; + let state = reset_using_cdict_test_state( + &mut probe, + 0x4000usize as *const c_void, + 0x3000usize as *const c_void, + &strategy, + &dedicated_dict_search, + &attach_dict_pref, + &force_window, + &pledged_src_size, + &zbuff, + ); + + let result = unsafe { ZSTD_rust_resetCCtxUsingCDict(&state) }; + + assert_eq!(result, 0); + assert_eq!(probe.events, ["attach"]); + } + unsafe fn cctx_policy_probe(context: *mut c_void) -> &'static mut CctxPolicyProbe { unsafe { &mut *context.cast::() } }