From e3c42c739b3e4e7541b9c9f6d063231819e866d3 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Jun 2018 13:53:30 -0700 Subject: [PATCH 1/7] clean ZSTD_compress() initialization The (pretty old) code inside ZSTD_compress() was making some pretty bold assumptions on what's inside a CCtx and how to init it. This is pretty fragile by design. CCtx content evolve. Knowledge of how to handle that should be concentrate in one place. A side effect of this strategy is that ZSTD_compress() wouldn't check for BMI2 capability, and is therefore missing out some potential speed opportunity. This patch makes ZSTD_compress() use the same initialization and release functions as the normal creator / destructor ones. Measured on my laptop, with a custom version of bench manually modified to use ZSTD_compress() (instead of the advanced API) : This patch : 1#silesia.tar : 211984896 -> 73651053 (2.878), 312.2 MB/s , 723.8 MB/s 2#silesia.tar : 211984896 -> 70163650 (3.021), 226.2 MB/s , 649.8 MB/s 3#silesia.tar : 211984896 -> 66996749 (3.164), 169.4 MB/s , 636.7 MB/s 4#silesia.tar : 211984896 -> 65998319 (3.212), 136.7 MB/s , 619.2 MB/s dev branch : 1#silesia.tar : 211984896 -> 73651053 (2.878), 291.7 MB/s , 727.5 MB/s 2#silesia.tar : 211984896 -> 70163650 (3.021), 216.2 MB/s , 655.7 MB/s 3#silesia.tar : 211984896 -> 66996749 (3.164), 162.2 MB/s , 633.1 MB/s 4#silesia.tar : 211984896 -> 65998319 (3.212), 130.6 MB/s , 618.6 MB/s --- lib/compress/zstd_compress.c | 41 ++++++++++++++++++++++++------------ 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 7a30d552..fe4d65b6 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -64,19 +64,26 @@ ZSTD_CCtx* ZSTD_createCCtx(void) return ZSTD_createCCtx_advanced(ZSTD_defaultCMem); } +static void ZSTD_initCCtx(ZSTD_CCtx* cctx, ZSTD_customMem memManager) +{ + assert(cctx != NULL); + memset(cctx, 0, sizeof(*cctx)); + cctx->customMem = memManager; + cctx->bmi2 = ZSTD_cpuid_bmi2(ZSTD_cpuid()); + { size_t const err = ZSTD_CCtx_resetParameters(cctx); + assert(!ZSTD_isError(err)); + (void)err; + } +} + ZSTD_CCtx* ZSTD_createCCtx_advanced(ZSTD_customMem customMem) { ZSTD_STATIC_ASSERT(zcss_init==0); ZSTD_STATIC_ASSERT(ZSTD_CONTENTSIZE_UNKNOWN==(0ULL - 1)); if (!customMem.customAlloc ^ !customMem.customFree) return NULL; - { ZSTD_CCtx* const cctx = (ZSTD_CCtx*)ZSTD_calloc(sizeof(ZSTD_CCtx), customMem); + { ZSTD_CCtx* const cctx = (ZSTD_CCtx*)ZSTD_malloc(sizeof(ZSTD_CCtx), customMem); if (!cctx) return NULL; - cctx->customMem = customMem; - cctx->bmi2 = ZSTD_cpuid_bmi2(ZSTD_cpuid()); - { size_t const err = ZSTD_CCtx_resetParameters(cctx); - assert(!ZSTD_isError(err)); - (void)err; - } + ZSTD_initCCtx(cctx, customMem); return cctx; } } @@ -104,17 +111,24 @@ ZSTD_CCtx* ZSTD_initStaticCCtx(void *workspace, size_t workspaceSize) return cctx; } -size_t ZSTD_freeCCtx(ZSTD_CCtx* cctx) +static void ZSTD_freeCCtxContent(ZSTD_CCtx* cctx) { - if (cctx==NULL) return 0; /* support free on NULL */ - if (cctx->staticSize) return ERROR(memory_allocation); /* not compatible with static CCtx */ + assert(cctx != NULL); + assert(cctx->staticSize == 0); ZSTD_free(cctx->workSpace, cctx->customMem); cctx->workSpace = NULL; ZSTD_freeCDict(cctx->cdictLocal); cctx->cdictLocal = NULL; #ifdef ZSTD_MULTITHREAD ZSTDMT_freeCCtx(cctx->mtctx); cctx->mtctx = NULL; #endif +} + +size_t ZSTD_freeCCtx(ZSTD_CCtx* cctx) +{ + if (cctx==NULL) return 0; /* support free on NULL */ + if (cctx->staticSize) return ERROR(memory_allocation); /* not compatible with static CCtx */ + ZSTD_freeCCtxContent(cctx); ZSTD_free(cctx, cctx->customMem); - return 0; /* reserved as a potential error code in the future */ + return 0; } @@ -2963,10 +2977,9 @@ size_t ZSTD_compress(void* dst, size_t dstCapacity, const void* src, size_t srcS { size_t result; ZSTD_CCtx ctxBody; - memset(&ctxBody, 0, sizeof(ctxBody)); - ctxBody.customMem = ZSTD_defaultCMem; + ZSTD_initCCtx(&ctxBody, ZSTD_defaultCMem); result = ZSTD_compressCCtx(&ctxBody, dst, dstCapacity, src, srcSize, compressionLevel); - ZSTD_free(ctxBody.workSpace, ZSTD_defaultCMem); /* can't free ctxBody itself, as it's on stack; free only heap content */ + ZSTD_freeCCtxContent(&ctxBody); /* can't free ctxBody itself, as it's on stack; free only heap content */ return result; } From 43a5697b24ef2a3959a5a9a993d2c6047e46f0b0 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Jun 2018 14:46:55 -0700 Subject: [PATCH 2/7] negative compression level test compare results from simple and advanced AIP --- tests/fuzzer.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index b0c425e1..e9978ed8 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1080,12 +1080,36 @@ static int basicUnitTests(U32 seed, double compressibility) ZSTD_freeCCtx(cctx); } + /* negative compression level test : ensure simple API and advanced API produce same result */ + DISPLAYLEVEL(3, "test%3i : negative compression level : ", testNb++); + { size_t const srcSize = CNBuffSize / 5; + int const compressionLevel = -2; + size_t const cSize_1pass = ZSTD_compress(compressedBuffer, compressedBufferSize, + CNBuffer, srcSize, + compressionLevel); + if (ZSTD_isError(cSize_1pass)) goto _output_error; + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + assert(cctx != NULL); + CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_p_compressionLevel, compressionLevel) ); + { ZSTD_inBuffer in = { CNBuffer, srcSize, 0 }; + ZSTD_outBuffer out = { compressedBuffer, compressedBufferSize, 0 }; + size_t const compressionResult = ZSTD_compress_generic(cctx, &out, &in, ZSTD_e_end); + DISPLAYLEVEL(5, "simple=%zu vs %zu=advanced : ", cSize_1pass, out.pos); + if (ZSTD_isError(compressionResult)) goto _output_error; + if (out.pos != cSize_1pass) goto _output_error; + } + ZSTD_freeCCtx(cctx); + } + } + DISPLAYLEVEL(3, "OK \n"); + /* parameters order test */ { size_t const inputSize = CNBuffSize / 2; U64 xxh64; - { ZSTD_CCtx* cctx = ZSTD_createCCtx(); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); DISPLAYLEVEL(3, "test%3i : parameters in order : ", testNb++); + assert(cctx != NULL); CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_p_compressionLevel, 2) ); CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_p_enableLongDistanceMatching, 1) ); CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_p_windowLog, 18) ); From 8537bfd85cb2cf55747186f76e419e45c1d6a5a4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Jun 2018 15:12:13 -0700 Subject: [PATCH 3/7] fuzzer: make negative compression level fail result of ZSTD_compress_advanced() is different from ZSTD_compress_generic() when using negative compression levels because the disabling of huffman compression is not passed in parameters. --- lib/compress/zstd_compress.c | 49 +++++++++++++++++++++++------------- tests/fuzzer.c | 26 +++++++++++-------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index fe4d65b6..43c3f27b 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2916,22 +2916,22 @@ size_t ZSTD_compressEnd (ZSTD_CCtx* cctx, static size_t ZSTD_compress_internal (ZSTD_CCtx* cctx, - void* dst, size_t dstCapacity, - const void* src, size_t srcSize, - const void* dict,size_t dictSize, - ZSTD_parameters params) + void* dst, size_t dstCapacity, + const void* src, size_t srcSize, + const void* dict,size_t dictSize, + ZSTD_parameters params) { ZSTD_CCtx_params const cctxParams = ZSTD_assignParamsToCCtxParams(cctx->requestedParams, params); DEBUGLOG(4, "ZSTD_compress_internal"); return ZSTD_compress_advanced_internal(cctx, - dst, dstCapacity, - src, srcSize, - dict, dictSize, - cctxParams); + dst, dstCapacity, + src, srcSize, + dict, dictSize, + cctxParams); } -size_t ZSTD_compress_advanced (ZSTD_CCtx* ctx, +size_t ZSTD_compress_advanced (ZSTD_CCtx* cctx, void* dst, size_t dstCapacity, const void* src, size_t srcSize, const void* dict,size_t dictSize, @@ -2939,7 +2939,11 @@ size_t ZSTD_compress_advanced (ZSTD_CCtx* ctx, { DEBUGLOG(4, "ZSTD_compress_advanced"); CHECK_F(ZSTD_checkCParams(params.cParams)); - return ZSTD_compress_internal(ctx, dst, dstCapacity, src, srcSize, dict, dictSize, params); + return ZSTD_compress_internal(cctx, + dst, dstCapacity, + src, srcSize, + dict, dictSize, + params); } /* Internal */ @@ -2950,15 +2954,18 @@ size_t ZSTD_compress_advanced_internal( const void* dict,size_t dictSize, ZSTD_CCtx_params params) { - DEBUGLOG(4, "ZSTD_compress_advanced_internal (srcSize:%u)", - (U32)srcSize); - CHECK_F( ZSTD_compressBegin_internal(cctx, dict, dictSize, ZSTD_dct_auto, ZSTD_dtlm_fast, NULL, - params, srcSize, ZSTDb_not_buffered) ); + DEBUGLOG(4, "ZSTD_compress_advanced_internal (srcSize:%u)", (U32)srcSize); + CHECK_F( ZSTD_compressBegin_internal(cctx, + dict, dictSize, ZSTD_dct_auto, ZSTD_dtlm_fast, NULL, + params, srcSize, ZSTDb_not_buffered) ); return ZSTD_compressEnd(cctx, dst, dstCapacity, src, srcSize); } -size_t ZSTD_compress_usingDict(ZSTD_CCtx* cctx, void* dst, size_t dstCapacity, const void* src, size_t srcSize, - const void* dict, size_t dictSize, int compressionLevel) +size_t ZSTD_compress_usingDict(ZSTD_CCtx* cctx, + void* dst, size_t dstCapacity, + const void* src, size_t srcSize, + const void* dict, size_t dictSize, + int compressionLevel) { ZSTD_parameters const params = ZSTD_getParams(compressionLevel, srcSize ? srcSize : 1, dict ? dictSize : 0); ZSTD_CCtx_params cctxParams = ZSTD_assignParamsToCCtxParams(cctx->requestedParams, params); @@ -2967,13 +2974,19 @@ size_t ZSTD_compress_usingDict(ZSTD_CCtx* cctx, void* dst, size_t dstCapacity, c return ZSTD_compress_advanced_internal(cctx, dst, dstCapacity, src, srcSize, dict, dictSize, cctxParams); } -size_t ZSTD_compressCCtx (ZSTD_CCtx* cctx, void* dst, size_t dstCapacity, const void* src, size_t srcSize, int compressionLevel) +size_t ZSTD_compressCCtx(ZSTD_CCtx* cctx, + void* dst, size_t dstCapacity, + const void* src, size_t srcSize, + int compressionLevel) { DEBUGLOG(4, "ZSTD_compressCCtx (srcSize=%u)", (U32)srcSize); + assert(cctx != NULL); return ZSTD_compress_usingDict(cctx, dst, dstCapacity, src, srcSize, NULL, 0, compressionLevel); } -size_t ZSTD_compress(void* dst, size_t dstCapacity, const void* src, size_t srcSize, int compressionLevel) +size_t ZSTD_compress(void* dst, size_t dstCapacity, + const void* src, size_t srcSize, + int compressionLevel) { size_t result; ZSTD_CCtx ctxBody; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index e9978ed8..af9ac9ac 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1082,14 +1082,19 @@ static int basicUnitTests(U32 seed, double compressibility) /* negative compression level test : ensure simple API and advanced API produce same result */ DISPLAYLEVEL(3, "test%3i : negative compression level : ", testNb++); - { size_t const srcSize = CNBuffSize / 5; - int const compressionLevel = -2; - size_t const cSize_1pass = ZSTD_compress(compressedBuffer, compressedBufferSize, - CNBuffer, srcSize, - compressionLevel); - if (ZSTD_isError(cSize_1pass)) goto _output_error; - { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); - assert(cctx != NULL); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + size_t const srcSize = CNBuffSize / 5; + int const compressionLevel = -1; + + assert(cctx != NULL); + { ZSTD_parameters const params = ZSTD_getParams(compressionLevel, srcSize, 0); + size_t const cSize_1pass = ZSTD_compress_advanced(cctx, + compressedBuffer, compressedBufferSize, + CNBuffer, srcSize, + NULL, 0, + params); + if (ZSTD_isError(cSize_1pass)) goto _output_error; + CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_p_compressionLevel, compressionLevel) ); { ZSTD_inBuffer in = { CNBuffer, srcSize, 0 }; ZSTD_outBuffer out = { compressedBuffer, compressedBufferSize, 0 }; @@ -1097,9 +1102,8 @@ static int basicUnitTests(U32 seed, double compressibility) DISPLAYLEVEL(5, "simple=%zu vs %zu=advanced : ", cSize_1pass, out.pos); if (ZSTD_isError(compressionResult)) goto _output_error; if (out.pos != cSize_1pass) goto _output_error; - } - ZSTD_freeCCtx(cctx); - } + } } + ZSTD_freeCCtx(cctx); } DISPLAYLEVEL(3, "OK \n"); From a57b4df85fb44d14fbb8f5ac3cf3cc6e0be59d8a Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Jun 2018 15:24:12 -0700 Subject: [PATCH 4/7] removed literalCompression directive in this version, literal compression is always disabled for ZSTD_fast strategy. Performance parity between ZSTD_compress_advanced() and ZSTD_compress_generic() --- doc/zstd_manual.html | 6 ------ lib/compress/zstd_compress.c | 19 ++++--------------- lib/compress/zstd_compress_internal.h | 1 - lib/compress/zstdmt_compress.c | 5 ++--- lib/zstd.h | 6 ------ 5 files changed, 6 insertions(+), 31 deletions(-) diff --git a/doc/zstd_manual.html b/doc/zstd_manual.html index 3dee74a3..ae297286 100644 --- a/doc/zstd_manual.html +++ b/doc/zstd_manual.html @@ -867,12 +867,6 @@ size_t ZSTD_decodingBufferSize_min(unsigned long long windowSize, unsigned long /* experimental parameters - no stability guaranteed */ /* =================================================================== */ - ZSTD_p_compressLiterals=1000, /* control huffman compression of literals (enabled) by default. - * disabling it improves speed and decreases compression ratio by a large amount. - * note : this setting is automatically updated when changing compression level. - * positive compression levels set ZSTD_p_compressLiterals to 1. - * negative compression levels set ZSTD_p_compressLiterals to 0. */ - ZSTD_p_forceMaxWindow=1100, /* Force back-reference distances to remain < windowSize, * even when referencing into Dictionary content (default:0) */ diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 43c3f27b..f6bee0b9 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -267,7 +267,6 @@ static int ZSTD_isUpdateAuthorized(ZSTD_cParameter param) case ZSTD_p_minMatch: case ZSTD_p_targetLength: case ZSTD_p_compressionStrategy: - case ZSTD_p_compressLiterals: return 1; case ZSTD_p_format: @@ -318,7 +317,6 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v if (cctx->cdict) return ERROR(stage_wrong); return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value); - case ZSTD_p_compressLiterals: case ZSTD_p_contentSizeFlag: case ZSTD_p_checksumFlag: case ZSTD_p_dictIDFlag: @@ -367,7 +365,6 @@ size_t ZSTD_CCtxParam_setParameter( int cLevel = (int)value; /* cast expected to restore negative sign */ if (cLevel > ZSTD_maxCLevel()) cLevel = ZSTD_maxCLevel(); if (cLevel) { /* 0 : does not change current level */ - CCtxParams->disableLiteralCompression = (cLevel<0); /* negative levels disable huffman */ CCtxParams->compressionLevel = cLevel; } if (CCtxParams->compressionLevel >= 0) return CCtxParams->compressionLevel; @@ -415,10 +412,6 @@ size_t ZSTD_CCtxParam_setParameter( CCtxParams->cParams.strategy = (ZSTD_strategy)value; return (size_t)CCtxParams->cParams.strategy; - case ZSTD_p_compressLiterals: - CCtxParams->disableLiteralCompression = !value; - return !CCtxParams->disableLiteralCompression; - case ZSTD_p_contentSizeFlag : /* Content size written in frame header _when known_ (default:1) */ DEBUGLOG(4, "set content size flag = %u", (value>0)); @@ -530,9 +523,6 @@ size_t ZSTD_CCtxParam_getParameter( case ZSTD_p_compressionStrategy : *value = (unsigned)CCtxParams->cParams.strategy; break; - case ZSTD_p_compressLiterals: - *value = !CCtxParams->disableLiteralCompression; - break; case ZSTD_p_contentSizeFlag : *value = CCtxParams->fParams.contentSizeFlag; break; @@ -2068,9 +2058,10 @@ MEM_STATIC size_t ZSTD_compressSequences_internal(seqStore_t* seqStorePtr, /* Compress literals */ { const BYTE* const literals = seqStorePtr->litStart; size_t const litSize = seqStorePtr->lit - literals; + int const disableLiteralCompression = (cctxParams->cParams.strategy == ZSTD_fast) && (cctxParams->cParams.targetLength > 0); size_t const cSize = ZSTD_compressLiterals( &prevEntropy->huf, &nextEntropy->huf, - cctxParams->cParams.strategy, cctxParams->disableLiteralCompression, + cctxParams->cParams.strategy, disableLiteralCompression, op, dstCapacity, literals, litSize, workspace, bmi2); @@ -2967,10 +2958,9 @@ size_t ZSTD_compress_usingDict(ZSTD_CCtx* cctx, const void* dict, size_t dictSize, int compressionLevel) { - ZSTD_parameters const params = ZSTD_getParams(compressionLevel, srcSize ? srcSize : 1, dict ? dictSize : 0); + ZSTD_parameters const params = ZSTD_getParams(compressionLevel, srcSize + (!srcSize), dict ? dictSize : 0); ZSTD_CCtx_params cctxParams = ZSTD_assignParamsToCCtxParams(cctx->requestedParams, params); assert(params.fParams.contentSizeFlag == 1); - ZSTD_CCtxParam_setParameter(&cctxParams, ZSTD_p_compressLiterals, compressionLevel>=0); return ZSTD_compress_advanced_internal(cctx, dst, dstCapacity, src, srcSize, dict, dictSize, cctxParams); } @@ -3293,8 +3283,7 @@ static size_t ZSTD_resetCStream_internal(ZSTD_CStream* cctx, const ZSTD_CDict* const cdict, ZSTD_CCtx_params const params, unsigned long long const pledgedSrcSize) { - DEBUGLOG(4, "ZSTD_resetCStream_internal (disableLiteralCompression=%i)", - params.disableLiteralCompression); + DEBUGLOG(4, "ZSTD_resetCStream_internal"); /* params are supposed to be fully validated at this point */ assert(!ZSTD_isError(ZSTD_checkCParams(params.cParams))); assert(!((dict) && (cdict))); /* either dict or cdict, not both */ diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index fd072f30..c973c1a8 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -181,7 +181,6 @@ struct ZSTD_CCtx_params_s { ZSTD_frameParameters fParams; int compressionLevel; - int disableLiteralCompression; int forceWindow; /* force back-references to respect limit of * 1<cctxPool->totalCCtx, params.disableLiteralCompression); + DEBUGLOG(4, "ZSTDMT_initCStream_internal (pledgedSrcSize=%u, nbWorkers=%u, cctxPool=%u)", + (U32)pledgedSrcSize, params.nbWorkers, mtctx->cctxPool->totalCCtx); /* params are supposed to be fully validated at this point */ assert(!ZSTD_isError(ZSTD_checkCParams(params.cParams))); assert(!((dict) && (cdict))); /* either dict or cdict, not both */ diff --git a/lib/zstd.h b/lib/zstd.h index 8f42ff81..829c8ad2 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1052,12 +1052,6 @@ typedef enum { /* experimental parameters - no stability guaranteed */ /* =================================================================== */ - ZSTD_p_compressLiterals=1000, /* control huffman compression of literals (enabled) by default. - * disabling it improves speed and decreases compression ratio by a large amount. - * note : this setting is automatically updated when changing compression level. - * positive compression levels set ZSTD_p_compressLiterals to 1. - * negative compression levels set ZSTD_p_compressLiterals to 0. */ - ZSTD_p_forceMaxWindow=1100, /* Force back-reference distances to remain < windowSize, * even when referencing into Dictionary content (default:0) */ From c2c47e24e0d5bc73c794d23ffd4001589885aec4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Jun 2018 15:49:01 -0700 Subject: [PATCH 5/7] support targetlen==0 with strategy==ZSTD_fast to mean "normal compression", targetlen >= 1 now means "disable huffman compression of literals" --- lib/compress/zstd_compress.c | 17 +++++++---------- lib/compress/zstd_fast.c | 7 +++++-- lib/zstd.h | 1 - tests/paramgrill.c | 2 +- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index f6bee0b9..2c566dd3 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -689,8 +689,6 @@ size_t ZSTD_checkCParams(ZSTD_compressionParameters cParams) CLAMPCHECK(cParams.hashLog, ZSTD_HASHLOG_MIN, ZSTD_HASHLOG_MAX); CLAMPCHECK(cParams.searchLog, ZSTD_SEARCHLOG_MIN, ZSTD_SEARCHLOG_MAX); CLAMPCHECK(cParams.searchLength, ZSTD_SEARCHLENGTH_MIN, ZSTD_SEARCHLENGTH_MAX); - if ((U32)(cParams.targetLength) < ZSTD_TARGETLENGTH_MIN) - return ERROR(parameter_unsupported); if ((U32)(cParams.strategy) > (U32)ZSTD_btultra) return ERROR(parameter_unsupported); return 0; @@ -710,7 +708,6 @@ static ZSTD_compressionParameters ZSTD_clampCParams(ZSTD_compressionParameters c CLAMP(cParams.hashLog, ZSTD_HASHLOG_MIN, ZSTD_HASHLOG_MAX); CLAMP(cParams.searchLog, ZSTD_SEARCHLOG_MIN, ZSTD_SEARCHLOG_MAX); CLAMP(cParams.searchLength, ZSTD_SEARCHLENGTH_MIN, ZSTD_SEARCHLENGTH_MAX); - if ((U32)(cParams.targetLength) < ZSTD_TARGETLENGTH_MIN) cParams.targetLength = ZSTD_TARGETLENGTH_MIN; if ((U32)(cParams.strategy) > (U32)ZSTD_btultra) cParams.strategy = ZSTD_btultra; return cParams; } @@ -3713,8 +3710,8 @@ static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEV { /* "default" - guarantees a monotonically increasing memory budget */ /* W, C, H, S, L, TL, strat */ { 19, 12, 13, 1, 6, 1, ZSTD_fast }, /* base for negative levels */ - { 19, 13, 14, 1, 7, 1, ZSTD_fast }, /* level 1 */ - { 19, 15, 16, 1, 6, 1, ZSTD_fast }, /* level 2 */ + { 19, 13, 14, 1, 7, 0, ZSTD_fast }, /* level 1 */ + { 19, 15, 16, 1, 6, 0, ZSTD_fast }, /* level 2 */ { 20, 16, 17, 1, 5, 1, ZSTD_dfast }, /* level 3 */ { 20, 18, 18, 1, 5, 1, ZSTD_dfast }, /* level 4 */ { 20, 18, 18, 2, 5, 2, ZSTD_greedy }, /* level 5 */ @@ -3739,7 +3736,7 @@ static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEV { /* for srcSize <= 256 KB */ /* W, C, H, S, L, T, strat */ { 18, 12, 13, 1, 5, 1, ZSTD_fast }, /* base for negative levels */ - { 18, 13, 14, 1, 6, 1, ZSTD_fast }, /* level 1 */ + { 18, 13, 14, 1, 6, 0, ZSTD_fast }, /* level 1 */ { 18, 14, 14, 1, 5, 1, ZSTD_dfast }, /* level 2 */ { 18, 16, 16, 1, 4, 1, ZSTD_dfast }, /* level 3 */ { 18, 16, 17, 2, 5, 2, ZSTD_greedy }, /* level 4.*/ @@ -3765,8 +3762,8 @@ static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEV { /* for srcSize <= 128 KB */ /* W, C, H, S, L, T, strat */ { 17, 12, 12, 1, 5, 1, ZSTD_fast }, /* base for negative levels */ - { 17, 12, 13, 1, 6, 1, ZSTD_fast }, /* level 1 */ - { 17, 13, 15, 1, 5, 1, ZSTD_fast }, /* level 2 */ + { 17, 12, 13, 1, 6, 0, ZSTD_fast }, /* level 1 */ + { 17, 13, 15, 1, 5, 0, ZSTD_fast }, /* level 2 */ { 17, 15, 16, 2, 5, 1, ZSTD_dfast }, /* level 3 */ { 17, 17, 17, 2, 4, 1, ZSTD_dfast }, /* level 4 */ { 17, 16, 17, 3, 4, 2, ZSTD_greedy }, /* level 5 */ @@ -3791,8 +3788,8 @@ static const ZSTD_compressionParameters ZSTD_defaultCParameters[4][ZSTD_MAX_CLEV { /* for srcSize <= 16 KB */ /* W, C, H, S, L, T, strat */ { 14, 12, 13, 1, 5, 1, ZSTD_fast }, /* base for negative levels */ - { 14, 14, 15, 1, 5, 1, ZSTD_fast }, /* level 1 */ - { 14, 14, 15, 1, 4, 1, ZSTD_fast }, /* level 2 */ + { 14, 14, 15, 1, 5, 0, ZSTD_fast }, /* level 1 */ + { 14, 14, 15, 1, 4, 0, ZSTD_fast }, /* level 2 */ { 14, 14, 14, 2, 4, 1, ZSTD_dfast }, /* level 3.*/ { 14, 14, 14, 4, 4, 2, ZSTD_greedy }, /* level 4.*/ { 14, 14, 14, 3, 4, 4, ZSTD_lazy }, /* level 5.*/ diff --git a/lib/compress/zstd_fast.c b/lib/compress/zstd_fast.c index 57a70eee..37a71516 100644 --- a/lib/compress/zstd_fast.c +++ b/lib/compress/zstd_fast.c @@ -45,7 +45,7 @@ FORCE_INLINE_TEMPLATE size_t ZSTD_compressBlock_fast_generic( ZSTD_matchState_t* ms, seqStore_t* seqStore, U32 rep[ZSTD_REP_NUM], void const* src, size_t srcSize, - U32 const hlog, U32 const stepSize, U32 const mls, + U32 const hlog, U32 stepSize, U32 const mls, ZSTD_dictMode_e const dictMode) { U32* const hashTable = ms->hashTable; @@ -84,6 +84,7 @@ size_t ZSTD_compressBlock_fast_generic( || prefixStartIndex >= (U32)(dictEnd - dictBase)); /* init */ + stepSize += !stepSize; /* support stepSize of 0 */ ip += (dictAndPrefixLength == 0); if (dictMode == ZSTD_noDict) { U32 const maxRep = (U32)(ip - prefixStart); @@ -264,7 +265,7 @@ size_t ZSTD_compressBlock_fast_dictMatchState( static size_t ZSTD_compressBlock_fast_extDict_generic( ZSTD_matchState_t* ms, seqStore_t* seqStore, U32 rep[ZSTD_REP_NUM], void const* src, size_t srcSize, - U32 const hlog, U32 const stepSize, U32 const mls) + U32 const hlog, U32 stepSize, U32 const mls) { U32* hashTable = ms->hashTable; const BYTE* const base = ms->window.base; @@ -281,6 +282,8 @@ static size_t ZSTD_compressBlock_fast_extDict_generic( const BYTE* const ilimit = iend - 8; U32 offset_1=rep[0], offset_2=rep[1]; + stepSize += !stepSize; /* support stepSize == 0 */ + /* Search Loop */ while (ip < ilimit) { /* < instead of <=, because (ip+1) */ const size_t h = ZSTD_hashPtr(ip, hlog, mls); diff --git a/lib/zstd.h b/lib/zstd.h index 829c8ad2..f8caeb0d 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -395,7 +395,6 @@ ZSTDLIB_API size_t ZSTD_DStreamOutSize(void); /*!< recommended size for output #define ZSTD_SEARCHLOG_MIN 1 #define ZSTD_SEARCHLENGTH_MAX 7 /* only for ZSTD_fast, other strategies are limited to 6 */ #define ZSTD_SEARCHLENGTH_MIN 3 /* only for ZSTD_btopt, other strategies are limited to 4 */ -#define ZSTD_TARGETLENGTH_MIN 1 /* only used by btopt, btultra and btfast */ #define ZSTD_LDM_MINMATCH_MIN 4 #define ZSTD_LDM_MINMATCH_MAX 4096 #define ZSTD_LDM_BUCKETSIZELOG_MAX 8 diff --git a/tests/paramgrill.c b/tests/paramgrill.c index 44e3cb37..6208effe 100644 --- a/tests/paramgrill.c +++ b/tests/paramgrill.c @@ -598,7 +598,7 @@ static ZSTD_compressionParameters randomParams(void) p.searchLog = (FUZ_rand(&g_rand) % (ZSTD_SEARCHLOG_MAX+1 - ZSTD_SEARCHLOG_MIN)) + ZSTD_SEARCHLOG_MIN; p.windowLog = (FUZ_rand(&g_rand) % (ZSTD_WINDOWLOG_MAX+1 - ZSTD_WINDOWLOG_MIN)) + ZSTD_WINDOWLOG_MIN; p.searchLength=(FUZ_rand(&g_rand) % (ZSTD_SEARCHLENGTH_MAX+1 - ZSTD_SEARCHLENGTH_MIN)) + ZSTD_SEARCHLENGTH_MIN; - p.targetLength=(FUZ_rand(&g_rand) % (512)) + ZSTD_TARGETLENGTH_MIN; + p.targetLength=(FUZ_rand(&g_rand) % (512)); p.strategy = (ZSTD_strategy) (FUZ_rand(&g_rand) % (ZSTD_btultra +1)); validated = !ZSTD_isError(ZSTD_checkCParams(p)); } From 0b1833c2cb622cc10e58124ffbc8be698304a7ea Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Jun 2018 16:07:46 -0700 Subject: [PATCH 6/7] fixed regressiontest ZSTD_TARGETLEN_MIN no longer exists since is would be tautological to check if an unsigned value is < 0. --- tests/fuzz/zstd_helpers.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/fuzz/zstd_helpers.c b/tests/fuzz/zstd_helpers.c index 6fc38361..ead004e4 100644 --- a/tests/fuzz/zstd_helpers.c +++ b/tests/fuzz/zstd_helpers.c @@ -34,8 +34,7 @@ ZSTD_compressionParameters FUZZ_randomCParams(size_t srcSize, uint32_t *state) cParams.searchLog = FUZZ_rand32(state, ZSTD_SEARCHLOG_MIN, 9); cParams.searchLength = FUZZ_rand32(state, ZSTD_SEARCHLENGTH_MIN, ZSTD_SEARCHLENGTH_MAX); - cParams.targetLength = FUZZ_rand32(state, ZSTD_TARGETLENGTH_MIN, - 512); + cParams.targetLength = FUZZ_rand32(state, 0, 512); cParams.strategy = FUZZ_rand32(state, ZSTD_fast, ZSTD_btultra); return ZSTD_adjustCParams(cParams, srcSize, 0); } From 56961e4cedee3ed5e7d9a9932226948efdc7b89b Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 7 Jun 2018 16:59:33 -0700 Subject: [PATCH 7/7] fixed minor conversion warning --- tests/fuzzer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index f72abca7..9b4cd58d 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1131,7 +1131,7 @@ static int basicUnitTests(U32 seed, double compressibility) params); if (ZSTD_isError(cSize_1pass)) goto _output_error; - CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_p_compressionLevel, compressionLevel) ); + CHECK( ZSTD_CCtx_setParameter(cctx, ZSTD_p_compressionLevel, (unsigned)compressionLevel) ); { ZSTD_inBuffer in = { CNBuffer, srcSize, 0 }; ZSTD_outBuffer out = { compressedBuffer, compressedBufferSize, 0 }; size_t const compressionResult = ZSTD_compress_generic(cctx, &out, &in, ZSTD_e_end);