From 6c060c901f483279ab5cb780aa9acab3f852c46a Mon Sep 17 00:00:00 2001 From: ddidderr Date: Mon, 20 Jul 2026 03:32:56 +0200 Subject: [PATCH] refactor(compress): move CCtx reset orchestration to Rust Move the residual ZSTD_resetCCtx_internal sequencing into a Rust policy entrypoint. The Rust side now owns the plan-to-workspace-to-tail order and scalar hand-off, while C retains private workspace checks, context initialization, match-state reset, and pointer publication callbacks. Preserve error short-circuiting and the existing static, dynamic, LDM, and index-reset behavior. Test Plan: - cargo fmt --manifest-path rust/Cargo.toml --all -- --check - git diff --check - Full capped Rust, native, CLI, and upstream test suites to follow --- lib/compress/zstd_compress.c | 172 ++++++++++++++++++---------- rust/src/zstd_compress.rs | 210 +++++++++++++++++++++++++++++++++++ 2 files changed, 323 insertions(+), 59 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 959dcee95..88ef4d486 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1551,6 +1551,11 @@ typedef size_t (*ZSTD_rust_resetCCtxWorkspaceCreate_f)( typedef void* (*ZSTD_rust_resetCCtxWorkspaceReserveObject_f)( void* context, size_t size); typedef size_t (*ZSTD_rust_resetCCtxTailCallback_f)(void* context); +typedef void (*ZSTD_rust_resetCCtxWorkspacePrepare_f)( + void* context, size_t neededSpace, size_t windowSize, + size_t blockSize, int* workspaceTooSmall, int* workspaceWasteful); +typedef void (*ZSTD_rust_resetCCtxTailPrepare_f)( + void* context, size_t blockSize, int indexResetPolicy); typedef struct { void* callbackContext; int ldmEnable; @@ -1681,6 +1686,31 @@ typedef char ZSTD_rust_reset_cctx_tail_state_layout[ == offsetof(ZSTD_rust_resetCCtxTailState, resetStorage) + sizeof(void*)) ? 1 : -1]; +typedef struct { + void* callbackContext; + const ZSTD_rustCCtxResetState* resetState; + ZSTD_rust_resetCCtxStorageState* storageState; + ZSTD_rust_resetCCtxWorkspaceState* workspaceState; + ZSTD_rust_resetCCtxTailState* tailState; + ZSTD_rust_resetCCtxWorkspacePrepare_f prepareWorkspace; + ZSTD_rust_resetCCtxTailPrepare_f prepareTail; +} ZSTD_rust_resetCCtxInternalState; +typedef char ZSTD_rust_reset_cctx_internal_state_layout[ + (offsetof(ZSTD_rust_resetCCtxInternalState, callbackContext) == 0 + && offsetof(ZSTD_rust_resetCCtxInternalState, resetState) + == sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxInternalState, storageState) + == 2 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxInternalState, workspaceState) + == 3 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxInternalState, tailState) + == 4 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxInternalState, prepareWorkspace) + == 5 * sizeof(void*) + && offsetof(ZSTD_rust_resetCCtxInternalState, prepareTail) + == 6 * sizeof(void*) + && sizeof(ZSTD_rust_resetCCtxInternalState) == 7 * sizeof(void*)) + ? 1 : -1]; enum { ZSTD_RUST_RESET_CCTX_RESERVE_ALIGNED64 = 0, ZSTD_RUST_RESET_CCTX_RESERVE_BUFFER = 1 @@ -1718,6 +1748,8 @@ size_t ZSTD_rust_resetCCtxStorage(const ZSTD_rust_resetCCtxStorageState* state); size_t ZSTD_rust_resetCCtxWorkspace( const ZSTD_rust_resetCCtxWorkspaceState* state); size_t ZSTD_rust_resetCCtxTail(const ZSTD_rust_resetCCtxTailState* state); +size_t ZSTD_rust_resetCCtxInternal( + const ZSTD_rust_resetCCtxInternalState* state); size_t ZSTD_rust_estimateCCtxWorkspaceSize( ZSTD_compressionParameters cParams, int ldmEnable, U32 ldmHashLog, U32 ldmBucketSizeLog, @@ -4431,8 +4463,38 @@ typedef struct { ZSTD_compResetPolicy_e compResetPolicy; ZSTD_indexResetPolicy_e indexResetPolicy; ZSTD_rust_resetCCtxStorageState* storageState; + ZSTD_rust_resetCCtxTailState* tailState; } ZSTD_rust_resetCCtxTailContext; +static void ZSTD_rust_resetCCtxWorkspace_prepare( + void* opaque, size_t neededSpace, size_t windowSize, + size_t blockSize, int* workspaceTooSmall, int* workspaceWasteful) +{ + ZSTD_rust_resetCCtxTailContext* const context = + (ZSTD_rust_resetCCtxTailContext*)opaque; + ZSTD_cwksp* const ws = context->ws; + *workspaceTooSmall = ZSTD_cwksp_sizeof(ws) < neededSpace; + *workspaceWasteful = ZSTD_cwksp_check_wasteful(ws, neededSpace); + DEBUGLOG(4, "Need %zu B workspace", neededSpace); + DEBUGLOG(4, "windowSize: %zu - blockSize: %zu", + windowSize, blockSize); + if (*workspaceTooSmall || *workspaceWasteful) { + DEBUGLOG(4, "Resize workspaceSize from %zuKB to %zuKB", + ZSTD_cwksp_sizeof(ws) >> 10, neededSpace >> 10); + } +} + +static void ZSTD_rust_resetCCtxTail_prepare( + void* opaque, size_t blockSize, int indexResetPolicy) +{ + ZSTD_rust_resetCCtxTailContext* const context = + (ZSTD_rust_resetCCtxTailContext*)opaque; + context->blockSize = blockSize; + context->indexResetPolicy = (ZSTD_indexResetPolicy_e)indexResetPolicy; + context->tailState->compressedBlockState = + context->cctx->blockState.prevCBlock; +} + static void ZSTD_rust_resetCCtxTail_initialize(void* opaque) { ZSTD_rust_resetCCtxTailContext* const context = @@ -4529,15 +4591,13 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, }; ZSTD_rustCCtxResetPlan resetPlan; ZSTD_rustCCtxResetState resetState; - size_t blockSize; - size_t maxNbSeq; - size_t buffOutSize; - size_t buffInSize; - size_t maxNbLdmSeq; - size_t neededSpace; - ZSTD_indexResetPolicy_e needsIndexReset; + ZSTD_indexResetPolicy_e needsIndexReset = ZSTDirp_continue; ZSTD_rust_resetCCtxStorageContext storageContext; ZSTD_rust_resetCCtxStorageState storageState; + ZSTD_rust_resetCCtxWorkspaceState workspaceState; + ZSTD_rust_resetCCtxTailContext tailContext; + ZSTD_rust_resetCCtxTailState tailState; + ZSTD_rust_resetCCtxInternalState internalState; resetState.cParams = params->cParams; resetState.ldmEnable = (int)params->ldmParams.enableLdm; resetState.ldmHashLog = params->ldmParams.hashLog; @@ -4558,15 +4618,6 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, resetState.sizing = &sizing; resetState.plan = &resetPlan; - FORWARD_IF_ERROR(ZSTD_rust_planCCtxReset(&resetState), "cctx reset plan failed!"); - - blockSize = resetPlan.blockSize; - maxNbSeq = resetPlan.maxNbSeq; - buffOutSize = resetPlan.buffOutSize; - buffInSize = resetPlan.buffInSize; - maxNbLdmSeq = resetPlan.maxNbLdmSeq; - neededSpace = resetPlan.neededSpace; - needsIndexReset = (ZSTD_indexResetPolicy_e)resetPlan.needsIndexReset; storageContext.cctx = zc; storageContext.ws = ws; storageState.callbackContext = &storageContext; @@ -4574,12 +4625,12 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, storageState.hasExtSeqProd = ZSTD_hasExtSeqProd(params); storageState.hashLog = params->ldmParams.hashLog; storageState.bucketSizeLog = params->ldmParams.bucketSizeLog; - storageState.blockSize = blockSize; - storageState.maxNbSeq = maxNbSeq; - storageState.maxNbLdmSeq = maxNbLdmSeq; - storageState.maxNbExternalSeq = resetPlan.maxNbExternalSeq; - storageState.buffInSize = buffInSize; - storageState.buffOutSize = buffOutSize; + storageState.blockSize = 0; + storageState.maxNbSeq = 0; + storageState.maxNbLdmSeq = 0; + storageState.maxNbExternalSeq = 0; + storageState.buffInSize = 0; + storageState.buffOutSize = 0; storageState.seqDefSize = sizeof(SeqDef); storageState.ldmEntrySize = sizeof(ldmEntry_t); storageState.rawSeqSize = sizeof(rawSeq); @@ -4597,46 +4648,49 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, storageState.resetExternalSequences = ZSTD_rust_resetCCtxStorage_resetExternalSequences; - { ZSTD_rust_resetCCtxWorkspaceState workspaceState; - workspaceState.callbackContext = &storageContext; - workspaceState.isStatic = zc->staticSize != 0; - workspaceState.workspaceTooSmall = ZSTD_cwksp_sizeof(ws) < neededSpace; - workspaceState.workspaceWasteful = ZSTD_cwksp_check_wasteful(ws, neededSpace); - workspaceState.neededSpace = neededSpace; - workspaceState.compressedBlockStateSize = sizeof(ZSTD_compressedBlockState_t); - workspaceState.tmpWorkspaceSize = TMP_WORKSPACE_SIZE; - workspaceState.needsIndexReset = (int*)&needsIndexReset; - workspaceState.bumpOversizedDuration = - ZSTD_rust_resetCCtxWorkspace_bumpOversizedDuration; - workspaceState.freeWorkspace = ZSTD_rust_resetCCtxWorkspace_free; - workspaceState.createWorkspace = ZSTD_rust_resetCCtxWorkspace_create; - workspaceState.reserveObject = ZSTD_rust_resetCCtxWorkspace_reserveObject; - workspaceState.setPointer = ZSTD_rust_resetCCtxStorage_setPointer; - workspaceState.setSize = ZSTD_rust_resetCCtxStorage_setSize; - workspaceState.clearWorkspace = ZSTD_rust_resetCCtxWorkspace_clear; - FORWARD_IF_ERROR(ZSTD_rust_resetCCtxWorkspace(&workspaceState), ""); - } + workspaceState.callbackContext = &storageContext; + workspaceState.isStatic = zc->staticSize != 0; + workspaceState.workspaceTooSmall = 0; + workspaceState.workspaceWasteful = 0; + workspaceState.neededSpace = 0; + workspaceState.compressedBlockStateSize = sizeof(ZSTD_compressedBlockState_t); + workspaceState.tmpWorkspaceSize = TMP_WORKSPACE_SIZE; + workspaceState.needsIndexReset = (int*)&needsIndexReset; + workspaceState.bumpOversizedDuration = + ZSTD_rust_resetCCtxWorkspace_bumpOversizedDuration; + workspaceState.freeWorkspace = ZSTD_rust_resetCCtxWorkspace_free; + workspaceState.createWorkspace = ZSTD_rust_resetCCtxWorkspace_create; + workspaceState.reserveObject = ZSTD_rust_resetCCtxWorkspace_reserveObject; + workspaceState.setPointer = ZSTD_rust_resetCCtxStorage_setPointer; + workspaceState.setSize = ZSTD_rust_resetCCtxStorage_setSize; + workspaceState.clearWorkspace = ZSTD_rust_resetCCtxWorkspace_clear; - { ZSTD_rust_resetCCtxTailContext tailContext; - ZSTD_rust_resetCCtxTailState tailState; - tailContext.cctx = zc; - tailContext.params = params; - tailContext.blockSize = blockSize; - tailContext.pledgedSrcSize = pledgedSrcSize; - tailContext.ws = ws; - tailContext.compResetPolicy = crp; - tailContext.indexResetPolicy = needsIndexReset; - tailContext.storageState = &storageState; - tailState.callbackContext = &tailContext; - tailState.initialize = ZSTD_rust_resetCCtxTail_initialize; - tailState.compressedBlockState = zc->blockState.prevCBlock; - tailState.resetMatchState = ZSTD_rust_resetCCtxTail_resetMatchState; - tailState.resetStorage = ZSTD_rust_resetCCtxTail_resetStorage; - FORWARD_IF_ERROR(ZSTD_rust_resetCCtxTail(&tailState), ""); - } + tailContext.cctx = zc; + tailContext.params = params; + tailContext.blockSize = 0; + tailContext.pledgedSrcSize = pledgedSrcSize; + tailContext.ws = ws; + tailContext.compResetPolicy = crp; + tailContext.indexResetPolicy = needsIndexReset; + tailContext.storageState = &storageState; + tailContext.tailState = &tailState; + tailState.callbackContext = &tailContext; + tailState.initialize = ZSTD_rust_resetCCtxTail_initialize; + tailState.compressedBlockState = NULL; + tailState.resetMatchState = ZSTD_rust_resetCCtxTail_resetMatchState; + tailState.resetStorage = ZSTD_rust_resetCCtxTail_resetStorage; + + internalState.callbackContext = &tailContext; + internalState.resetState = &resetState; + internalState.storageState = &storageState; + internalState.workspaceState = &workspaceState; + internalState.tailState = &tailState; + internalState.prepareWorkspace = ZSTD_rust_resetCCtxWorkspace_prepare; + internalState.prepareTail = ZSTD_rust_resetCCtxTail_prepare; + FORWARD_IF_ERROR(ZSTD_rust_resetCCtxInternal(&internalState), ""); DEBUGLOG(3, "wksp: finished allocating, %zd bytes remain available", ZSTD_cwksp_available_space(ws)); - assert(ZSTD_cwksp_estimated_space_within_bounds(ws, neededSpace)); + assert(ZSTD_cwksp_estimated_space_within_bounds(ws, resetPlan.neededSpace)); return 0; } diff --git a/rust/src/zstd_compress.rs b/rust/src/zstd_compress.rs index 7635908eb..edda9dbdc 100644 --- a/rust/src/zstd_compress.rs +++ b/rust/src/zstd_compress.rs @@ -7619,6 +7619,155 @@ const _: () = { ); }; +type ResetCCtxWorkspacePrepare = + unsafe extern "C" fn(*mut c_void, usize, usize, usize, *mut c_int, *mut c_int); +type ResetCCtxTailPrepare = unsafe extern "C" fn(*mut c_void, usize, c_int); + +/// State for composing the already-projected CCtx reset policies. +/// +/// Rust owns the plan/workspace/tail order and the scalar hand-off between +/// those helpers. C retains the workspace checks, context initialization, +/// match-state reset, and private pointer publication behind callbacks. +#[repr(C)] +pub struct ZSTD_rust_resetCCtxInternalState { + pub callbackContext: *mut c_void, + pub resetState: *const ZSTD_rustCCtxResetState, + pub storageState: *mut ZSTD_rust_resetCCtxStorageState, + pub workspaceState: *mut ZSTD_rust_resetCCtxWorkspaceState, + pub tailState: *mut ZSTD_rust_resetCCtxTailState, + pub prepareWorkspace: Option, + pub prepareTail: Option, +} + +const _: () = { + assert!(size_of::() == size_of::()); + assert!(size_of::() == size_of::()); + assert!(offset_of!(ZSTD_rust_resetCCtxInternalState, callbackContext) == 0); + assert!(offset_of!(ZSTD_rust_resetCCtxInternalState, resetState) == size_of::()); + assert!(offset_of!(ZSTD_rust_resetCCtxInternalState, storageState) == 2 * size_of::()); + assert!(offset_of!(ZSTD_rust_resetCCtxInternalState, workspaceState) == 3 * size_of::()); + assert!(offset_of!(ZSTD_rust_resetCCtxInternalState, tailState) == 4 * size_of::()); + assert!( + offset_of!(ZSTD_rust_resetCCtxInternalState, prepareWorkspace) == 5 * size_of::() + ); + assert!(offset_of!(ZSTD_rust_resetCCtxInternalState, prepareTail) == 6 * size_of::()); + assert!(size_of::() == 7 * size_of::()); +}; + +#[inline] +fn project_cctx_reset_plan( + plan: &ZSTD_rustCCtxResetPlan, + storage_state: &mut ZSTD_rust_resetCCtxStorageState, + workspace_state: &mut ZSTD_rust_resetCCtxWorkspaceState, +) { + storage_state.blockSize = plan.blockSize; + storage_state.maxNbSeq = plan.maxNbSeq; + storage_state.maxNbLdmSeq = plan.maxNbLdmSeq; + storage_state.maxNbExternalSeq = plan.maxNbExternalSeq; + storage_state.buffInSize = plan.buffInSize; + storage_state.buffOutSize = plan.buffOutSize; + workspace_state.neededSpace = plan.neededSpace; +} + +#[inline] +fn run_cctx_reset_policy( + plan: impl FnOnce() -> usize, + prepare_workspace: impl FnOnce(), + reset_workspace: impl FnOnce() -> usize, + prepare_tail: impl FnOnce(), + reset_tail: impl FnOnce() -> usize, +) -> usize { + let result = plan(); + if ERR_isError(result) { + return result; + } + prepare_workspace(); + let result = reset_workspace(); + if ERR_isError(result) { + return result; + } + prepare_tail(); + reset_tail() +} + +/// Compose the private CCtx reset without moving C-only layout operations. +#[no_mangle] +pub unsafe extern "C" fn ZSTD_rust_resetCCtxInternal( + state: *const ZSTD_rust_resetCCtxInternalState, +) -> usize { + if state.is_null() { + return ERROR(ZstdErrorCode::Generic); + } + let state = unsafe { &*state }; + let Some(prepare_workspace) = state.prepareWorkspace else { + return ERROR(ZstdErrorCode::Generic); + }; + let Some(prepare_tail) = state.prepareTail else { + return ERROR(ZstdErrorCode::Generic); + }; + if state.callbackContext.is_null() + || state.resetState.is_null() + || state.storageState.is_null() + || state.workspaceState.is_null() + || state.tailState.is_null() + { + return ERROR(ZstdErrorCode::Generic); + } + + let reset_state = unsafe { &*state.resetState }; + if reset_state.plan.is_null() { + return ERROR(ZstdErrorCode::Generic); + } + let workspace_state = unsafe { &*state.workspaceState }; + if workspace_state.needsIndexReset.is_null() { + return ERROR(ZstdErrorCode::Generic); + } + + let plan = reset_state.plan; + let reset_state_ptr = state.resetState; + let storage_state_ptr = state.storageState; + let workspace_state_ptr = state.workspaceState; + let tail_state_ptr = state.tailState; + let callback_context = state.callbackContext; + + run_cctx_reset_policy( + || unsafe { + let result = ZSTD_rust_planCCtxReset(reset_state_ptr); + if ERR_isError(result) { + return result; + } + let plan = &*plan; + let storage_state = &mut *storage_state_ptr; + let workspace_state = &mut *workspace_state_ptr; + project_cctx_reset_plan(plan, storage_state, workspace_state); + *workspace_state.needsIndexReset = plan.needsIndexReset; + 0 + }, + || unsafe { + let plan = &*plan; + let workspace_state = &mut *workspace_state_ptr; + prepare_workspace( + callback_context, + plan.neededSpace, + plan.windowSize, + plan.blockSize, + &mut workspace_state.workspaceTooSmall, + &mut workspace_state.workspaceWasteful, + ); + }, + || unsafe { ZSTD_rust_resetCCtxWorkspace(workspace_state_ptr) }, + || unsafe { + let workspace_state = &*workspace_state_ptr; + prepare_tail( + callback_context, + (&*plan).blockSize, + *workspace_state.needsIndexReset, + ); + }, + || unsafe { ZSTD_rust_resetCCtxTail(tail_state_ptr) }, + ) +} + /// Reserve and publish the private CCtx storage that follows match-state reset. #[no_mangle] pub unsafe extern "C" fn ZSTD_rust_resetCCtxStorage( @@ -17882,6 +18031,67 @@ mod tests { ); } + #[test] + fn cctx_reset_policy_preserves_plan_workspace_tail_order() { + let events = std::cell::RefCell::new(Vec::new()); + let result = run_cctx_reset_policy( + || { + events.borrow_mut().push("plan"); + 0 + }, + || events.borrow_mut().push("prepare-workspace"), + || { + events.borrow_mut().push("workspace"); + 0 + }, + || events.borrow_mut().push("prepare-tail"), + || { + events.borrow_mut().push("tail"); + 0 + }, + ); + + assert_eq!(result, 0); + assert_eq!( + events.into_inner(), + [ + "plan", + "prepare-workspace", + "workspace", + "prepare-tail", + "tail" + ] + ); + } + + #[test] + fn cctx_reset_policy_propagates_workspace_error_before_tail() { + let events = std::cell::RefCell::new(Vec::new()); + let workspace_error = ERROR(ZstdErrorCode::MemoryAllocation); + let result = run_cctx_reset_policy( + || { + events.borrow_mut().push("plan"); + 0 + }, + || events.borrow_mut().push("prepare-workspace"), + || { + events.borrow_mut().push("workspace"); + workspace_error + }, + || events.borrow_mut().push("prepare-tail"), + || { + events.borrow_mut().push("tail"); + 0 + }, + ); + + assert_eq!(result, workspace_error); + assert_eq!( + events.into_inner(), + ["plan", "prepare-workspace", "workspace"] + ); + } + #[derive(Debug, PartialEq)] enum ResetCCtxStorageTestEvent { Reserve(c_int, usize),