From 590f7f55f0ca68bb1a90043efb014f18c4b07cd1 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 19 Oct 2020 10:26:17 -0400 Subject: [PATCH 1/7] Add ldm enable condition in ZSTD_resetCCtx_internal --- lib/compress/zstd_compress.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index b1bb9fa10..434c23000 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1542,6 +1542,12 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, zc->isFirstBlock = 1; + if (params.cParams.strategy >= ZSTD_btopt && params.cParams.windowLog >= 27) { + /* Enable LDM by default for optimal parser and window size >= 128MB */ + DEBUGLOG(4, "LDM enabled by default (window size >= 128MB, strategy >= btopt)"); + params.ldmParams.enableLdm = 1; + } + if (params.ldmParams.enableLdm) { /* Adjust long distance matching parameters */ ZSTD_ldm_adjustParameters(¶ms.ldmParams, ¶ms.cParams); From df470e176bdd4240a18997f4cf57e100c0ac6755 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 19 Oct 2020 10:52:41 -0400 Subject: [PATCH 2/7] Add unit test for no cctx requested params change --- tests/fuzzer.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 302bfaf62..8c310f3d4 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1196,6 +1196,26 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3d : ldm conditionally enabled by default doesn't change cctx params: ", testNb++); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + ZSTD_outBuffer out = {NULL, 0, 0}; + ZSTD_inBuffer in = {NULL, 0, 0}; + int value; + + /* Even if LDM will be enabled by default in the applied params (since wlog >= 27 and strategy >= btopt), + * we should not modify the actual parameter specified by the user within the CCtx + */ + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_windowLog, 27)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_strategy, ZSTD_btopt)); + + CHECK_Z(ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue)); + CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_enableLongDistanceMatching, &value)); + CHECK_EQ(value, 0); + + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); + /* this test is really too long, and should be made faster */ DISPLAYLEVEL(3, "test%3d : overflow protection with large windowLog : ", testNb++); { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); From aad436da37168bfa4860db92c2669905426cefc3 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 19 Oct 2020 11:02:29 -0400 Subject: [PATCH 3/7] Document ldm enabled by default in zstd.h --- lib/zstd.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/zstd.h b/lib/zstd.h index 75d5bb237..733595d9c 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -339,7 +339,9 @@ typedef enum { * for large inputs, by finding large matches at long distance. * It increases memory usage and window size. * Note: enabling this parameter increases default ZSTD_c_windowLog to 128 MB - * except when expressly set to a different value. */ + * except when expressly set to a different value. + * Note: will be enabled by default if ZSTD_c_windowLog >= 128 MB and + * compression strategy >= ZSTD_btopt (== compression level 16+) */ ZSTD_c_ldmHashLog=161, /* Size of the table for long distance matching, as a power of 2. * Larger values increase memory usage and compression ratio, * but decrease compression speed. From b1c7fc57687e208b82bcbe46387f6de5d5052d94 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 19 Oct 2020 12:07:06 -0400 Subject: [PATCH 4/7] Add compatibility for multithreading --- lib/compress/zstd_compress.c | 2 +- lib/compress/zstdmt_compress.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 434c23000..0bfaeec4f 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1542,7 +1542,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, zc->isFirstBlock = 1; - if (params.cParams.strategy >= ZSTD_btopt && params.cParams.windowLog >= 27) { + if (params.cParams.strategy >= ZSTD_btopt && params.cParams.windowLog >= 27 && params.nbWorkers == 0) { /* Enable LDM by default for optimal parser and window size >= 128MB */ DEBUGLOG(4, "LDM enabled by default (window size >= 128MB, strategy >= btopt)"); params.ldmParams.enableLdm = 1; diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index d68f47189..bf4782cdd 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -1459,6 +1459,13 @@ size_t ZSTDMT_initCStream_internal( mtctx->params = params; mtctx->frameContentSize = pledgedSrcSize; + + if (params.cParams.strategy >= ZSTD_btopt && params.cParams.windowLog >= 27) { + /* Enable LDM by default for optimal parser and window size >= 128MB */ + DEBUGLOG(4, "LDM enabled by default (window size >= 128MB, strategy >= btopt)"); + params.ldmParams.enableLdm = 1; + } + if (dict) { ZSTD_freeCDict(mtctx->cdictLocal); mtctx->cdictLocal = ZSTD_createCDict_advanced(dict, dictSize, From d28d8a1d725ca1885a69f012dd0525d59c5b066e Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 19 Oct 2020 15:22:10 -0400 Subject: [PATCH 5/7] Include LDM tables size for CCtx size estimation where relevant --- lib/compress/zstd_compress.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 0bfaeec4f..5e7fd5374 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -209,6 +209,16 @@ static ZSTD_CCtx_params ZSTD_makeCCtxParamsFromCParams( /* should not matter, as all cParams are presumed properly defined */ ZSTD_CCtxParams_init(&cctxParams, ZSTD_CLEVEL_DEFAULT); cctxParams.cParams = cParams; + + if (cParams.strategy >= ZSTD_btopt && cParams.windowLog >= 27 && cctxParams.nbWorkers == 0) { + DEBUGLOG(4, "ZSTD_makeCCtxParamsFromCParams(): Including LDM into cctx params"); + cctxParams.ldmParams.enableLdm = 1; + /* LDM is enabled by default for optimal parser and window size >= 128MB */ + ZSTD_ldm_adjustParameters(&cctxParams.ldmParams, &cParams); + assert(cctxParams.ldmParams.hashLog >= cctxParams.ldmParams.bucketSizeLog); + assert(cctxParams.ldmParams.hashRateLog < 32); + } + assert(!ZSTD_checkCParams(cParams)); return cctxParams; } From 578e889ec1797b96f62cf73480d0ba3b3823b79c Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 20 Oct 2020 13:01:04 -0400 Subject: [PATCH 6/7] Move ldm enable to compressStream2() --- lib/compress/zstd_compress.c | 11 +++++------ lib/compress/zstdmt_compress.c | 7 ------- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 5e7fd5374..6d5da4b31 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1552,12 +1552,6 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, zc->isFirstBlock = 1; - if (params.cParams.strategy >= ZSTD_btopt && params.cParams.windowLog >= 27 && params.nbWorkers == 0) { - /* Enable LDM by default for optimal parser and window size >= 128MB */ - DEBUGLOG(4, "LDM enabled by default (window size >= 128MB, strategy >= btopt)"); - params.ldmParams.enableLdm = 1; - } - if (params.ldmParams.enableLdm) { /* Adjust long distance matching parameters */ ZSTD_ldm_adjustParameters(¶ms.ldmParams, ¶ms.cParams); @@ -4194,6 +4188,11 @@ size_t ZSTD_compressStream2( ZSTD_CCtx* cctx, dictSize, mode); } + if (params.cParams.strategy >= ZSTD_btopt && params.cParams.windowLog >= 27) { + /* Enable LDM by default for optimal parser and window size >= 128MB */ + DEBUGLOG(4, "LDM enabled by default (window size >= 128MB, strategy >= btopt)"); + params.ldmParams.enableLdm = 1; + } #ifdef ZSTD_MULTITHREAD if ((cctx->pledgedSrcSizePlusOne-1) <= ZSTDMT_JOBSIZE_MIN) { diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index bf4782cdd..d68f47189 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -1459,13 +1459,6 @@ size_t ZSTDMT_initCStream_internal( mtctx->params = params; mtctx->frameContentSize = pledgedSrcSize; - - if (params.cParams.strategy >= ZSTD_btopt && params.cParams.windowLog >= 27) { - /* Enable LDM by default for optimal parser and window size >= 128MB */ - DEBUGLOG(4, "LDM enabled by default (window size >= 128MB, strategy >= btopt)"); - params.ldmParams.enableLdm = 1; - } - if (dict) { ZSTD_freeCDict(mtctx->cdictLocal); mtctx->cdictLocal = ZSTD_createCDict_advanced(dict, dictSize, From 8bdb32aebe181ebfa62237be49e9d183c2daf596 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 20 Oct 2020 13:46:02 -0400 Subject: [PATCH 7/7] Add a function for LDM enable check --- lib/compress/zstd_compress.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 6d5da4b31..404f38dac 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -202,6 +202,14 @@ size_t ZSTD_sizeof_CStream(const ZSTD_CStream* zcs) /* private API call, for dictBuilder only */ const seqStore_t* ZSTD_getSeqStore(const ZSTD_CCtx* ctx) { return &(ctx->seqStore); } +/* Returns 1 if compression parameters are such that we should + * enable long distance matching (wlog >= 27, strategy >= btopt). + * Returns 0 otherwise. + */ +static U32 ZSTD_CParams_shouldEnableLdm(const ZSTD_compressionParameters* const cParams) { + return cParams->strategy >= ZSTD_btopt && cParams->windowLog >= 27; +} + static ZSTD_CCtx_params ZSTD_makeCCtxParamsFromCParams( ZSTD_compressionParameters cParams) { @@ -210,7 +218,7 @@ static ZSTD_CCtx_params ZSTD_makeCCtxParamsFromCParams( ZSTD_CCtxParams_init(&cctxParams, ZSTD_CLEVEL_DEFAULT); cctxParams.cParams = cParams; - if (cParams.strategy >= ZSTD_btopt && cParams.windowLog >= 27 && cctxParams.nbWorkers == 0) { + if (ZSTD_CParams_shouldEnableLdm(&cParams)) { DEBUGLOG(4, "ZSTD_makeCCtxParamsFromCParams(): Including LDM into cctx params"); cctxParams.ldmParams.enableLdm = 1; /* LDM is enabled by default for optimal parser and window size >= 128MB */ @@ -4188,7 +4196,7 @@ size_t ZSTD_compressStream2( ZSTD_CCtx* cctx, dictSize, mode); } - if (params.cParams.strategy >= ZSTD_btopt && params.cParams.windowLog >= 27) { + if (ZSTD_CParams_shouldEnableLdm(¶ms.cParams)) { /* Enable LDM by default for optimal parser and window size >= 128MB */ DEBUGLOG(4, "LDM enabled by default (window size >= 128MB, strategy >= btopt)"); params.ldmParams.enableLdm = 1;