From 5432ef6921b840c6cfe25f5d8ffcc87f4f91deff Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 13 Dec 2017 17:45:26 -0800 Subject: [PATCH 1/2] fixes adaptation on srcSize This patch restores capability for each file to receive adapted compression parameters depending on its size. The bug breaking this feature was relatively silly : setting a parameter with a value "0" is supposed to be a no-op. Unfortunately, it would pin down compression parameters as if they were manually set, preventing later automatic adaptation. Unfortunately, I'm currently short of a test case that could check this situation and trigger an error. Compression parameters selection between tableID 0,1,2,3 is largely internal, leaving no trace to outside world, not even in frame header. --- lib/compress/zstd_compress.c | 12 ++++++++++-- programs/fileio.c | 2 +- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 8cd288d2a..d52261aae 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -144,6 +144,8 @@ const seqStore_t* ZSTD_getSeqStore(const ZSTD_CCtx* ctx) { return &(ctx->seqStor static ZSTD_compressionParameters ZSTD_getCParamsFromCCtxParams( ZSTD_CCtx_params CCtxParams, U64 srcSizeHint, size_t dictSize) { + DEBUGLOG(4, "ZSTD_getCParamsFromCCtxParams: srcSize = %u, dictSize = %u", + (U32)srcSizeHint, (U32)dictSize); return (CCtxParams.compressionLevel == ZSTD_CLEVEL_CUSTOM) ? CCtxParams.cParams : ZSTD_getCParams(CCtxParams.compressionLevel, srcSizeHint, dictSize); @@ -151,18 +153,22 @@ static ZSTD_compressionParameters ZSTD_getCParamsFromCCtxParams( static void ZSTD_cLevelToCCtxParams_srcSize(ZSTD_CCtx_params* CCtxParams, U64 srcSize) { + DEBUGLOG(4, "ZSTD_cLevelToCCtxParams_srcSize: srcSize = %u", + (U32)srcSize); CCtxParams->cParams = ZSTD_getCParamsFromCCtxParams(*CCtxParams, srcSize, 0); CCtxParams->compressionLevel = ZSTD_CLEVEL_CUSTOM; } static void ZSTD_cLevelToCParams(ZSTD_CCtx* cctx) { + DEBUGLOG(4, "ZSTD_cLevelToCParams"); ZSTD_cLevelToCCtxParams_srcSize( &cctx->requestedParams, cctx->pledgedSrcSizePlusOne-1); } static void ZSTD_cLevelToCCtxParams(ZSTD_CCtx_params* CCtxParams) { + DEBUGLOG(4, "ZSTD_cLevelToCCtxParams"); ZSTD_cLevelToCCtxParams_srcSize(CCtxParams, 0); } @@ -261,7 +267,7 @@ size_t ZSTD_CCtx_setParameter(ZSTD_CCtx* cctx, ZSTD_cParameter param, unsigned v case ZSTD_p_targetLength: case ZSTD_p_compressionStrategy: if (cctx->cdict) return ERROR(stage_wrong); - ZSTD_cLevelToCParams(cctx); /* Can optimize if srcSize is known */ + if (value>0) ZSTD_cLevelToCParams(cctx); /* Can optimize if srcSize is known */ return ZSTD_CCtxParam_setParameter(&cctx->requestedParams, param, value); case ZSTD_p_contentSizeFlag: @@ -422,7 +428,7 @@ size_t ZSTD_CCtxParam_setParameter( #endif case ZSTD_p_enableLongDistanceMatching : - if (value != 0) { + if (value) { ZSTD_cLevelToCCtxParams(CCtxParams); CCtxParams->cParams.windowLog = ZSTD_LDM_DEFAULT_WINDOW_LOG; } @@ -3056,6 +3062,8 @@ ZSTD_compressionParameters ZSTD_getCParams(int compressionLevel, unsigned long l size_t const addedSize = srcSizeHint ? 0 : 500; U64 const rSize = srcSizeHint+dictSize ? srcSizeHint+dictSize+addedSize : (U64)-1; U32 const tableID = (rSize <= 256 KB) + (rSize <= 128 KB) + (rSize <= 16 KB); /* intentional underflow for srcSizeHint == 0 */ + DEBUGLOG(4, "ZSTD_getCParams: cLevel=%i, srcSize=%u, dictSize=%u => table %u", + compressionLevel, (U32)srcSizeHint, (U32)dictSize, tableID); #if defined(ZSTD_DEBUG) && (ZSTD_DEBUG>=1) static int g_monotonicTest = 1; diff --git a/programs/fileio.c b/programs/fileio.c index 4758fa0ed..879954078 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -446,7 +446,7 @@ static cRess_t FIO_createCResources(const char* dictFileName, int cLevel, DISPLAYLEVEL(5,"set nb threads = %u \n", g_nbThreads); CHECK( ZSTD_CCtx_setParameter(ress.cctx, ZSTD_p_nbThreads, g_nbThreads) ); /* dictionary */ - CHECK( ZSTD_CCtx_setPledgedSrcSize(ress.cctx, srcSize) ); /* just for dictionary loading, using good compression parameters */ + CHECK( ZSTD_CCtx_setPledgedSrcSize(ress.cctx, srcSize) ); /* just for dictionary loading, for compression parameters adaptation */ CHECK( ZSTD_CCtx_loadDictionary(ress.cctx, dictBuffer, dictBuffSize) ); CHECK( ZSTD_CCtx_setPledgedSrcSize(ress.cctx, ZSTD_CONTENTSIZE_UNKNOWN) ); /* reset */ From 2e97a6d464d85e82584ba77f971526f1f1fd106b Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 13 Dec 2017 18:50:05 -0800 Subject: [PATCH 2/2] fixed minor declaration-after-statement warning --- lib/compress/zstd_compress.c | 6 ++++-- programs/fileio.c | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index d52261aae..02235c11a 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1738,6 +1738,8 @@ static size_t ZSTD_compress_frameChunk (ZSTD_CCtx* cctx, op += cSize; assert(dstCapacity >= cSize); dstCapacity -= cSize; + DEBUGLOG(5, "ZSTD_compress_frameChunk: adding a block of size %u", + (U32)cSize); } } if (lastFrameChunk && (op>ostart)) cctx->stage = ZSTDcs_ending; @@ -3062,8 +3064,6 @@ ZSTD_compressionParameters ZSTD_getCParams(int compressionLevel, unsigned long l size_t const addedSize = srcSizeHint ? 0 : 500; U64 const rSize = srcSizeHint+dictSize ? srcSizeHint+dictSize+addedSize : (U64)-1; U32 const tableID = (rSize <= 256 KB) + (rSize <= 128 KB) + (rSize <= 16 KB); /* intentional underflow for srcSizeHint == 0 */ - DEBUGLOG(4, "ZSTD_getCParams: cLevel=%i, srcSize=%u, dictSize=%u => table %u", - compressionLevel, (U32)srcSizeHint, (U32)dictSize, tableID); #if defined(ZSTD_DEBUG) && (ZSTD_DEBUG>=1) static int g_monotonicTest = 1; @@ -3073,6 +3073,8 @@ ZSTD_compressionParameters ZSTD_getCParams(int compressionLevel, unsigned long l } #endif + DEBUGLOG(4, "ZSTD_getCParams: cLevel=%i, srcSize=%u, dictSize=%u => table %u", + compressionLevel, (U32)srcSizeHint, (U32)dictSize, tableID); if (compressionLevel <= 0) compressionLevel = ZSTD_CLEVEL_DEFAULT; /* 0 == default; no negative compressionLevel yet */ if (compressionLevel > ZSTD_MAX_CLEVEL) compressionLevel = ZSTD_MAX_CLEVEL; { ZSTD_compressionParameters const cp = ZSTD_defaultCParameters[tableID][compressionLevel]; diff --git a/programs/fileio.c b/programs/fileio.c index 879954078..68032af94 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -781,6 +781,8 @@ static int FIO_compressFilename_internal(cRess_t ress, &outBuff, &inBuff, ZSTD_e_continue) ); /* Write compressed stream */ + DISPLAYLEVEL(6, "ZSTD_compress_generic,ZSTD_e_continue: generated %u bytes \n", + (U32)outBuff.pos); if (outBuff.pos) { size_t const sizeCheck = fwrite(ress.dstBuffer, 1, outBuff.pos, dstFile); if (sizeCheck!=outBuff.pos) @@ -816,6 +818,8 @@ static int FIO_compressFilename_internal(cRess_t ress, EXM_THROW(26, "Compression error during frame end : %s", ZSTD_getErrorName(result)); } + DISPLAYLEVEL(6, "ZSTD_compress_generic,ZSTD_e_end: generated %u bytes \n", + (U32)outBuff.pos); { size_t const sizeCheck = fwrite(ress.dstBuffer, 1, outBuff.pos, dstFile); if (sizeCheck != outBuff.pos) EXM_THROW(27, "Write error : cannot write frame end into %s", dstFileName);