From 3d523c741be041f17c28e43b89ab6dfcaee281d2 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 5 Jun 2018 11:23:18 -0700 Subject: [PATCH] added workSpaceTooLarge and workSpaceWasteful also : slightly increased speed of test fuzzer.16 --- lib/compress/zstd_compress.c | 73 ++++++++++++++++++--------- lib/compress/zstd_compress_internal.h | 6 ++- tests/fuzzer.c | 12 +++-- 3 files changed, 62 insertions(+), 29 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 7a30d552..b87ae381 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -695,7 +695,8 @@ size_t ZSTD_checkCParams(ZSTD_compressionParameters cParams) /** ZSTD_clampCParams() : * make CParam values within valid range. * @return : valid CParams */ -static ZSTD_compressionParameters ZSTD_clampCParams(ZSTD_compressionParameters cParams) +static ZSTD_compressionParameters +ZSTD_clampCParams(ZSTD_compressionParameters cParams) { # define CLAMP(val,min,max) { \ if (val (U32)ZSTD_btultra) cParams.strategy = ZSTD_btultra; + if ((U32)(cParams.targetLength) < ZSTD_TARGETLENGTH_MIN) + cParams.targetLength = ZSTD_TARGETLENGTH_MIN; + CLAMP(cParams.strategy, ZSTD_fast, ZSTD_btultra); return cParams; } @@ -723,8 +725,11 @@ static U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat) optimize `cPar` for a given input (`srcSize` and `dictSize`). mostly downsizing to reduce memory consumption and initialization latency. Both `srcSize` and `dictSize` are optional (use 0 if unknown). - Note : cPar is considered validated at this stage. Use ZSTD_checkCParams() to ensure that condition. */ -ZSTD_compressionParameters ZSTD_adjustCParams_internal(ZSTD_compressionParameters cPar, unsigned long long srcSize, size_t dictSize) + Note : cPar is assumed validated. Use ZSTD_checkCParams() to ensure this condition. */ +static ZSTD_compressionParameters +ZSTD_adjustCParams_internal(ZSTD_compressionParameters cPar, + unsigned long long srcSize, + size_t dictSize) { static const U64 minSrcSize = 513; /* (1<<9) + 1 */ static const U64 maxWindowResize = 1ULL << (ZSTD_WINDOWLOG_MAX-1); @@ -756,13 +761,18 @@ ZSTD_compressionParameters ZSTD_adjustCParams_internal(ZSTD_compressionParameter return cPar; } -ZSTD_compressionParameters ZSTD_adjustCParams(ZSTD_compressionParameters cPar, unsigned long long srcSize, size_t dictSize) +ZSTD_compressionParameters +ZSTD_adjustCParams(ZSTD_compressionParameters cPar, + unsigned long long srcSize, + size_t dictSize) { cPar = ZSTD_clampCParams(cPar); return ZSTD_adjustCParams_internal(cPar, srcSize, dictSize); } -static size_t ZSTD_sizeof_matchState(ZSTD_compressionParameters const* cParams, const U32 forCCtx) +static size_t +ZSTD_sizeof_matchState(const ZSTD_compressionParameters* const cParams, + const U32 forCCtx) { size_t const chainSize = (cParams->strategy == ZSTD_fast) ? 0 : ((size_t)1 << cParams->chainLog); size_t const hSize = ((size_t)1) << cParams->hashLog; @@ -848,12 +858,14 @@ size_t ZSTD_estimateCStreamSize_usingCParams(ZSTD_compressionParameters cParams) return ZSTD_estimateCStreamSize_usingCCtxParams(¶ms); } -static size_t ZSTD_estimateCStreamSize_internal(int compressionLevel) { +static size_t ZSTD_estimateCStreamSize_internal(int compressionLevel) +{ ZSTD_compressionParameters const cParams = ZSTD_getCParams(compressionLevel, 0, 0); return ZSTD_estimateCStreamSize_usingCParams(cParams); } -size_t ZSTD_estimateCStreamSize(int compressionLevel) { +size_t ZSTD_estimateCStreamSize(int compressionLevel) +{ int level; size_t memBudget = 0; for (level=1; level<=compressionLevel; level++) { @@ -1042,10 +1054,18 @@ ZSTD_reset_matchState(ZSTD_matchState_t* ms, return ptr; } +#define ZSTD_WORKSPACETOOLARGE_FACTOR 3 /* define "workspace is too large" as ZSTD_WORKSPACETOOLARGE_FACTOR times larger than needed */ +#define ZSTD_WORKSPACETOOLARGE_MAX 128 /* when workspace is continuously too large + * at least that number of times, + * context's memory usage is actually wasteful, + * because it's sized to handle a worst case scenario which rarely happens. + * In which case, resize it down to free some memory */ + /*! ZSTD_resetCCtx_internal() : note : `params` are assumed fully validated at this stage */ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, - ZSTD_CCtx_params params, U64 pledgedSrcSize, + ZSTD_CCtx_params params, + U64 pledgedSrcSize, ZSTD_compResetPolicy_e const crp, ZSTD_buffered_policy_e const zbuff) { @@ -1057,8 +1077,8 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, if (ZSTD_equivalentParams(zc->appliedParams, params, zc->inBuffSize, zc->blockSize, zbuff, pledgedSrcSize)) { - DEBUGLOG(4, "ZSTD_equivalentParams()==1 -> continue mode (wLog1=%u, blockSize1=%u)", - zc->appliedParams.cParams.windowLog, (U32)zc->blockSize); + DEBUGLOG(4, "ZSTD_equivalentParams()==1 -> continue mode (wLog1=%u, blockSize1=%zu)", + zc->appliedParams.cParams.windowLog, zc->blockSize); return ZSTD_continueCCtx(zc, params, pledgedSrcSize); } } DEBUGLOG(4, "ZSTD_equivalentParams()==0 -> reset CCtx"); @@ -1069,8 +1089,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, ZSTD_ldm_adjustParameters(¶ms.ldmParams, ¶ms.cParams); assert(params.ldmParams.hashLog >= params.ldmParams.bucketSizeLog); assert(params.ldmParams.hashEveryLog < 32); - zc->ldmState.hashPower = - ZSTD_ldm_getHashPower(params.ldmParams.minMatchLength); + zc->ldmState.hashPower = ZSTD_ldm_getHashPower(params.ldmParams.minMatchLength); } { size_t const windowSize = MAX(1, (size_t)MIN(((U64)1 << params.cParams.windowLog), pledgedSrcSize)); @@ -1082,7 +1101,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, size_t const buffInSize = (zbuff==ZSTDb_buffered) ? windowSize + blockSize : 0; size_t const matchStateSize = ZSTD_sizeof_matchState(¶ms.cParams, /* forCCtx */ 1); size_t const maxNbLdmSeq = ZSTD_ldm_getMaxNbSeq(params.ldmParams, blockSize); - void* ptr; + void* ptr; /* used to partition workSpace */ /* Check if workSpace is large enough, alloc a new one if needed */ { size_t const entropySpace = HUF_WORKSPACE_SIZE; @@ -1094,14 +1113,19 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, size_t const neededSpace = entropySpace + blockStateSpace + ldmSpace + ldmSeqSpace + matchStateSize + tokenSpace + bufferSpace; - DEBUGLOG(4, "Need %uKB workspace, including %uKB for match state, and %uKB for buffers", - (U32)(neededSpace>>10), (U32)(matchStateSize>>10), (U32)(bufferSpace>>10)); - DEBUGLOG(4, "windowSize: %u - blockSize: %u", (U32)windowSize, (U32)blockSize); - if (zc->workSpaceSize < neededSpace) { /* too small : resize */ - DEBUGLOG(4, "Need to update workSpaceSize from %uK to %uK", - (unsigned)(zc->workSpaceSize>>10), - (unsigned)(neededSpace>>10)); + int const workSpaceTooSmall = zc->workSpaceSize < neededSpace; + int const workSpaceTooLarge = zc->workSpaceSize > ZSTD_WORKSPACETOOLARGE_FACTOR * neededSpace; + int const workSpaceWasteful = 0 && workSpaceTooLarge && (zc->workSpaceTooLarge > ZSTD_WORKSPACETOOLARGE_MAX); + DEBUGLOG(4, "Need %zuKB workspace, including %zuKB for match state, and %zuKB for buffers", + neededSpace>>10, matchStateSize>>10, bufferSpace>>10); + DEBUGLOG(4, "windowSize: %zu - blockSize: %zu", windowSize, blockSize); + + zc->workSpaceTooLarge = workSpaceTooLarge ? zc->workSpaceTooLarge+1 : 0; + if (workSpaceTooSmall || workSpaceWasteful) { + DEBUGLOG(4, "Need to resize workSpaceSize from %zuKB to %zuKB", + zc->workSpaceSize >> 10, + neededSpace >> 10); /* static cctx : no resize, error out */ if (zc->staticSize) return ERROR(memory_allocation); @@ -1110,9 +1134,12 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, zc->workSpace = ZSTD_malloc(neededSpace, zc->customMem); if (zc->workSpace == NULL) return ERROR(memory_allocation); zc->workSpaceSize = neededSpace; + zc->workSpaceTooLarge = 0; ptr = zc->workSpace; - /* Statically sized space. entropyWorkspace never moves (but prev/next block swap places) */ + /* Statically sized space. + * entropyWorkspace never moves, + * though prev/next block swap places */ assert(((size_t)zc->workSpace & 3) == 0); /* ensure correct alignment */ assert(zc->workSpaceSize >= 2 * sizeof(ZSTD_compressedBlockState_t)); zc->blockState.prevCBlock = (ZSTD_compressedBlockState_t*)zc->workSpace; diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index fd072f30..b2475aae 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -27,6 +27,7 @@ extern "C" { #endif + /*-************************************* * Constants ***************************************/ @@ -37,7 +38,8 @@ extern "C" { It's not a big deal though : candidate will just be sorted again. Additionnally, candidate position 1 will be lost. But candidate 1 cannot hide a large tree of candidates, so it's a minimal loss. - The benefit is that ZSTD_DUBT_UNSORTED_MARK cannot be misdhandled after table re-use with a different strategy */ + The benefit is that ZSTD_DUBT_UNSORTED_MARK cannot be misdhandled after table re-use with a different strategy + Constant required by ZSTD_compressBlock_btlazy2() and ZSTD_reduceTable_internal() */ /*-************************************* @@ -204,6 +206,8 @@ struct ZSTD_CCtx_s { ZSTD_CCtx_params requestedParams; ZSTD_CCtx_params appliedParams; U32 dictID; + + int workSpaceTooLarge; void* workSpace; size_t workSpaceSize; size_t blockSize; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index b0c425e1..8dabcddb 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -450,14 +450,16 @@ static int basicUnitTests(U32 seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); - DISPLAYLEVEL(3, "test%3d : large window log smaller data : ", testNb++); + DISPLAYLEVEL(3, "test%3d : overflow protection with large windowLog : ", testNb++); { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); - ZSTD_parameters params = ZSTD_getParams(1, ZSTD_CONTENTSIZE_UNKNOWN, 0); - size_t const nbCompressions = (1U << 31) / CNBuffSize + 1; - size_t i; + ZSTD_parameters params = ZSTD_getParams(-9, ZSTD_CONTENTSIZE_UNKNOWN, 0); + size_t const nbCompressions = ((1U << 31) / CNBuffSize) + 1; /* ensure U32 overflow protection is triggered */ + size_t cnb; + assert(cctx != NULL); params.fParams.contentSizeFlag = 0; params.cParams.windowLog = ZSTD_WINDOWLOG_MAX; - for (i = 0; i < nbCompressions; ++i) { + for (cnb = 0; cnb < nbCompressions; ++cnb) { + DISPLAYLEVEL(6, "run %zu / %zu \n", cnb, nbCompressions); CHECK_Z( ZSTD_compressBegin_advanced(cctx, NULL, 0, params, ZSTD_CONTENTSIZE_UNKNOWN) ); /* re-use same parameters */ CHECK_Z( ZSTD_compressEnd(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize) ); }