From 01474bf73b357fe0c7bcf51f5cd41928462e2488 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 25 Oct 2024 15:51:03 -0700 Subject: [PATCH 1/7] add internal compression parameter preBlockSplitter_level not yet exposed to the interface. Also: renames `useBlockSplitter` to `postBlockSplitter` to better qualify the difference between the 2 settings. --- lib/compress/zstd_compress.c | 52 +++++++++++++++++---------- lib/compress/zstd_compress_internal.h | 20 ++++++++--- 2 files changed, 48 insertions(+), 24 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 5f8f5aba4..05edcd201 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -323,7 +323,7 @@ static ZSTD_CCtx_params ZSTD_makeCCtxParamsFromCParams( assert(cctxParams.ldmParams.hashLog >= cctxParams.ldmParams.bucketSizeLog); assert(cctxParams.ldmParams.hashRateLog < 32); } - cctxParams.useBlockSplitter = ZSTD_resolveBlockSplitterMode(cctxParams.useBlockSplitter, &cParams); + cctxParams.postBlockSplitter = ZSTD_resolveBlockSplitterMode(cctxParams.postBlockSplitter, &cParams); cctxParams.useRowMatchFinder = ZSTD_resolveRowMatchFinderMode(cctxParams.useRowMatchFinder, &cParams); cctxParams.validateSequences = ZSTD_resolveExternalSequenceValidation(cctxParams.validateSequences); cctxParams.maxBlockSize = ZSTD_resolveMaxBlockSize(cctxParams.maxBlockSize); @@ -391,13 +391,13 @@ ZSTD_CCtxParams_init_internal(ZSTD_CCtx_params* cctxParams, */ cctxParams->compressionLevel = compressionLevel; cctxParams->useRowMatchFinder = ZSTD_resolveRowMatchFinderMode(cctxParams->useRowMatchFinder, ¶ms->cParams); - cctxParams->useBlockSplitter = ZSTD_resolveBlockSplitterMode(cctxParams->useBlockSplitter, ¶ms->cParams); + cctxParams->postBlockSplitter = ZSTD_resolveBlockSplitterMode(cctxParams->postBlockSplitter, ¶ms->cParams); cctxParams->ldmParams.enableLdm = ZSTD_resolveEnableLdm(cctxParams->ldmParams.enableLdm, ¶ms->cParams); cctxParams->validateSequences = ZSTD_resolveExternalSequenceValidation(cctxParams->validateSequences); cctxParams->maxBlockSize = ZSTD_resolveMaxBlockSize(cctxParams->maxBlockSize); cctxParams->searchForExternalRepcodes = ZSTD_resolveExternalRepcodeSearch(cctxParams->searchForExternalRepcodes, compressionLevel); DEBUGLOG(4, "ZSTD_CCtxParams_init_internal: useRowMatchFinder=%d, useBlockSplitter=%d ldm=%d", - cctxParams->useRowMatchFinder, cctxParams->useBlockSplitter, cctxParams->ldmParams.enableLdm); + cctxParams->useRowMatchFinder, cctxParams->postBlockSplitter, cctxParams->ldmParams.enableLdm); } size_t ZSTD_CCtxParams_init_advanced(ZSTD_CCtx_params* cctxParams, ZSTD_parameters params) @@ -977,8 +977,8 @@ size_t ZSTD_CCtxParams_setParameter(ZSTD_CCtx_params* CCtxParams, case ZSTD_c_useBlockSplitter: BOUNDCHECK(ZSTD_c_useBlockSplitter, value); - CCtxParams->useBlockSplitter = (ZSTD_paramSwitch_e)value; - return CCtxParams->useBlockSplitter; + CCtxParams->postBlockSplitter = (ZSTD_paramSwitch_e)value; + return CCtxParams->postBlockSplitter; case ZSTD_c_useRowMatchFinder: BOUNDCHECK(ZSTD_c_useRowMatchFinder, value); @@ -1136,7 +1136,7 @@ size_t ZSTD_CCtxParams_getParameter( *value = (int)CCtxParams->validateSequences; break; case ZSTD_c_useBlockSplitter : - *value = (int)CCtxParams->useBlockSplitter; + *value = (int)CCtxParams->postBlockSplitter; break; case ZSTD_c_useRowMatchFinder : *value = (int)CCtxParams->useRowMatchFinder; @@ -2111,7 +2111,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, params = &zc->appliedParams; assert(params->useRowMatchFinder != ZSTD_ps_auto); - assert(params->useBlockSplitter != ZSTD_ps_auto); + assert(params->postBlockSplitter != ZSTD_ps_auto); assert(params->ldmParams.enableLdm != ZSTD_ps_auto); assert(params->maxBlockSize != 0); if (params->ldmParams.enableLdm == ZSTD_ps_enable) { @@ -2517,10 +2517,10 @@ static size_t ZSTD_copyCCtx_internal(ZSTD_CCtx* dstCCtx, /* Copy only compression parameters related to tables. */ params.cParams = srcCCtx->appliedParams.cParams; assert(srcCCtx->appliedParams.useRowMatchFinder != ZSTD_ps_auto); - assert(srcCCtx->appliedParams.useBlockSplitter != ZSTD_ps_auto); + assert(srcCCtx->appliedParams.postBlockSplitter != ZSTD_ps_auto); assert(srcCCtx->appliedParams.ldmParams.enableLdm != ZSTD_ps_auto); params.useRowMatchFinder = srcCCtx->appliedParams.useRowMatchFinder; - params.useBlockSplitter = srcCCtx->appliedParams.useBlockSplitter; + params.postBlockSplitter = srcCCtx->appliedParams.postBlockSplitter; params.ldmParams = srcCCtx->appliedParams.ldmParams; params.fParams = fParams; params.maxBlockSize = srcCCtx->appliedParams.maxBlockSize; @@ -2728,9 +2728,9 @@ static int ZSTD_useTargetCBlockSize(const ZSTD_CCtx_params* cctxParams) * Returns 1 if true, 0 otherwise. */ static int ZSTD_blockSplitterEnabled(ZSTD_CCtx_params* cctxParams) { - DEBUGLOG(5, "ZSTD_blockSplitterEnabled (useBlockSplitter=%d)", cctxParams->useBlockSplitter); - assert(cctxParams->useBlockSplitter != ZSTD_ps_auto); - return (cctxParams->useBlockSplitter == ZSTD_ps_enable); + DEBUGLOG(5, "ZSTD_blockSplitterEnabled (postBlockSplitter=%d)", cctxParams->postBlockSplitter); + assert(cctxParams->postBlockSplitter != ZSTD_ps_auto); + return (cctxParams->postBlockSplitter == ZSTD_ps_enable); } /* Type returned by ZSTD_buildSequencesStatistics containing finalized symbol encoding types @@ -4300,7 +4300,7 @@ ZSTD_compressBlock_splitBlock(ZSTD_CCtx* zc, U32 nbSeq; size_t cSize; DEBUGLOG(4, "ZSTD_compressBlock_splitBlock"); - assert(zc->appliedParams.useBlockSplitter == ZSTD_ps_enable); + assert(zc->appliedParams.postBlockSplitter == ZSTD_ps_enable); { const size_t bss = ZSTD_buildSeqStore(zc, src, srcSize); FORWARD_IF_ERROR(bss, "ZSTD_buildSeqStore failed"); @@ -4491,7 +4491,7 @@ static void ZSTD_overflowCorrectIfNeeded(ZSTD_matchState_t* ms, #include "zstd_preSplit.h" -static size_t ZSTD_optimalBlockSize(ZSTD_CCtx* cctx, const void* src, size_t srcSize, size_t blockSizeMax, ZSTD_strategy strat, S64 savings) +static size_t ZSTD_optimalBlockSize(ZSTD_CCtx* cctx, const void* src, size_t srcSize, size_t blockSizeMax, int splitLevel, ZSTD_strategy strat, S64 savings) { /* split level based on compression strategy, from `fast` to `btultra2` */ static const int splitLevels[] = { 0, 0, 1, 2, 2, 3, 3, 4, 4, 4 }; @@ -4506,9 +4506,18 @@ static size_t ZSTD_optimalBlockSize(ZSTD_CCtx* cctx, const void* src, size_t src * Note: as a consequence, the first full block is not split. */ if (savings < 3) return 128 KB; - /* dynamic splitting has a cpu cost for analysis, - * select a variant among multiple gradual speed/accuracy tradeoffs */ - return ZSTD_splitBlock(src, blockSizeMax, splitLevels[strat], cctx->tmpWorkspace, cctx->tmpWkspSize); + /* apply @splitLevel, or use default value (which depends on @strat). + * note that splitting heuristic is still conditioned by @savings >= 3, + * so the first block will not reach this code path */ + if (splitLevel == 1) return 128 KB; + if (splitLevel == 0) { + assert(ZSTD_fast <= strat && strat <= ZSTD_btultra2); + splitLevel = splitLevels[strat]; + } else { + assert(2 <= splitLevel && splitLevel <= 6); + splitLevel -= 2; + } + return ZSTD_splitBlock(src, blockSizeMax, splitLevel, cctx->tmpWorkspace, cctx->tmpWkspSize); } /*! ZSTD_compress_frameChunk() : @@ -4539,7 +4548,12 @@ static size_t ZSTD_compress_frameChunk(ZSTD_CCtx* cctx, while (remaining) { ZSTD_matchState_t* const ms = &cctx->blockState.matchState; - size_t const blockSize = ZSTD_optimalBlockSize(cctx, ip, remaining, blockSizeMax, cctx->appliedParams.cParams.strategy, savings); + size_t const blockSize = ZSTD_optimalBlockSize(cctx, + ip, remaining, + blockSizeMax, + cctx->appliedParams.preBlockSplitter_level, + cctx->appliedParams.cParams.strategy, + savings); U32 const lastBlock = lastFrameChunk & (blockSize == remaining); assert(blockSize <= remaining); @@ -6286,7 +6300,7 @@ static size_t ZSTD_CCtx_init_compressStream2(ZSTD_CCtx* cctx, dictSize, mode); } - params.useBlockSplitter = ZSTD_resolveBlockSplitterMode(params.useBlockSplitter, ¶ms.cParams); + params.postBlockSplitter = ZSTD_resolveBlockSplitterMode(params.postBlockSplitter, ¶ms.cParams); params.ldmParams.enableLdm = ZSTD_resolveEnableLdm(params.ldmParams.enableLdm, ¶ms.cParams); params.useRowMatchFinder = ZSTD_resolveRowMatchFinderMode(params.useRowMatchFinder, ¶ms.cParams); params.validateSequences = ZSTD_resolveExternalSequenceValidation(params.validateSequences); diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index e76bbda71..2d3510adc 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -343,8 +343,21 @@ struct ZSTD_CCtx_params_s { ZSTD_sequenceFormat_e blockDelimiters; int validateSequences; - /* Block splitting */ - ZSTD_paramSwitch_e useBlockSplitter; + /* Block splitting + * @postBlockSplitter executes split analysis after sequences are produced, + * it's more accurate but consumes more resources. + * @preBlockSplitter_level splits before knowing sequences, + * it's more approximative but also cheaper. + * Valid @preBlockSplitter_level values range from 0 to 6 (included). + * 0 means auto, 1 means do not split, + * then levels are sorted in increasing cpu budget, from 2 (fastest) to 6 (slowest). + * Highest @preBlockSplitter_level combines well with @postBlockSplitter. + */ + ZSTD_paramSwitch_e postBlockSplitter; + int preBlockSplitter_level; + + /* Adjust the max block size*/ + size_t maxBlockSize; /* Param for deciding whether to use row-based matchfinder */ ZSTD_paramSwitch_e useRowMatchFinder; @@ -368,9 +381,6 @@ struct ZSTD_CCtx_params_s { void* extSeqProdState; ZSTD_sequenceProducer_F extSeqProdFunc; - /* Adjust the max block size*/ - size_t maxBlockSize; - /* Controls repcode search in external sequence parsing */ ZSTD_paramSwitch_e searchForExternalRepcodes; }; /* typedef'd to ZSTD_CCtx_params within "zstd.h" */ From 226ae73311d1ffd0c3488d8bc6a59576d910d6d4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 25 Oct 2024 16:08:22 -0700 Subject: [PATCH 2/7] expose new parameter ZSTD_c_blockSplitter_level --- lib/compress/zstd_compress.c | 15 +++++++++++++++ lib/zstd.h | 23 +++++++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 05edcd201..fbc11349e 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -603,6 +603,11 @@ ZSTD_bounds ZSTD_cParam_getBounds(ZSTD_cParameter param) bounds.upperBound = (int)ZSTD_ps_disable; return bounds; + case ZSTD_c_blockSplitter_level: + bounds.lowerBound = 0; + bounds.upperBound = ZSTD_BLOCKSPLITTER_LEVEL_MAX; + return bounds; + case ZSTD_c_useRowMatchFinder: bounds.lowerBound = (int)ZSTD_ps_auto; bounds.upperBound = (int)ZSTD_ps_disable; @@ -669,6 +674,7 @@ static int ZSTD_isUpdateAuthorized(ZSTD_cParameter param) case ZSTD_c_minMatch: case ZSTD_c_targetLength: case ZSTD_c_strategy: + case ZSTD_c_blockSplitter_level: return 1; case ZSTD_c_format: @@ -755,6 +761,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, int value) case ZSTD_c_blockDelimiters: case ZSTD_c_validateSequences: case ZSTD_c_useBlockSplitter: + case ZSTD_c_blockSplitter_level: case ZSTD_c_useRowMatchFinder: case ZSTD_c_deterministicRefPrefix: case ZSTD_c_prefetchCDictTables: @@ -980,6 +987,11 @@ size_t ZSTD_CCtxParams_setParameter(ZSTD_CCtx_params* CCtxParams, CCtxParams->postBlockSplitter = (ZSTD_paramSwitch_e)value; return CCtxParams->postBlockSplitter; + case ZSTD_c_blockSplitter_level: + BOUNDCHECK(ZSTD_c_blockSplitter_level, value); + CCtxParams->preBlockSplitter_level = value; + return (size_t)CCtxParams->preBlockSplitter_level; + case ZSTD_c_useRowMatchFinder: BOUNDCHECK(ZSTD_c_useRowMatchFinder, value); CCtxParams->useRowMatchFinder = (ZSTD_paramSwitch_e)value; @@ -1138,6 +1150,9 @@ size_t ZSTD_CCtxParams_getParameter( case ZSTD_c_useBlockSplitter : *value = (int)CCtxParams->postBlockSplitter; break; + case ZSTD_c_blockSplitter_level : + *value = CCtxParams->preBlockSplitter_level; + break; case ZSTD_c_useRowMatchFinder : *value = (int)CCtxParams->useRowMatchFinder; break; diff --git a/lib/zstd.h b/lib/zstd.h index 3a88c74d1..c1ad26a8d 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -496,6 +496,7 @@ typedef enum { * ZSTD_c_prefetchCDictTables * ZSTD_c_enableSeqProducerFallback * ZSTD_c_maxBlockSize + * ZSTD_c_blockSplitter_level * Because they are not stable, it's necessary to define ZSTD_STATIC_LINKING_ONLY to access them. * note : never ever use experimentalParam? names directly; * also, the enums values themselves are unstable and can still change. @@ -518,7 +519,8 @@ typedef enum { ZSTD_c_experimentalParam16=1013, ZSTD_c_experimentalParam17=1014, ZSTD_c_experimentalParam18=1015, - ZSTD_c_experimentalParam19=1016 + ZSTD_c_experimentalParam19=1016, + ZSTD_c_experimentalParam20=1017 } ZSTD_cParameter; typedef struct { @@ -2236,7 +2238,6 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo * that overrides the default ZSTD_BLOCKSIZE_MAX. It cannot be used to set upper * bounds greater than ZSTD_BLOCKSIZE_MAX or bounds lower than 1KB (will make * compressBound() inaccurate). Only currently meant to be used for testing. - * */ #define ZSTD_c_maxBlockSize ZSTD_c_experimentalParam18 @@ -2264,6 +2265,24 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo */ #define ZSTD_c_searchForExternalRepcodes ZSTD_c_experimentalParam19 +/* ZSTD_c_blockSplitter_level + * note: this parameter only influences the first splitter stage, + * which is active before producing the sequences. + * ZSTD_c_useBlockSplitter influence the next splitter stage, + * which is active after sequence production, + * and is more accurate but also slower. + * Both can be combined. + * Allowed values are between 0 and 6. + * 0 means "auto", which will select a value depending on current ZSTD_c_strategy. + * 1 means no splitting. + * Then, values from 2 to 6 are sorted in increasing cpu load order. + * + * Note that currently the first block is never split, + * to ensure expansion guarantees in presence of incompressible data. + */ +#define ZSTD_BLOCKSPLITTER_LEVEL_MAX 6 +#define ZSTD_c_blockSplitter_level ZSTD_c_experimentalParam20 + /*! ZSTD_CCtx_getParameter() : * Get the requested compression parameter value, selected by enum ZSTD_cParameter, * and store it into int* value. From 37706a677c09a1051f8b02361928c475bd094e67 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 25 Oct 2024 20:29:15 -0700 Subject: [PATCH 3/7] added a test test both that the new parameter works as intended, and that the over-split protection works as intended --- lib/compress/zstd_compress.c | 7 ++-- lib/compress/zstd_preSplit.c | 1 + tests/fuzzer.c | 63 ++++++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index fbc11349e..ca9e5edff 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2114,7 +2114,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, { ZSTD_cwksp* const ws = &zc->workspace; DEBUGLOG(4, "ZSTD_resetCCtx_internal: pledgedSrcSize=%u, wlog=%u, useRowMatchFinder=%d useBlockSplitter=%d", - (U32)pledgedSrcSize, params->cParams.windowLog, (int)params->useRowMatchFinder, (int)params->useBlockSplitter); + (U32)pledgedSrcSize, params->cParams.windowLog, (int)params->useRowMatchFinder, (int)params->postBlockSplitter); assert(!ZSTD_isError(ZSTD_checkCParams(params->cParams))); zc->isFirstBlock = 1; @@ -4520,7 +4520,10 @@ static size_t ZSTD_optimalBlockSize(ZSTD_CCtx* cctx, const void* src, size_t src * require verified savings to allow pre-splitting. * Note: as a consequence, the first full block is not split. */ - if (savings < 3) return 128 KB; + if (savings < 3) { + DEBUGLOG(6, "don't attempt splitting: savings (%lli) too low", savings); + return 128 KB; + } /* apply @splitLevel, or use default value (which depends on @strat). * note that splitting heuristic is still conditioned by @savings >= 3, * so the first block will not reach this code path */ diff --git a/lib/compress/zstd_preSplit.c b/lib/compress/zstd_preSplit.c index d25773950..d820c20ac 100644 --- a/lib/compress/zstd_preSplit.c +++ b/lib/compress/zstd_preSplit.c @@ -229,6 +229,7 @@ size_t ZSTD_splitBlock(const void* blockStart, size_t blockSize, int level, void* workspace, size_t wkspSize) { + DEBUGLOG(6, "ZSTD_splitBlock (level=%i)", level); assert(0<=level && level<=4); if (level == 0) return ZSTD_splitBlock_fromBorders(blockStart, blockSize, workspace, wkspSize); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index f5a894354..e32c0130d 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -559,6 +559,67 @@ static void test_setCParams(unsigned tnb) DISPLAYLEVEL(3, "OK \n"); } +static void test_blockSplitter_incompressibleExpansionProtection(unsigned testNb, unsigned seed) +{ + DISPLAYLEVEL(3, "test%3i : Check block splitter doesn't oversplit incompressible data (seed %u): ", testNb, seed); + { ZSTD_CCtx* cctx = ZSTD_createCCtx(); + size_t const srcSize = 256 * 1024; /* needs to be at least 2 blocks */ + void* incompressible = malloc(srcSize); + size_t const dstCapacity = ZSTD_compressBound(srcSize); + void* cBuffer = malloc(dstCapacity); + size_t const chunkSize = 8 KB; + size_t const nbChunks = srcSize / chunkSize; + size_t chunkNb, cSizeNoSplit, cSizeWithSplit; + assert(cctx != NULL); + assert(incompressible != NULL); + assert(cBuffer != NULL); + + /* let's fill input with random noise (incompressible) */ + RDG_genBuffer(incompressible, srcSize, 0.0, 0.0, seed); + DISPLAYLEVEL(4, "(hash: %llx) ", XXH64(incompressible, srcSize, 0)); + + /* this pattern targets the fastest _byChunk variant's sampling (level 3). + * manually checked that, without the @savings protection, it would over-split. + */ + for (chunkNb=0; chunkNb Date: Mon, 28 Oct 2024 16:47:38 -0700 Subject: [PATCH 4/7] fixed minor conversion warning --- lib/compress/zstd_compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index ca9e5edff..ea7a45fa7 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -4521,7 +4521,7 @@ static size_t ZSTD_optimalBlockSize(ZSTD_CCtx* cctx, const void* src, size_t src * Note: as a consequence, the first full block is not split. */ if (savings < 3) { - DEBUGLOG(6, "don't attempt splitting: savings (%lli) too low", savings); + DEBUGLOG(6, "don't attempt splitting: savings (%i) too low", (int)savings); return 128 KB; } /* apply @splitLevel, or use default value (which depends on @strat). From f593ccda04f89be6fc4f966b9b816d542941771f Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 28 Oct 2024 16:57:01 -0700 Subject: [PATCH 5/7] removed trace left over --- tests/fuzzer.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index e32c0130d..c1bff7a5a 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -576,7 +576,6 @@ static void test_blockSplitter_incompressibleExpansionProtection(unsigned testNb /* let's fill input with random noise (incompressible) */ RDG_genBuffer(incompressible, srcSize, 0.0, 0.0, seed); - DISPLAYLEVEL(4, "(hash: %llx) ", XXH64(incompressible, srcSize, 0)); /* this pattern targets the fastest _byChunk variant's sampling (level 3). * manually checked that, without the @savings protection, it would over-split. From 4f93206d62f306040a8e35033312e16986d0aeab Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 29 Oct 2024 11:04:11 -0700 Subject: [PATCH 6/7] changed variable name to ZSTD_c_blockSplitterLevel suggested by @terrelln --- lib/compress/zstd_compress.c | 12 ++++++------ lib/zstd.h | 6 +++--- tests/fuzzer.c | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index ea7a45fa7..cf3e08dca 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -603,7 +603,7 @@ ZSTD_bounds ZSTD_cParam_getBounds(ZSTD_cParameter param) bounds.upperBound = (int)ZSTD_ps_disable; return bounds; - case ZSTD_c_blockSplitter_level: + case ZSTD_c_blockSplitterLevel: bounds.lowerBound = 0; bounds.upperBound = ZSTD_BLOCKSPLITTER_LEVEL_MAX; return bounds; @@ -674,7 +674,7 @@ static int ZSTD_isUpdateAuthorized(ZSTD_cParameter param) case ZSTD_c_minMatch: case ZSTD_c_targetLength: case ZSTD_c_strategy: - case ZSTD_c_blockSplitter_level: + case ZSTD_c_blockSplitterLevel: return 1; case ZSTD_c_format: @@ -761,7 +761,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, int value) case ZSTD_c_blockDelimiters: case ZSTD_c_validateSequences: case ZSTD_c_useBlockSplitter: - case ZSTD_c_blockSplitter_level: + case ZSTD_c_blockSplitterLevel: case ZSTD_c_useRowMatchFinder: case ZSTD_c_deterministicRefPrefix: case ZSTD_c_prefetchCDictTables: @@ -987,8 +987,8 @@ size_t ZSTD_CCtxParams_setParameter(ZSTD_CCtx_params* CCtxParams, CCtxParams->postBlockSplitter = (ZSTD_paramSwitch_e)value; return CCtxParams->postBlockSplitter; - case ZSTD_c_blockSplitter_level: - BOUNDCHECK(ZSTD_c_blockSplitter_level, value); + case ZSTD_c_blockSplitterLevel: + BOUNDCHECK(ZSTD_c_blockSplitterLevel, value); CCtxParams->preBlockSplitter_level = value; return (size_t)CCtxParams->preBlockSplitter_level; @@ -1150,7 +1150,7 @@ size_t ZSTD_CCtxParams_getParameter( case ZSTD_c_useBlockSplitter : *value = (int)CCtxParams->postBlockSplitter; break; - case ZSTD_c_blockSplitter_level : + case ZSTD_c_blockSplitterLevel : *value = CCtxParams->preBlockSplitter_level; break; case ZSTD_c_useRowMatchFinder : diff --git a/lib/zstd.h b/lib/zstd.h index c1ad26a8d..cc5bb9879 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -496,7 +496,7 @@ typedef enum { * ZSTD_c_prefetchCDictTables * ZSTD_c_enableSeqProducerFallback * ZSTD_c_maxBlockSize - * ZSTD_c_blockSplitter_level + * ZSTD_c_blockSplitterLevel * Because they are not stable, it's necessary to define ZSTD_STATIC_LINKING_ONLY to access them. * note : never ever use experimentalParam? names directly; * also, the enums values themselves are unstable and can still change. @@ -2265,7 +2265,7 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo */ #define ZSTD_c_searchForExternalRepcodes ZSTD_c_experimentalParam19 -/* ZSTD_c_blockSplitter_level +/* ZSTD_c_blockSplitterLevel * note: this parameter only influences the first splitter stage, * which is active before producing the sequences. * ZSTD_c_useBlockSplitter influence the next splitter stage, @@ -2281,7 +2281,7 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo * to ensure expansion guarantees in presence of incompressible data. */ #define ZSTD_BLOCKSPLITTER_LEVEL_MAX 6 -#define ZSTD_c_blockSplitter_level ZSTD_c_experimentalParam20 +#define ZSTD_c_blockSplitterLevel ZSTD_c_experimentalParam20 /*! ZSTD_CCtx_getParameter() : * Get the requested compression parameter value, selected by enum ZSTD_cParameter, diff --git a/tests/fuzzer.c b/tests/fuzzer.c index c1bff7a5a..08c2662fb 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -595,11 +595,11 @@ static void test_blockSplitter_incompressibleExpansionProtection(unsigned testNb } /* run first without splitting */ - ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockSplitter_level, 1 /* no split */); + ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockSplitterLevel, 1 /* no split */); cSizeNoSplit = ZSTD_compress2(cctx, cBuffer, dstCapacity, incompressible, srcSize); /* run with sample43 splitter, check it's still the same */ - ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockSplitter_level, 3 /* sample43, fastest _byChunk variant */); + ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockSplitterLevel, 3 /* sample43, fastest _byChunk variant */); cSizeWithSplit = ZSTD_compress2(cctx, cBuffer, dstCapacity, incompressible, srcSize); if (cSizeWithSplit != cSizeNoSplit) { From bbaba45589f233b91a310806d97ee7b4d9ee8320 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 31 Oct 2024 13:43:40 -0700 Subject: [PATCH 7/7] change experimental parameter name from ZSTD_c_useBlockSplitter to ZSTD_c_splitAfterSequences. --- lib/compress/zstd_compress.c | 12 ++++----- lib/zstd.h | 51 ++++++++++++++++++++---------------- tests/fuzz/zstd_helpers.c | 3 ++- tests/fuzzer.c | 14 +++++----- 4 files changed, 44 insertions(+), 36 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index cf3e08dca..3d78b7da3 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -598,7 +598,7 @@ ZSTD_bounds ZSTD_cParam_getBounds(ZSTD_cParameter param) bounds.upperBound = 1; return bounds; - case ZSTD_c_useBlockSplitter: + case ZSTD_c_splitAfterSequences: bounds.lowerBound = (int)ZSTD_ps_auto; bounds.upperBound = (int)ZSTD_ps_disable; return bounds; @@ -701,7 +701,7 @@ static int ZSTD_isUpdateAuthorized(ZSTD_cParameter param) case ZSTD_c_stableOutBuffer: case ZSTD_c_blockDelimiters: case ZSTD_c_validateSequences: - case ZSTD_c_useBlockSplitter: + case ZSTD_c_splitAfterSequences: case ZSTD_c_useRowMatchFinder: case ZSTD_c_deterministicRefPrefix: case ZSTD_c_prefetchCDictTables: @@ -760,7 +760,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, int value) case ZSTD_c_stableOutBuffer: case ZSTD_c_blockDelimiters: case ZSTD_c_validateSequences: - case ZSTD_c_useBlockSplitter: + case ZSTD_c_splitAfterSequences: case ZSTD_c_blockSplitterLevel: case ZSTD_c_useRowMatchFinder: case ZSTD_c_deterministicRefPrefix: @@ -982,8 +982,8 @@ size_t ZSTD_CCtxParams_setParameter(ZSTD_CCtx_params* CCtxParams, CCtxParams->validateSequences = value; return (size_t)CCtxParams->validateSequences; - case ZSTD_c_useBlockSplitter: - BOUNDCHECK(ZSTD_c_useBlockSplitter, value); + case ZSTD_c_splitAfterSequences: + BOUNDCHECK(ZSTD_c_splitAfterSequences, value); CCtxParams->postBlockSplitter = (ZSTD_paramSwitch_e)value; return CCtxParams->postBlockSplitter; @@ -1147,7 +1147,7 @@ size_t ZSTD_CCtxParams_getParameter( case ZSTD_c_validateSequences : *value = (int)CCtxParams->validateSequences; break; - case ZSTD_c_useBlockSplitter : + case ZSTD_c_splitAfterSequences : *value = (int)CCtxParams->postBlockSplitter; break; case ZSTD_c_blockSplitterLevel : diff --git a/lib/zstd.h b/lib/zstd.h index cc5bb9879..ecf837bc9 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -491,12 +491,12 @@ typedef enum { * ZSTD_c_stableOutBuffer * ZSTD_c_blockDelimiters * ZSTD_c_validateSequences - * ZSTD_c_useBlockSplitter + * ZSTD_c_blockSplitterLevel + * ZSTD_c_splitAfterSequences * ZSTD_c_useRowMatchFinder * ZSTD_c_prefetchCDictTables * ZSTD_c_enableSeqProducerFallback * ZSTD_c_maxBlockSize - * ZSTD_c_blockSplitterLevel * Because they are not stable, it's necessary to define ZSTD_STATIC_LINKING_ONLY to access them. * note : never ever use experimentalParam? names directly; * also, the enums values themselves are unstable and can still change. @@ -2150,8 +2150,32 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo */ #define ZSTD_c_validateSequences ZSTD_c_experimentalParam12 -/* ZSTD_c_useBlockSplitter - * Controlled with ZSTD_paramSwitch_e enum. +/* ZSTD_c_blockSplitterLevel + * note: this parameter only influences the first splitter stage, + * which is active before producing the sequences. + * ZSTD_c_splitAfterSequences controls the next splitter stage, + * which is active after sequence production. + * Note that both can be combined. + * Allowed values are between 0 and ZSTD_BLOCKSPLITTER_LEVEL_MAX included. + * 0 means "auto", which will select a value depending on current ZSTD_c_strategy. + * 1 means no splitting. + * Then, values from 2 to 6 are sorted in increasing cpu load order. + * + * Note that currently the first block is never split, + * to ensure expansion guarantees in presence of incompressible data. + */ +#define ZSTD_BLOCKSPLITTER_LEVEL_MAX 6 +#define ZSTD_c_blockSplitterLevel ZSTD_c_experimentalParam20 + +/* ZSTD_c_splitAfterSequences + * This is a stronger splitter algorithm, + * based on actual sequences previously produced by the selected parser. + * It's also slower, and as a consequence, mostly used for high compression levels. + * While the post-splitter does overlap with the pre-splitter, + * both can nonetheless be combined, + * notably with ZSTD_c_blockSplitterLevel at ZSTD_BLOCKSPLITTER_LEVEL_MAX, + * resulting in higher compression ratio than just one of them. + * * Default is ZSTD_ps_auto. * Set to ZSTD_ps_disable to never use block splitter. * Set to ZSTD_ps_enable to always use block splitter. @@ -2159,7 +2183,7 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo * By default, in ZSTD_ps_auto, the library will decide at runtime whether to use * block splitting based on the compression parameters. */ -#define ZSTD_c_useBlockSplitter ZSTD_c_experimentalParam13 +#define ZSTD_c_splitAfterSequences ZSTD_c_experimentalParam13 /* ZSTD_c_useRowMatchFinder * Controlled with ZSTD_paramSwitch_e enum. @@ -2265,23 +2289,6 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo */ #define ZSTD_c_searchForExternalRepcodes ZSTD_c_experimentalParam19 -/* ZSTD_c_blockSplitterLevel - * note: this parameter only influences the first splitter stage, - * which is active before producing the sequences. - * ZSTD_c_useBlockSplitter influence the next splitter stage, - * which is active after sequence production, - * and is more accurate but also slower. - * Both can be combined. - * Allowed values are between 0 and 6. - * 0 means "auto", which will select a value depending on current ZSTD_c_strategy. - * 1 means no splitting. - * Then, values from 2 to 6 are sorted in increasing cpu load order. - * - * Note that currently the first block is never split, - * to ensure expansion guarantees in presence of incompressible data. - */ -#define ZSTD_BLOCKSPLITTER_LEVEL_MAX 6 -#define ZSTD_c_blockSplitterLevel ZSTD_c_experimentalParam20 /*! ZSTD_CCtx_getParameter() : * Get the requested compression parameter value, selected by enum ZSTD_cParameter, diff --git a/tests/fuzz/zstd_helpers.c b/tests/fuzz/zstd_helpers.c index f4cb10823..c89e448d0 100644 --- a/tests/fuzz/zstd_helpers.c +++ b/tests/fuzz/zstd_helpers.c @@ -140,7 +140,8 @@ void FUZZ_setRandomParameters(ZSTD_CCtx *cctx, size_t srcSize, FUZZ_dataProducer setRand(cctx, ZSTD_c_forceMaxWindow, 0, 1, producer); setRand(cctx, ZSTD_c_literalCompressionMode, 0, 2, producer); setRand(cctx, ZSTD_c_forceAttachDict, 0, 2, producer); - setRand(cctx, ZSTD_c_useBlockSplitter, 0, 2, producer); + setRand(cctx, ZSTD_c_blockSplitterLevel, 0, ZSTD_BLOCKSPLITTER_LEVEL_MAX, producer); + setRand(cctx, ZSTD_c_splitAfterSequences, 0, 2, producer); setRand(cctx, ZSTD_c_deterministicRefPrefix, 0, 1, producer); setRand(cctx, ZSTD_c_prefetchCDictTables, 0, 2, producer); setRand(cctx, ZSTD_c_maxBlockSize, ZSTD_BLOCKSIZE_MAX_MIN, ZSTD_BLOCKSIZE_MAX, producer); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 08c2662fb..4a6ebd69c 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1420,7 +1420,7 @@ static int basicUnitTests(U32 const seed, double compressibility) CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 19)); CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_minMatch, 7)); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_useBlockSplitter, ZSTD_ps_enable)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_splitAfterSequences, ZSTD_ps_enable)); cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, data, srcSize); CHECK_Z(cSize); @@ -1737,8 +1737,8 @@ static int basicUnitTests(U32 const seed, double compressibility) { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); int value; ZSTD_compressionParameters cparams = ZSTD_getCParams(1, 0, 0); - cparams.strategy = -1; - /* Set invalid cParams == no change. */ + cparams.strategy = (ZSTD_strategy)-1; /* set invalid value, on purpose */ + /* Set invalid cParams == error out, and no change. */ CHECK(ZSTD_isError(ZSTD_CCtx_setCParams(cctx, cparams))); CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_windowLog, &value)); @@ -1801,12 +1801,12 @@ static int basicUnitTests(U32 const seed, double compressibility) ZSTD_freeCCtx(cctx); } - DISPLAYLEVEL(3, "test%3d : ZSTD_CCtx_setCarams() : ", testNb++); + DISPLAYLEVEL(3, "test%3d : ZSTD_CCtx_setParams() : ", testNb++); { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); int value; ZSTD_parameters params = ZSTD_getParams(1, 0, 0); - params.cParams.strategy = -1; - /* Set invalid params == no change. */ + params.cParams.strategy = (ZSTD_strategy)-1; /* set invalid value, on purpose */ + /* Set invalid params == error out, and no change. */ CHECK(ZSTD_isError(ZSTD_CCtx_setParams(cctx, params))); CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_windowLog, &value)); @@ -2252,7 +2252,7 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "test%3i : compress with block splitting : ", testNb++) { ZSTD_CCtx* cctx = ZSTD_createCCtx(); - CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_useBlockSplitter, ZSTD_ps_enable) ); + CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_splitAfterSequences, ZSTD_ps_enable) ); cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize); CHECK_Z(cSize); ZSTD_freeCCtx(cctx);