From f933668d3f9dc527528ce13b58bfa78509dbcc2e Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Wed, 23 Dec 2020 15:46:22 -0500 Subject: [PATCH 01/11] Implement hashset for dictIDs --- lib/decompress/zstd_decompress.c | 94 ++++++++++++++++++++++++++++++++ programs/have_lzma.c | 2 + 2 files changed, 96 insertions(+) create mode 100644 programs/have_lzma.c diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 172d46c04..c3d4552f8 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1307,6 +1307,100 @@ size_t ZSTD_freeDStream(ZSTD_DStream* zds) size_t ZSTD_DStreamInSize(void) { return ZSTD_BLOCKSIZE_MAX + ZSTD_blockHeaderSize; } size_t ZSTD_DStreamOutSize(void) { return ZSTD_BLOCKSIZE_MAX; } + +/*************************** + * Multiple DDicts HashSet * + ***************************/ + +#define DDICT_HASHSET_MAX_LOAD_FACTOR 0.75 +#define DDICT_HASHSET_TABLE_BASE_SIZE 64 +#define DDICT_HASHSET_RESIZE_FACTOR 2 + +/* Hashset using linear probing */ +typedef struct { + ZSTD_DDict** ddictPtrTable; + size_t ddictPtrTableSize; + size_t ddictPtrCount; +} ZSTD_DDictHashSet; + +/* Hash function to determine starting position of dict insertion */ +static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, size_t seed) { + return seed % hashSet->ddictPtrTableSize; +} + +/* Adds ddict to a hashset without any chance of resizing it + * Returns 0 on success or a zstd error code + */ +static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, ZSTD_DDict* ddict, int dictID) { + RETURN_ERROR_IF(hashSet->ddictPtrCount == hashSet->ddictPtrTableSize, GENERIC, "Hash set is full!"); + size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); + while (hashSet->ddictPtrTable[idx] != NULL) { + /* Replace existing ddict if inserting ddict with same dictID */ + if (ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]) == dictID) break; + idx++; + } + hashSet->ddictPtrTable[idx] = ddict; + hashSet->ddictPtrCount++; + return 0; +} + +/* Expands hash table by factor of DDICT_HASHSET_RESIZE_FACTOR and rehashes all values */ +static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { + size_t newTableSize = hashSet->ddictPtrTableSize * DDICT_HASHSET_RESIZE_FACTOR; + ZSTD_DDict** newTable = (ZSTD_DDict**)ZSTD_customCalloc(sizeof(ZSTD_DDict*) * newTableSize, customMem); + ZSTD_DDict** oldTable = hashSet->ddictPtrTable; + size_t oldTableSize = hashSet->ddictPtrTableSize; + size_t i = 0; + + RETURN_ERROR_IF(!newTable, memory_allocation, "Expanded hashset allocation failed!"); + hashSet->ddictPtrTable = newTable; + hashSet->ddictPtrTableSize = newTableSize; + hashSet->ddictPtrCount = 0; + for (i = 0; i < oldTableSize; ++i) { + if (oldTable[i] != NULL) { + FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, oldTable[i], ZSTD_getDictID_fromDDict(oldTable[i])), ""); + } + } + ZSTD_customFree(oldTable, customMem); + return 0; +} + +/* Adds a DDict into the ZSTD_DDictHashSet, possibly triggering a resize of the hash set. + * Returns 0 on success, or a ZSTD error. + */ +static size_t ZSTD_DDictHashSet_addDDict(ZSTD_DDictHashSet* hashSet, ZSTD_DDict* ddict, int dictID, ZSTD_customMem customMem) { + if ((float)hashSet->ddictPtrCount / (float)hashSet->ddictPtrTableSize > DDICT_HASHSET_MAX_LOAD_FACTOR) { + FORWARD_IF_ERROR(ZSTD_DDictHashSet_expand(hashSet, customMem), ""); + } + FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, ddict, dictID), ""); + return 0; +} + +/* Fetches a DDict with the given dictID */ +static ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, int dictID) { + size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); + for (;;) { + size_t currDictID = ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]); + if (currDictID == dictID || currDictID == 0) { + /* currDictID == 0 implies a NULL ddict entry */ + break; + } else { + idx++; + } + } + return hashSet->ddictPtrTable[idx]; +} + +/* Allocates space for and returns a ddict hash set */ +static ZSTD_DDictHashSet* ZSTD_createDDictHashSet(ZSTD_customMem customMem) { + ZSTD_DDictHashSet* ret = (ZSTD_DDictHashSet*)ZSTD_customMalloc(sizeof(ZSTD_DDictHashSet), customMem); + ret->ddictPtrTable = (ZSTD_DDict**)ZSTD_customCalloc(DDICT_HASHSET_TABLE_BASE_SIZE * sizeof(ZSTD_DDict*), customMem); + ret->ddictPtrTableSize = DDICT_HASHSET_TABLE_BASE_SIZE; + ret->ddictPtrCount = 0; + return ret; +} + + size_t ZSTD_DCtx_loadDictionary_advanced(ZSTD_DCtx* dctx, const void* dict, size_t dictSize, ZSTD_dictLoadMethod_e dictLoadMethod, diff --git a/programs/have_lzma.c b/programs/have_lzma.c new file mode 100644 index 000000000..bad7f6e8a --- /dev/null +++ b/programs/have_lzma.c @@ -0,0 +1,2 @@ +#include +int main(void) { return 0; } \ No newline at end of file From fd5b608f1ca9dcf883d58bb253ee951117b4ce24 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Wed, 23 Dec 2020 16:09:31 -0500 Subject: [PATCH 02/11] Add parameter to control multiple DDicts --- lib/decompress/zstd_decompress.c | 9 +++++++++ lib/decompress/zstd_decompress_internal.h | 1 + lib/zstd.h | 23 ++++++++++++++++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index c3d4552f8..3cb94eae9 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1530,6 +1530,9 @@ ZSTD_bounds ZSTD_dParam_getBounds(ZSTD_dParameter dParam) bounds.lowerBound = (int)ZSTD_d_validateChecksum; bounds.upperBound = (int)ZSTD_d_ignoreChecksum; return bounds; + case ZSTD_d_refMultipleDDicts: + bounds.lowerBound = (int)ZSTD_d_refSingleDict; + bounds.upperBound = (int)ZSTD_d_refMultipleDicts; default:; } bounds.error = ERROR(parameter_unsupported); @@ -1567,6 +1570,9 @@ size_t ZSTD_DCtx_getParameter(ZSTD_DCtx* dctx, ZSTD_dParameter param, int* value case ZSTD_d_forceIgnoreChecksum: *value = (int)dctx->forceIgnoreChecksum; return 0; + case ZSTD_d_refMultipleDDicts: + *value = (int)dctx->refMultipleDDicts; + return 0; default:; } RETURN_ERROR(parameter_unsupported, ""); @@ -1593,6 +1599,9 @@ size_t ZSTD_DCtx_setParameter(ZSTD_DCtx* dctx, ZSTD_dParameter dParam, int value CHECK_DBOUNDS(ZSTD_d_forceIgnoreChecksum, value); dctx->forceIgnoreChecksum = (ZSTD_forceIgnoreChecksum_e)value; return 0; + case ZSTD_d_refMultipleDDicts: + CHECK_DBOUNDS(ZSTD_d_refMultipleDDicts, value); + dctx->refMultipleDDicts = (ZSTD_refMultipleDDicts_e)value; default:; } RETURN_ERROR(parameter_unsupported, ""); diff --git a/lib/decompress/zstd_decompress_internal.h b/lib/decompress/zstd_decompress_internal.h index 453487de5..3724521e5 100644 --- a/lib/decompress/zstd_decompress_internal.h +++ b/lib/decompress/zstd_decompress_internal.h @@ -136,6 +136,7 @@ struct ZSTD_DCtx_s U32 dictID; int ddictIsCold; /* if == 1 : dictionary is "new" for working context, and presumed "cold" (not in cpu cache) */ ZSTD_dictUses_e dictUses; + ZSTD_refMultipleDDicts_e refMultipleDDicts; /* User specified: if == 1, will allow references to multiple DDicts. Default == 0 (disabled) */ /* streaming */ ZSTD_dStreamStage streamStage; diff --git a/lib/zstd.h b/lib/zstd.h index 4ff03e1e9..b240f2dd4 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -546,12 +546,14 @@ typedef enum { * ZSTD_d_format * ZSTD_d_stableOutBuffer * ZSTD_d_forceIgnoreChecksum + * ZSTD_d_refMultipleDDicts * Because they are not stable, it's necessary to define ZSTD_STATIC_LINKING_ONLY to access them. * note : never ever use experimentalParam? names directly */ ZSTD_d_experimentalParam1=1000, ZSTD_d_experimentalParam2=1001, - ZSTD_d_experimentalParam3=1002 + ZSTD_d_experimentalParam3=1002, + ZSTD_d_experimentalParam4=1003 } ZSTD_dParameter; @@ -1205,6 +1207,12 @@ typedef enum { ZSTD_d_ignoreChecksum = 1 } ZSTD_forceIgnoreChecksum_e; +typedef enum { + /* Note: this enum controls ZSTD_d_refMultipleDDicts */ + ZSTD_d_refSingleDict = 0, + ZSTD_d_refMultipleDicts = 1, +} ZSTD_refMultipleDDicts_e; + typedef enum { /* Note: this enum and the behavior it controls are effectively internal * implementation details of the compressor. They are expected to continue @@ -2000,6 +2008,19 @@ ZSTDLIB_API size_t ZSTD_DCtx_getParameter(ZSTD_DCtx* dctx, ZSTD_dParameter param */ #define ZSTD_d_forceIgnoreChecksum ZSTD_d_experimentalParam3 +/* ZSTD_d_refMultipleDDicts + * Experimental parameter. + * Default is 0 == disabled. Set to 1 to enable + * + * If enabled and dctx is allocated on the heap, then additional memory will be allocated + * to store references to multiple ZSTD_DDict. That is, multiple calls of ZSTD_refDDict() + * using a given ZSTD_DCtx, rather than overwriting the previous DCtx referenced, will + * store all references, and at decompression time, the appropriate dictID is selected + * from the set of DDicts based on the dictID in the frame. + */ +#define ZSTD_d_refMultipleDDicts ZSTD_d_experimentalParam4 + + /*! ZSTD_DCtx_setFormat() : * Instruct the decoder context about what kind of data to decode next. * This instruction is mandatory to decode data without a fully-formed header, From 5a6d3eef2b54111f6946c91c3454d6ef9318139e Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Wed, 23 Dec 2020 16:33:57 -0500 Subject: [PATCH 03/11] Allocate memory for DDict hash set when parameter is set --- lib/decompress/zstd_decompress.c | 22 ++++++++++++++++++++++ lib/decompress/zstd_decompress_internal.h | 1 + 2 files changed, 23 insertions(+) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 3cb94eae9..21fae1ae6 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -122,6 +122,8 @@ static void ZSTD_initDCtx_internal(ZSTD_DCtx* dctx) dctx->bmi2 = ZSTD_cpuid_bmi2(ZSTD_cpuid()); ZSTD_DCtx_resetParameters(dctx); dctx->validateChecksum = 1; + dctx->refMultipleDDicts = 0; + dctx->ddictSet = NULL; #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION dctx->dictContentEndForFuzzing = NULL; #endif @@ -178,6 +180,10 @@ size_t ZSTD_freeDCtx(ZSTD_DCtx* dctx) if (dctx->legacyContext) ZSTD_freeLegacyStreamContext(dctx->legacyContext, dctx->previousLegacyVersion); #endif + if (dctx->ddictSet) { + ZSTD_freeDDictHashSet(dctx->ddictSet, cMem); + dctx->ddictSet = NULL; + } ZSTD_customFree(dctx, cMem); return 0; } @@ -1400,6 +1406,10 @@ static ZSTD_DDictHashSet* ZSTD_createDDictHashSet(ZSTD_customMem customMem) { return ret; } +static void ZSTD_freeDDictHashSet(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { + if (hashSet->ddictPtrTable) ZSTD_customFree(hashSet->ddictPtrTable, customMem); + ZSTD_customFree(hashSet, customMem); +} size_t ZSTD_DCtx_loadDictionary_advanced(ZSTD_DCtx* dctx, const void* dict, size_t dictSize, @@ -1602,6 +1612,18 @@ size_t ZSTD_DCtx_setParameter(ZSTD_DCtx* dctx, ZSTD_dParameter dParam, int value case ZSTD_d_refMultipleDDicts: CHECK_DBOUNDS(ZSTD_d_refMultipleDDicts, value); dctx->refMultipleDDicts = (ZSTD_refMultipleDDicts_e)value; + if (dctx->refMultipleDDicts == ZSTD_d_refMultipleDicts) { + if (dctx->ddictSet == NULL) { + if (dctx->staticSize != 0) { + RETURN_ERROR(memory_allocation, "Multiple DDicts are not allowed with static dctx!"); + } + dctx->ddictSet = ZSTD_createDDictHashSet(dctx->customMem); + } + } else { + ZSTD_freeDDictHashSet(dctx->ddictSet, dctx->customMem); + dctx->ddictSet = NULL; + } + return 0; default:; } RETURN_ERROR(parameter_unsupported, ""); diff --git a/lib/decompress/zstd_decompress_internal.h b/lib/decompress/zstd_decompress_internal.h index 3724521e5..053ecc162 100644 --- a/lib/decompress/zstd_decompress_internal.h +++ b/lib/decompress/zstd_decompress_internal.h @@ -136,6 +136,7 @@ struct ZSTD_DCtx_s U32 dictID; int ddictIsCold; /* if == 1 : dictionary is "new" for working context, and presumed "cold" (not in cpu cache) */ ZSTD_dictUses_e dictUses; + ZSTD_DDictHashSet* ddictSet; /* Hash set for multiple ddicts */ ZSTD_refMultipleDDicts_e refMultipleDDicts; /* User specified: if == 1, will allow references to multiple DDicts. Default == 0 (disabled) */ /* streaming */ From d1a6a9d285d3a84ca30574b40a4e613322276f3c Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 28 Dec 2020 10:40:29 -0500 Subject: [PATCH 04/11] Reference requested dict ID at decompression time --- lib/decompress/zstd_decompress.c | 34 +++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 21fae1ae6..abcb48320 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -648,6 +648,16 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx, ip += frameHeaderSize; remainingSrcSize -= frameHeaderSize; } + /* Reference DDict requested by frame if dctx references multiple ddicts */ + if (dctx->refMultipleDDicts && dctx->ddictSet) { + ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(dctx->ddictSet, dctx->fParams.dictID); + if (frameDDict) { + ZSTD_clearDict(dctx); + dctx->ddict = frameDDict; + dctx->dictUses = ZSTD_use_indefinitely; + } + } + /* Loop on each block */ while (1) { size_t decodedSize; @@ -1495,6 +1505,10 @@ size_t ZSTD_DCtx_refDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) if (ddict) { dctx->ddict = ddict; dctx->dictUses = ZSTD_use_indefinitely; + if (dctx->refMultipleDDicts && dctx->ddictSet) { + assert(!dctx->staticSize); /* Impossible: ddictSet cannot have been allocated if static dctx */ + ZSTD_DDictHashSet_addDDict(dctx->ddictSet, ddict, ZSTD_getDictID_fromDDict(ddict), dctx->customMem); + } } return 0; } @@ -1611,14 +1625,12 @@ size_t ZSTD_DCtx_setParameter(ZSTD_DCtx* dctx, ZSTD_dParameter dParam, int value return 0; case ZSTD_d_refMultipleDDicts: CHECK_DBOUNDS(ZSTD_d_refMultipleDDicts, value); + if (dctx->staticSize != 0) { + RETURN_ERROR(parameter_unsupported, "Static dctx does not support multiple DDicts!"); + } dctx->refMultipleDDicts = (ZSTD_refMultipleDDicts_e)value; - if (dctx->refMultipleDDicts == ZSTD_d_refMultipleDicts) { - if (dctx->ddictSet == NULL) { - if (dctx->staticSize != 0) { - RETURN_ERROR(memory_allocation, "Multiple DDicts are not allowed with static dctx!"); - } - dctx->ddictSet = ZSTD_createDDictHashSet(dctx->customMem); - } + if (dctx->refMultipleDDicts == ZSTD_d_refMultipleDicts && dctx->ddictSet == NULL) { + dctx->ddictSet = ZSTD_createDDictHashSet(dctx->customMem); } else { ZSTD_freeDDictHashSet(dctx->ddictSet, dctx->customMem); dctx->ddictSet = NULL; @@ -1805,6 +1817,14 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB } } #endif { size_t const hSize = ZSTD_getFrameHeader_advanced(&zds->fParams, zds->headerBuffer, zds->lhSize, zds->format); + if (zds->refMultipleDDicts && zds->ddictSet) { + ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(zds->ddictSet, zds->fParams.dictID); + if (frameDDict) { + ZSTD_clearDict(zds); + zds->ddict = frameDDict; + zds->dictUses = ZSTD_use_indefinitely; + } + } DEBUGLOG(5, "header size : %u", (U32)hSize); if (ZSTD_isError(hSize)) { #if defined(ZSTD_LEGACY_SUPPORT) && (ZSTD_LEGACY_SUPPORT>=1) From 7c1a79f232f0661a0e119aea6e57d9a8e40c96f4 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 28 Dec 2020 11:59:58 -0500 Subject: [PATCH 05/11] Add debuglog statements --- lib/decompress/zstd_decompress.c | 233 +++++++++++----------- lib/decompress/zstd_decompress_internal.h | 7 + lib/zstd.h | 3 + programs/have_lzma.c | 2 - 4 files changed, 132 insertions(+), 113 deletions(-) delete mode 100644 programs/have_lzma.c diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index abcb48320..ce6c9fe4f 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -72,6 +72,108 @@ #endif + +/*************************** + * Multiple DDicts Hashset * + ***************************/ + +#define DDICT_HASHSET_MAX_LOAD_FACTOR 0.75 +#define DDICT_HASHSET_TABLE_BASE_SIZE 64 +#define DDICT_HASHSET_RESIZE_FACTOR 2 + +/* Hash function to determine starting position of dict insertion */ +static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, size_t seed) { + return seed % hashSet->ddictPtrTableSize; +} + +/* Adds ddict to a hashset without any chance of resizing it + * Returns 0 on success or a zstd error code + */ +static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, int dictID) { + RETURN_ERROR_IF(hashSet->ddictPtrCount == hashSet->ddictPtrTableSize, GENERIC, "Hash set is full!"); + size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); + DEBUGLOG(2, "Hashed index: for dictID: %d is %zu", dictID, idx); + while (hashSet->ddictPtrTable[idx] != NULL) { + /* Replace existing ddict if inserting ddict with same dictID */ + if (ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]) == dictID) { + DEBUGLOG(2, "DictID already resides here!"); + break; + } + idx++; + } + DEBUGLOG(2, "Final idx after probing for dictID %d is: %zu", dictID, idx); + hashSet->ddictPtrTable[idx] = ddict; + hashSet->ddictPtrCount++; + return 0; +} + +/* Expands hash table by factor of DDICT_HASHSET_RESIZE_FACTOR and rehashes all values */ +static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { + size_t newTableSize = hashSet->ddictPtrTableSize * DDICT_HASHSET_RESIZE_FACTOR; + const ZSTD_DDict** newTable = (const ZSTD_DDict**)ZSTD_customCalloc(sizeof(ZSTD_DDict*) * newTableSize, customMem); + const ZSTD_DDict** oldTable = hashSet->ddictPtrTable; + size_t oldTableSize = hashSet->ddictPtrTableSize; + size_t i = 0; + + DEBUGLOG(2, "Expanding DDict hash table! Old size: %zu new size: %zu", oldTableSize, newTableSize); + RETURN_ERROR_IF(!newTable, memory_allocation, "Expanded hashset allocation failed!"); + hashSet->ddictPtrTable = newTable; + hashSet->ddictPtrTableSize = newTableSize; + hashSet->ddictPtrCount = 0; + for (i = 0; i < oldTableSize; ++i) { + if (oldTable[i] != NULL) { + FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, oldTable[i], ZSTD_getDictID_fromDDict(oldTable[i])), ""); + } + } + ZSTD_customFree(oldTable, customMem); + return 0; +} + +/* Adds a DDict into the ZSTD_DDictHashSet, possibly triggering a resize of the hash set. + * Returns 0 on success, or a ZSTD error. + */ +static size_t ZSTD_DDictHashSet_addDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, int dictID, ZSTD_customMem customMem) { + DEBUGLOG(2, "Adding dict ID: %d to set. Count: %zu Tablesize: %zu", dictID, hashSet->ddictPtrCount, hashSet->ddictPtrTableSize); + if ((float)hashSet->ddictPtrCount / (float)hashSet->ddictPtrTableSize > DDICT_HASHSET_MAX_LOAD_FACTOR) { + FORWARD_IF_ERROR(ZSTD_DDictHashSet_expand(hashSet, customMem), ""); + } + FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, ddict, dictID), ""); + return 0; +} + +/* Fetches a DDict with the given dictID */ +static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, int dictID) { + size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); + DEBUGLOG(2, "Hashed index: for dictID: %d is %zu", dictID, idx); + for (;;) { + size_t currDictID = ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]); + if (currDictID == dictID || currDictID == 0) { + /* currDictID == 0 implies a NULL ddict entry */ + break; + } else { + idx++; + } + } + DEBUGLOG(2, "Final idx after probing for dictID %d is: %zu", dictID, idx); + return hashSet->ddictPtrTable[idx]; +} + +/* Allocates space for and returns a ddict hash set */ +static ZSTD_DDictHashSet* ZSTD_createDDictHashSet(ZSTD_customMem customMem) { + ZSTD_DDictHashSet* ret = (ZSTD_DDictHashSet*)ZSTD_customMalloc(sizeof(ZSTD_DDictHashSet), customMem); + ret->ddictPtrTable = (const ZSTD_DDict**)ZSTD_customCalloc(DDICT_HASHSET_TABLE_BASE_SIZE * sizeof(ZSTD_DDict*), customMem); + ret->ddictPtrTableSize = DDICT_HASHSET_TABLE_BASE_SIZE; + ret->ddictPtrCount = 0; + return ret; +} + +static void ZSTD_freeDDictHashSet(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { + if (hashSet->ddictPtrTable) ZSTD_customFree(hashSet->ddictPtrTable, customMem); + ZSTD_customFree(hashSet, customMem); +} + + + /*-************************************************************* * Context management ***************************************************************/ @@ -447,12 +549,25 @@ unsigned long long ZSTD_getDecompressedSize(const void* src, size_t srcSize) /** ZSTD_decodeFrameHeader() : * `headerSize` must be the size provided by ZSTD_frameHeaderSize(). + * If multiple DDict references are enabled, also will choose the correct DDict to use. * @return : 0 if success, or an error code, which can be tested using ZSTD_isError() */ static size_t ZSTD_decodeFrameHeader(ZSTD_DCtx* dctx, const void* src, size_t headerSize) { size_t const result = ZSTD_getFrameHeader_advanced(&(dctx->fParams), src, headerSize, dctx->format); if (ZSTD_isError(result)) return result; /* invalid header */ RETURN_ERROR_IF(result>0, srcSize_wrong, "headerSize too small"); + /* Reference DDict requested by frame if dctx references multiple ddicts */ + if (dctx->refMultipleDDicts && dctx->ddictSet) { + DEBUGLOG(2, "Choosing the correct DDict at decompression time!"); + const ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(dctx->ddictSet, dctx->fParams.dictID); + if (frameDDict) { + DEBUGLOG(2, "DDict found!"); + ZSTD_clearDict(dctx); + dctx->dictID = dctx->fParams.dictID; + dctx->ddict = frameDDict; + dctx->dictUses = ZSTD_use_indefinitely; + } + } #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION /* Skip the dictID check in fuzzing mode, because it makes the search * harder. @@ -574,7 +689,6 @@ unsigned long long ZSTD_decompressBound(const void* src, size_t srcSize) return bound; } - /*-************************************************************* * Frame decoding ***************************************************************/ @@ -648,16 +762,6 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx, ip += frameHeaderSize; remainingSrcSize -= frameHeaderSize; } - /* Reference DDict requested by frame if dctx references multiple ddicts */ - if (dctx->refMultipleDDicts && dctx->ddictSet) { - ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(dctx->ddictSet, dctx->fParams.dictID); - if (frameDDict) { - ZSTD_clearDict(dctx); - dctx->ddict = frameDDict; - dctx->dictUses = ZSTD_use_indefinitely; - } - } - /* Loop on each block */ while (1) { size_t decodedSize; @@ -1323,104 +1427,6 @@ size_t ZSTD_freeDStream(ZSTD_DStream* zds) size_t ZSTD_DStreamInSize(void) { return ZSTD_BLOCKSIZE_MAX + ZSTD_blockHeaderSize; } size_t ZSTD_DStreamOutSize(void) { return ZSTD_BLOCKSIZE_MAX; } - -/*************************** - * Multiple DDicts HashSet * - ***************************/ - -#define DDICT_HASHSET_MAX_LOAD_FACTOR 0.75 -#define DDICT_HASHSET_TABLE_BASE_SIZE 64 -#define DDICT_HASHSET_RESIZE_FACTOR 2 - -/* Hashset using linear probing */ -typedef struct { - ZSTD_DDict** ddictPtrTable; - size_t ddictPtrTableSize; - size_t ddictPtrCount; -} ZSTD_DDictHashSet; - -/* Hash function to determine starting position of dict insertion */ -static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, size_t seed) { - return seed % hashSet->ddictPtrTableSize; -} - -/* Adds ddict to a hashset without any chance of resizing it - * Returns 0 on success or a zstd error code - */ -static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, ZSTD_DDict* ddict, int dictID) { - RETURN_ERROR_IF(hashSet->ddictPtrCount == hashSet->ddictPtrTableSize, GENERIC, "Hash set is full!"); - size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); - while (hashSet->ddictPtrTable[idx] != NULL) { - /* Replace existing ddict if inserting ddict with same dictID */ - if (ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]) == dictID) break; - idx++; - } - hashSet->ddictPtrTable[idx] = ddict; - hashSet->ddictPtrCount++; - return 0; -} - -/* Expands hash table by factor of DDICT_HASHSET_RESIZE_FACTOR and rehashes all values */ -static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { - size_t newTableSize = hashSet->ddictPtrTableSize * DDICT_HASHSET_RESIZE_FACTOR; - ZSTD_DDict** newTable = (ZSTD_DDict**)ZSTD_customCalloc(sizeof(ZSTD_DDict*) * newTableSize, customMem); - ZSTD_DDict** oldTable = hashSet->ddictPtrTable; - size_t oldTableSize = hashSet->ddictPtrTableSize; - size_t i = 0; - - RETURN_ERROR_IF(!newTable, memory_allocation, "Expanded hashset allocation failed!"); - hashSet->ddictPtrTable = newTable; - hashSet->ddictPtrTableSize = newTableSize; - hashSet->ddictPtrCount = 0; - for (i = 0; i < oldTableSize; ++i) { - if (oldTable[i] != NULL) { - FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, oldTable[i], ZSTD_getDictID_fromDDict(oldTable[i])), ""); - } - } - ZSTD_customFree(oldTable, customMem); - return 0; -} - -/* Adds a DDict into the ZSTD_DDictHashSet, possibly triggering a resize of the hash set. - * Returns 0 on success, or a ZSTD error. - */ -static size_t ZSTD_DDictHashSet_addDDict(ZSTD_DDictHashSet* hashSet, ZSTD_DDict* ddict, int dictID, ZSTD_customMem customMem) { - if ((float)hashSet->ddictPtrCount / (float)hashSet->ddictPtrTableSize > DDICT_HASHSET_MAX_LOAD_FACTOR) { - FORWARD_IF_ERROR(ZSTD_DDictHashSet_expand(hashSet, customMem), ""); - } - FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, ddict, dictID), ""); - return 0; -} - -/* Fetches a DDict with the given dictID */ -static ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, int dictID) { - size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); - for (;;) { - size_t currDictID = ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]); - if (currDictID == dictID || currDictID == 0) { - /* currDictID == 0 implies a NULL ddict entry */ - break; - } else { - idx++; - } - } - return hashSet->ddictPtrTable[idx]; -} - -/* Allocates space for and returns a ddict hash set */ -static ZSTD_DDictHashSet* ZSTD_createDDictHashSet(ZSTD_customMem customMem) { - ZSTD_DDictHashSet* ret = (ZSTD_DDictHashSet*)ZSTD_customMalloc(sizeof(ZSTD_DDictHashSet), customMem); - ret->ddictPtrTable = (ZSTD_DDict**)ZSTD_customCalloc(DDICT_HASHSET_TABLE_BASE_SIZE * sizeof(ZSTD_DDict*), customMem); - ret->ddictPtrTableSize = DDICT_HASHSET_TABLE_BASE_SIZE; - ret->ddictPtrCount = 0; - return ret; -} - -static void ZSTD_freeDDictHashSet(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { - if (hashSet->ddictPtrTable) ZSTD_customFree(hashSet->ddictPtrTable, customMem); - ZSTD_customFree(hashSet, customMem); -} - size_t ZSTD_DCtx_loadDictionary_advanced(ZSTD_DCtx* dctx, const void* dict, size_t dictSize, ZSTD_dictLoadMethod_e dictLoadMethod, @@ -1507,7 +1513,8 @@ size_t ZSTD_DCtx_refDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) dctx->dictUses = ZSTD_use_indefinitely; if (dctx->refMultipleDDicts && dctx->ddictSet) { assert(!dctx->staticSize); /* Impossible: ddictSet cannot have been allocated if static dctx */ - ZSTD_DDictHashSet_addDDict(dctx->ddictSet, ddict, ZSTD_getDictID_fromDDict(ddict), dctx->customMem); + FORWARD_IF_ERROR(ZSTD_DDictHashSet_addDDict(dctx->ddictSet, ddict, + ZSTD_getDictID_fromDDict(ddict), dctx->customMem), ""); } } return 0; @@ -1557,6 +1564,7 @@ ZSTD_bounds ZSTD_dParam_getBounds(ZSTD_dParameter dParam) case ZSTD_d_refMultipleDDicts: bounds.lowerBound = (int)ZSTD_d_refSingleDict; bounds.upperBound = (int)ZSTD_d_refMultipleDicts; + return bounds; default:; } bounds.error = ERROR(parameter_unsupported); @@ -1625,11 +1633,13 @@ size_t ZSTD_DCtx_setParameter(ZSTD_DCtx* dctx, ZSTD_dParameter dParam, int value return 0; case ZSTD_d_refMultipleDDicts: CHECK_DBOUNDS(ZSTD_d_refMultipleDDicts, value); + DEBUGLOG(2, "Referencing multiple ddicts param enabled"); if (dctx->staticSize != 0) { RETURN_ERROR(parameter_unsupported, "Static dctx does not support multiple DDicts!"); } dctx->refMultipleDDicts = (ZSTD_refMultipleDDicts_e)value; if (dctx->refMultipleDDicts == ZSTD_d_refMultipleDicts && dctx->ddictSet == NULL) { + DEBUGLOG(2, "Allocating new hash set"); dctx->ddictSet = ZSTD_createDDictHashSet(dctx->customMem); } else { ZSTD_freeDDictHashSet(dctx->ddictSet, dctx->customMem); @@ -1818,9 +1828,10 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB #endif { size_t const hSize = ZSTD_getFrameHeader_advanced(&zds->fParams, zds->headerBuffer, zds->lhSize, zds->format); if (zds->refMultipleDDicts && zds->ddictSet) { - ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(zds->ddictSet, zds->fParams.dictID); + const ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(zds->ddictSet, zds->fParams.dictID); if (frameDDict) { ZSTD_clearDict(zds); + zds->dictID = zds->fParams.dictID; zds->ddict = frameDDict; zds->dictUses = ZSTD_use_indefinitely; } diff --git a/lib/decompress/zstd_decompress_internal.h b/lib/decompress/zstd_decompress_internal.h index 053ecc162..8877e4d2c 100644 --- a/lib/decompress/zstd_decompress_internal.h +++ b/lib/decompress/zstd_decompress_internal.h @@ -99,6 +99,13 @@ typedef enum { ZSTD_use_once = 1 /* Use the dictionary once and set to ZSTD_dont_use */ } ZSTD_dictUses_e; +/* Hashset for storing references to multiple ZSTD_DDict within ZSTD_DCtx */ +typedef struct { + const ZSTD_DDict** ddictPtrTable; + size_t ddictPtrTableSize; + size_t ddictPtrCount; +} ZSTD_DDictHashSet; + struct ZSTD_DCtx_s { const ZSTD_seqSymbol* LLTptr; diff --git a/lib/zstd.h b/lib/zstd.h index b240f2dd4..c0b590c8d 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -2017,6 +2017,9 @@ ZSTDLIB_API size_t ZSTD_DCtx_getParameter(ZSTD_DCtx* dctx, ZSTD_dParameter param * using a given ZSTD_DCtx, rather than overwriting the previous DCtx referenced, will * store all references, and at decompression time, the appropriate dictID is selected * from the set of DDicts based on the dictID in the frame. + * + * WARNING: Enabling this parameter will trigger memory allocation for the hash table, and disabling + * this parameter will trigger memory freeing for the hashtable. */ #define ZSTD_d_refMultipleDDicts ZSTD_d_experimentalParam4 diff --git a/programs/have_lzma.c b/programs/have_lzma.c deleted file mode 100644 index bad7f6e8a..000000000 --- a/programs/have_lzma.c +++ /dev/null @@ -1,2 +0,0 @@ -#include -int main(void) { return 0; } \ No newline at end of file From ea52fc36069f05c581020191e5c878e77db10d7c Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 28 Dec 2020 13:14:58 -0500 Subject: [PATCH 06/11] Use XXHash for hash function, create a sensible public interface --- lib/decompress/zstd_decompress.c | 135 ++++++++++++++++++++----------- lib/zstd.h | 3 +- 2 files changed, 88 insertions(+), 50 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index ce6c9fe4f..f4f8269df 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -81,33 +81,45 @@ #define DDICT_HASHSET_TABLE_BASE_SIZE 64 #define DDICT_HASHSET_RESIZE_FACTOR 2 -/* Hash function to determine starting position of dict insertion */ -static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, size_t seed) { - return seed % hashSet->ddictPtrTableSize; +/* Hash function to determine starting position of dict insertion within the table + * Returns an index between [0, hashSet->ddictPtrTableSize] + */ +static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, U32 dictID) { + U32 hash = XXH32(&dictID, sizeof(U32), 0); + /* DDict ptr table size is a multiple of 2, use size - 1 as mask to get index within [0, hashSet->ddictPtrTableSize) */ + return hash & (hashSet->ddictPtrTableSize - 1); } -/* Adds ddict to a hashset without any chance of resizing it - * Returns 0 on success or a zstd error code +/* Adds ddict to a hashset resizing it + * Returns 0 if successful (ddict may or may not have actually been added), or a zstd error code if something went wrong */ -static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, int dictID) { +static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, U32 dictID) { RETURN_ERROR_IF(hashSet->ddictPtrCount == hashSet->ddictPtrTableSize, GENERIC, "Hash set is full!"); size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); - DEBUGLOG(2, "Hashed index: for dictID: %d is %zu", dictID, idx); + DEBUGLOG(4, "Hashed index: for dictID: %u is %zu", dictID, idx); while (hashSet->ddictPtrTable[idx] != NULL) { /* Replace existing ddict if inserting ddict with same dictID */ if (ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]) == dictID) { - DEBUGLOG(2, "DictID already resides here!"); - break; + DEBUGLOG(2, "DictID already exists, replacing rather than adding"); + hashSet->ddictPtrTable[idx] = ddict; + return 0; + } + if (idx == hashSet->ddictPtrTableSize - 1) { + idx = 0; + continue; } idx++; } - DEBUGLOG(2, "Final idx after probing for dictID %d is: %zu", dictID, idx); + DEBUGLOG(4, "Final idx after probing for dictID %u is: %zu", dictID, idx); hashSet->ddictPtrTable[idx] = ddict; hashSet->ddictPtrCount++; return 0; } -/* Expands hash table by factor of DDICT_HASHSET_RESIZE_FACTOR and rehashes all values */ +/* Expands hash table by factor of DDICT_HASHSET_RESIZE_FACTOR and + * rehashes all values, allocates new table, frees old table. + * Returns 0 on success, otherwise a zstd error code. + */ static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { size_t newTableSize = hashSet->ddictPtrTableSize * DDICT_HASHSET_RESIZE_FACTOR; const ZSTD_DDict** newTable = (const ZSTD_DDict**)ZSTD_customCalloc(sizeof(ZSTD_DDict*) * newTableSize, customMem); @@ -115,7 +127,7 @@ static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMe size_t oldTableSize = hashSet->ddictPtrTableSize; size_t i = 0; - DEBUGLOG(2, "Expanding DDict hash table! Old size: %zu new size: %zu", oldTableSize, newTableSize); + DEBUGLOG(4, "Expanding DDict hash table! Old size: %zu new size: %zu", oldTableSize, newTableSize); RETURN_ERROR_IF(!newTable, memory_allocation, "Expanded hashset allocation failed!"); hashSet->ddictPtrTable = newTable; hashSet->ddictPtrTableSize = newTableSize; @@ -126,53 +138,67 @@ static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMe } } ZSTD_customFree(oldTable, customMem); + DEBUGLOG(4, "Finished re-hash"); return 0; } -/* Adds a DDict into the ZSTD_DDictHashSet, possibly triggering a resize of the hash set. - * Returns 0 on success, or a ZSTD error. +/* Fetches a DDict with the given dictID + * Returns the ZSTD_DDict* with the requested dictID. If it doesn't exist, then returns NULL. */ -static size_t ZSTD_DDictHashSet_addDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, int dictID, ZSTD_customMem customMem) { - DEBUGLOG(2, "Adding dict ID: %d to set. Count: %zu Tablesize: %zu", dictID, hashSet->ddictPtrCount, hashSet->ddictPtrTableSize); - if ((float)hashSet->ddictPtrCount / (float)hashSet->ddictPtrTableSize > DDICT_HASHSET_MAX_LOAD_FACTOR) { - FORWARD_IF_ERROR(ZSTD_DDictHashSet_expand(hashSet, customMem), ""); - } - FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, ddict, dictID), ""); - return 0; -} - -/* Fetches a DDict with the given dictID */ -static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, int dictID) { +static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, U32 dictID) { size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); - DEBUGLOG(2, "Hashed index: for dictID: %d is %zu", dictID, idx); + DEBUGLOG(4, "Hashed index: for dictID: %u is %zu", dictID, idx); for (;;) { size_t currDictID = ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]); if (currDictID == dictID || currDictID == 0) { /* currDictID == 0 implies a NULL ddict entry */ + DEBUGLOG(4, "Dict ID not found"); break; } else { + if (idx == hashSet->ddictPtrTableSize - 1) { + idx = 0; + continue; + } idx++; } } - DEBUGLOG(2, "Final idx after probing for dictID %d is: %zu", dictID, idx); + DEBUGLOG(4, "Final idx after probing for dictID %u is: %zu", dictID, idx); return hashSet->ddictPtrTable[idx]; } -/* Allocates space for and returns a ddict hash set */ +/* Allocates space for and returns a ddict hash set + * The hash set's ZSTD_DDict* table has all values automatically set to NULL to begin with. + * Returns NULL if allocation failed. + */ static ZSTD_DDictHashSet* ZSTD_createDDictHashSet(ZSTD_customMem customMem) { ZSTD_DDictHashSet* ret = (ZSTD_DDictHashSet*)ZSTD_customMalloc(sizeof(ZSTD_DDictHashSet), customMem); ret->ddictPtrTable = (const ZSTD_DDict**)ZSTD_customCalloc(DDICT_HASHSET_TABLE_BASE_SIZE * sizeof(ZSTD_DDict*), customMem); ret->ddictPtrTableSize = DDICT_HASHSET_TABLE_BASE_SIZE; ret->ddictPtrCount = 0; + if (!ret || !ret->ddictPtrTable) { + return NULL; + } return ret; } +/* Frees the table of ZSTD_DDict* within a hashset, then frees the hashset itself. + */ static void ZSTD_freeDDictHashSet(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { if (hashSet->ddictPtrTable) ZSTD_customFree(hashSet->ddictPtrTable, customMem); ZSTD_customFree(hashSet, customMem); } - +/* Public function: Adds a DDict into the ZSTD_DDictHashSet, possibly triggering a resize of the hash set. + * Returns 0 on success, or a ZSTD error. + */ +static size_t ZSTD_DDictHashSet_addDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, U32 dictID, ZSTD_customMem customMem) { + DEBUGLOG(4, "Adding dict ID: %d to hashset with - Count: %zu Tablesize: %zu", dictID, hashSet->ddictPtrCount, hashSet->ddictPtrTableSize); + if ((float)hashSet->ddictPtrCount / (float)hashSet->ddictPtrTableSize >= DDICT_HASHSET_MAX_LOAD_FACTOR) { + FORWARD_IF_ERROR(ZSTD_DDictHashSet_expand(hashSet, customMem), ""); + } + FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, ddict, dictID), ""); + return 0; +} /*-************************************************************* * Context management @@ -298,6 +324,24 @@ void ZSTD_copyDCtx(ZSTD_DCtx* dstDCtx, const ZSTD_DCtx* srcDCtx) ZSTD_memcpy(dstDCtx, srcDCtx, toCopy); /* no need to copy workspace */ } +/* Public function: Given a dctx with a digested frame params, re-selects the correct DDict based on + * the requested dict ID from the frame. ZSTD_d_refMultipleDDicts must be enabled for this function + * to be called. + */ +static void ZSTD_DCtx_selectFrameDDict(ZSTD_DCtx* dctx) { + assert(dctx->refMultipleDDicts && dctx->ddictSet); + DEBUGLOG(4, "Adjusting DDict based on requested dict ID from frame"); + { const ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(dctx->ddictSet, dctx->fParams.dictID); + if (frameDDict) { + DEBUGLOG(4, "DDict found!"); + ZSTD_clearDict(dctx); + dctx->dictID = dctx->fParams.dictID; + dctx->ddict = frameDDict; + dctx->dictUses = ZSTD_use_indefinitely; + } + } +} + /*-************************************************************* * Frame header decoding @@ -556,18 +600,12 @@ static size_t ZSTD_decodeFrameHeader(ZSTD_DCtx* dctx, const void* src, size_t he size_t const result = ZSTD_getFrameHeader_advanced(&(dctx->fParams), src, headerSize, dctx->format); if (ZSTD_isError(result)) return result; /* invalid header */ RETURN_ERROR_IF(result>0, srcSize_wrong, "headerSize too small"); + /* Reference DDict requested by frame if dctx references multiple ddicts */ if (dctx->refMultipleDDicts && dctx->ddictSet) { - DEBUGLOG(2, "Choosing the correct DDict at decompression time!"); - const ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(dctx->ddictSet, dctx->fParams.dictID); - if (frameDDict) { - DEBUGLOG(2, "DDict found!"); - ZSTD_clearDict(dctx); - dctx->dictID = dctx->fParams.dictID; - dctx->ddict = frameDDict; - dctx->dictUses = ZSTD_use_indefinitely; - } + ZSTD_DCtx_selectFrameDDict(dctx); } + #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION /* Skip the dictID check in fuzzing mode, because it makes the search * harder. @@ -1633,17 +1671,22 @@ size_t ZSTD_DCtx_setParameter(ZSTD_DCtx* dctx, ZSTD_dParameter dParam, int value return 0; case ZSTD_d_refMultipleDDicts: CHECK_DBOUNDS(ZSTD_d_refMultipleDDicts, value); - DEBUGLOG(2, "Referencing multiple ddicts param enabled"); + DEBUGLOG(4, "Referencing multiple ddicts param enabled"); if (dctx->staticSize != 0) { RETURN_ERROR(parameter_unsupported, "Static dctx does not support multiple DDicts!"); } dctx->refMultipleDDicts = (ZSTD_refMultipleDDicts_e)value; if (dctx->refMultipleDDicts == ZSTD_d_refMultipleDicts && dctx->ddictSet == NULL) { - DEBUGLOG(2, "Allocating new hash set"); + DEBUGLOG(4, "Allocating new hash set"); dctx->ddictSet = ZSTD_createDDictHashSet(dctx->customMem); + if (!dctx->ddictSet) { + RETURN_ERROR(memory_allocation, "Failed to allocate memory for hash set!"); + } } else { - ZSTD_freeDDictHashSet(dctx->ddictSet, dctx->customMem); - dctx->ddictSet = NULL; + if (dctx->ddictSet) { + ZSTD_freeDDictHashSet(dctx->ddictSet, dctx->customMem); + dctx->ddictSet = NULL; + } } return 0; default:; @@ -1828,13 +1871,7 @@ size_t ZSTD_decompressStream(ZSTD_DStream* zds, ZSTD_outBuffer* output, ZSTD_inB #endif { size_t const hSize = ZSTD_getFrameHeader_advanced(&zds->fParams, zds->headerBuffer, zds->lhSize, zds->format); if (zds->refMultipleDDicts && zds->ddictSet) { - const ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(zds->ddictSet, zds->fParams.dictID); - if (frameDDict) { - ZSTD_clearDict(zds); - zds->dictID = zds->fParams.dictID; - zds->ddict = frameDDict; - zds->dictUses = ZSTD_use_indefinitely; - } + ZSTD_DCtx_selectFrameDDict(zds); } DEBUGLOG(5, "header size : %u", (U32)hSize); if (ZSTD_isError(hSize)) { diff --git a/lib/zstd.h b/lib/zstd.h index c0b590c8d..ede9c5785 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -2019,7 +2019,8 @@ ZSTDLIB_API size_t ZSTD_DCtx_getParameter(ZSTD_DCtx* dctx, ZSTD_dParameter param * from the set of DDicts based on the dictID in the frame. * * WARNING: Enabling this parameter will trigger memory allocation for the hash table, and disabling - * this parameter will trigger memory freeing for the hashtable. + * this parameter will free the memory allocated for the hashtable. Memory is allocated as per + * ZSTD_DCtx::customMem. */ #define ZSTD_d_refMultipleDDicts ZSTD_d_experimentalParam4 From 22b7bff2bcdfab8ece992420bd703fb14fd0f49b Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Mon, 28 Dec 2020 16:43:04 -0500 Subject: [PATCH 07/11] Add unit test, improve documentation --- lib/decompress/zstd_decompress.c | 70 +++++++++++++------------ lib/zstd.h | 30 ++++++++--- tests/fuzzer.c | 88 +++++++++++++++++++++++++++----- 3 files changed, 134 insertions(+), 54 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index f4f8269df..72abba3b9 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -73,9 +73,9 @@ -/*************************** - * Multiple DDicts Hashset * - ***************************/ +/************************************* + * Multiple DDicts Hashset internals * + *************************************/ #define DDICT_HASHSET_MAX_LOAD_FACTOR 0.75 #define DDICT_HASHSET_TABLE_BASE_SIZE 64 @@ -85,7 +85,7 @@ * Returns an index between [0, hashSet->ddictPtrTableSize] */ static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, U32 dictID) { - U32 hash = XXH32(&dictID, sizeof(U32), 0); + U32 hash = XXH64(&dictID, sizeof(U32), 0); /* DDict ptr table size is a multiple of 2, use size - 1 as mask to get index within [0, hashSet->ddictPtrTableSize) */ return hash & (hashSet->ddictPtrTableSize - 1); } @@ -94,8 +94,8 @@ static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, U32 d * Returns 0 if successful (ddict may or may not have actually been added), or a zstd error code if something went wrong */ static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, U32 dictID) { - RETURN_ERROR_IF(hashSet->ddictPtrCount == hashSet->ddictPtrTableSize, GENERIC, "Hash set is full!"); size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); + RETURN_ERROR_IF(hashSet->ddictPtrCount == hashSet->ddictPtrTableSize, GENERIC, "Hash set is full!"); DEBUGLOG(4, "Hashed index: for dictID: %u is %zu", dictID, idx); while (hashSet->ddictPtrTable[idx] != NULL) { /* Replace existing ddict if inserting ddict with same dictID */ @@ -125,7 +125,7 @@ static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMe const ZSTD_DDict** newTable = (const ZSTD_DDict**)ZSTD_customCalloc(sizeof(ZSTD_DDict*) * newTableSize, customMem); const ZSTD_DDict** oldTable = hashSet->ddictPtrTable; size_t oldTableSize = hashSet->ddictPtrTableSize; - size_t i = 0; + size_t i; DEBUGLOG(4, "Expanding DDict hash table! Old size: %zu new size: %zu", oldTableSize, newTableSize); RETURN_ERROR_IF(!newTable, memory_allocation, "Expanded hashset allocation failed!"); @@ -152,7 +152,6 @@ static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, size_t currDictID = ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]); if (currDictID == dictID || currDictID == 0) { /* currDictID == 0 implies a NULL ddict entry */ - DEBUGLOG(4, "Dict ID not found"); break; } else { if (idx == hashSet->ddictPtrTableSize - 1) { @@ -172,6 +171,7 @@ static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, */ static ZSTD_DDictHashSet* ZSTD_createDDictHashSet(ZSTD_customMem customMem) { ZSTD_DDictHashSet* ret = (ZSTD_DDictHashSet*)ZSTD_customMalloc(sizeof(ZSTD_DDictHashSet), customMem); + DEBUGLOG(4, "Allocating new hash set"); ret->ddictPtrTable = (const ZSTD_DDict**)ZSTD_customCalloc(DDICT_HASHSET_TABLE_BASE_SIZE * sizeof(ZSTD_DDict*), customMem); ret->ddictPtrTableSize = DDICT_HASHSET_TABLE_BASE_SIZE; ret->ddictPtrCount = 0; @@ -182,10 +182,16 @@ static ZSTD_DDictHashSet* ZSTD_createDDictHashSet(ZSTD_customMem customMem) { } /* Frees the table of ZSTD_DDict* within a hashset, then frees the hashset itself. + * Note: The ZSTD_DDict* within the table are NOT freed. */ static void ZSTD_freeDDictHashSet(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { - if (hashSet->ddictPtrTable) ZSTD_customFree(hashSet->ddictPtrTable, customMem); - ZSTD_customFree(hashSet, customMem); + DEBUGLOG(4, "Freeing ddict hash set"); + if (hashSet && hashSet->ddictPtrTable) { + ZSTD_customFree(hashSet->ddictPtrTable, customMem); + } + if (hashSet) { + ZSTD_customFree(hashSet, customMem); + } } /* Public function: Adds a DDict into the ZSTD_DDictHashSet, possibly triggering a resize of the hash set. @@ -229,6 +235,7 @@ static void ZSTD_DCtx_resetParameters(ZSTD_DCtx* dctx) dctx->maxWindowSize = ZSTD_MAXWINDOWSIZE_DEFAULT; dctx->outBufferMode = ZSTD_bm_buffered; dctx->forceIgnoreChecksum = ZSTD_d_validateChecksum; + dctx->refMultipleDDicts = ZSTD_rmd_refSingleDDict; } static void ZSTD_initDCtx_internal(ZSTD_DCtx* dctx) @@ -248,10 +255,8 @@ static void ZSTD_initDCtx_internal(ZSTD_DCtx* dctx) dctx->noForwardProgress = 0; dctx->oversizedDuration = 0; dctx->bmi2 = ZSTD_cpuid_bmi2(ZSTD_cpuid()); - ZSTD_DCtx_resetParameters(dctx); - dctx->validateChecksum = 1; - dctx->refMultipleDDicts = 0; dctx->ddictSet = NULL; + ZSTD_DCtx_resetParameters(dctx); #ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION dctx->dictContentEndForFuzzing = NULL; #endif @@ -324,14 +329,19 @@ void ZSTD_copyDCtx(ZSTD_DCtx* dstDCtx, const ZSTD_DCtx* srcDCtx) ZSTD_memcpy(dstDCtx, srcDCtx, toCopy); /* no need to copy workspace */ } -/* Public function: Given a dctx with a digested frame params, re-selects the correct DDict based on - * the requested dict ID from the frame. ZSTD_d_refMultipleDDicts must be enabled for this function - * to be called. +/* Given a dctx with a digested frame params, re-selects the correct ZSTD_DDict based on + * the requested dict ID from the frame. If there exists a reference to the correct ZSTD_DDict, then + * accordingly sets the ddict to be used to decompress the frame. + * + * If no DDict is found, then no action is taken, and the ZSTD_DCtx::ddict remains as-is. + * + * ZSTD_d_refMultipleDDicts must be enabled for this function to be called. */ static void ZSTD_DCtx_selectFrameDDict(ZSTD_DCtx* dctx) { assert(dctx->refMultipleDDicts && dctx->ddictSet); DEBUGLOG(4, "Adjusting DDict based on requested dict ID from frame"); - { const ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(dctx->ddictSet, dctx->fParams.dictID); + if (dctx->ddict) { + const ZSTD_DDict* frameDDict = ZSTD_DDictHashSet_getDDict(dctx->ddictSet, dctx->fParams.dictID); if (frameDDict) { DEBUGLOG(4, "DDict found!"); ZSTD_clearDict(dctx); @@ -602,7 +612,7 @@ static size_t ZSTD_decodeFrameHeader(ZSTD_DCtx* dctx, const void* src, size_t he RETURN_ERROR_IF(result>0, srcSize_wrong, "headerSize too small"); /* Reference DDict requested by frame if dctx references multiple ddicts */ - if (dctx->refMultipleDDicts && dctx->ddictSet) { + if (dctx->refMultipleDDicts == ZSTD_rmd_refMultipleDDicts && dctx->ddictSet) { ZSTD_DCtx_selectFrameDDict(dctx); } @@ -727,6 +737,7 @@ unsigned long long ZSTD_decompressBound(const void* src, size_t srcSize) return bound; } + /*-************************************************************* * Frame decoding ***************************************************************/ @@ -1549,7 +1560,13 @@ size_t ZSTD_DCtx_refDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) if (ddict) { dctx->ddict = ddict; dctx->dictUses = ZSTD_use_indefinitely; - if (dctx->refMultipleDDicts && dctx->ddictSet) { + if (dctx->refMultipleDDicts == ZSTD_rmd_refMultipleDDicts) { + if (dctx->ddictSet == NULL) { + dctx->ddictSet = ZSTD_createDDictHashSet(dctx->customMem); + if (!dctx->ddictSet) { + RETURN_ERROR(memory_allocation, "Failed to allocate memory for hash set!"); + } + } assert(!dctx->staticSize); /* Impossible: ddictSet cannot have been allocated if static dctx */ FORWARD_IF_ERROR(ZSTD_DDictHashSet_addDDict(dctx->ddictSet, ddict, ZSTD_getDictID_fromDDict(ddict), dctx->customMem), ""); @@ -1600,8 +1617,8 @@ ZSTD_bounds ZSTD_dParam_getBounds(ZSTD_dParameter dParam) bounds.upperBound = (int)ZSTD_d_ignoreChecksum; return bounds; case ZSTD_d_refMultipleDDicts: - bounds.lowerBound = (int)ZSTD_d_refSingleDict; - bounds.upperBound = (int)ZSTD_d_refMultipleDicts; + bounds.lowerBound = (int)ZSTD_rmd_refSingleDDict; + bounds.upperBound = (int)ZSTD_rmd_refMultipleDDicts; return bounds; default:; } @@ -1671,23 +1688,10 @@ size_t ZSTD_DCtx_setParameter(ZSTD_DCtx* dctx, ZSTD_dParameter dParam, int value return 0; case ZSTD_d_refMultipleDDicts: CHECK_DBOUNDS(ZSTD_d_refMultipleDDicts, value); - DEBUGLOG(4, "Referencing multiple ddicts param enabled"); if (dctx->staticSize != 0) { RETURN_ERROR(parameter_unsupported, "Static dctx does not support multiple DDicts!"); } dctx->refMultipleDDicts = (ZSTD_refMultipleDDicts_e)value; - if (dctx->refMultipleDDicts == ZSTD_d_refMultipleDicts && dctx->ddictSet == NULL) { - DEBUGLOG(4, "Allocating new hash set"); - dctx->ddictSet = ZSTD_createDDictHashSet(dctx->customMem); - if (!dctx->ddictSet) { - RETURN_ERROR(memory_allocation, "Failed to allocate memory for hash set!"); - } - } else { - if (dctx->ddictSet) { - ZSTD_freeDDictHashSet(dctx->ddictSet, dctx->customMem); - dctx->ddictSet = NULL; - } - } return 0; default:; } diff --git a/lib/zstd.h b/lib/zstd.h index ede9c5785..884a2f54a 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1001,6 +1001,13 @@ ZSTDLIB_API size_t ZSTD_DCtx_loadDictionary(ZSTD_DCtx* dctx, const void* dict, s /*! ZSTD_DCtx_refDDict() : * Reference a prepared dictionary, to be used to decompress next frames. * The dictionary remains active for decompression of future frames using same DCtx. + * + * If called with ZSTD_d_refMultipleDDicts enabled, repeated calls of this function + * will store the DDict references in a table, and the DDict used for decompression + * will be determined at decompression time, as per the dict ID in the frame. + * The memory for the table is allocated on the first call to refDDict, and can be + * freed with ZSTD_freeDCtx(). + * * @result : 0, or an error code (which can be tested with ZSTD_isError()). * Note 1 : Currently, only one dictionary can be managed. * Referencing a new dictionary effectively "discards" any previous one. @@ -1209,8 +1216,8 @@ typedef enum { typedef enum { /* Note: this enum controls ZSTD_d_refMultipleDDicts */ - ZSTD_d_refSingleDict = 0, - ZSTD_d_refMultipleDicts = 1, + ZSTD_rmd_refSingleDDict = 0, + ZSTD_rmd_refMultipleDDicts = 1 } ZSTD_refMultipleDDicts_e; typedef enum { @@ -2014,13 +2021,20 @@ ZSTDLIB_API size_t ZSTD_DCtx_getParameter(ZSTD_DCtx* dctx, ZSTD_dParameter param * * If enabled and dctx is allocated on the heap, then additional memory will be allocated * to store references to multiple ZSTD_DDict. That is, multiple calls of ZSTD_refDDict() - * using a given ZSTD_DCtx, rather than overwriting the previous DCtx referenced, will - * store all references, and at decompression time, the appropriate dictID is selected - * from the set of DDicts based on the dictID in the frame. + * using a given ZSTD_DCtx, rather than overwriting the previous DDict reference, will instead + * store all references. At decompression time, the appropriate dictID is selected + * from the set of DDicts based on the dictID in the frame. * - * WARNING: Enabling this parameter will trigger memory allocation for the hash table, and disabling - * this parameter will free the memory allocated for the hashtable. Memory is allocated as per - * ZSTD_DCtx::customMem. + * Usage is simply calling ZSTD_refDDict() on multiple dict buffers. + * + * Param has values of byte ZSTD_refMultipleDDicts_e + * + * WARNING: Enabling this parameter and calling ZSTD_DCtx_refDDict(), will trigger memory + * allocation for the hash table. ZSTD_freeDCtx() also frees this memory. + * Memory is allocated as per ZSTD_DCtx::customMem. + * + * Although this function allocates memory for the table, the user is still responsible for + * memory management of the underlying ZSTD_DDict* themselves. */ #define ZSTD_d_refMultipleDDicts ZSTD_d_experimentalParam4 diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 7e3b4628e..a67113edd 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1770,6 +1770,19 @@ static int basicUnitTests(U32 const seed, double compressibility) size_t dictSize; U32 dictID; size_t dictHeaderSize; + size_t dictBufferFixedSize = 144; + unsigned char const dictBufferFixed[144] = {0x37, 0xa4, 0x30, 0xec, 0x63, 0x00, 0x00, 0x00, 0x08, 0x10, 0x00, 0x1f, + 0x0f, 0x00, 0x28, 0xe5, 0x03, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x80, 0x0f, 0x9e, 0x0f, 0x00, 0x00, 0x24, 0x40, 0x80, 0x00, 0x01, + 0x02, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0xde, 0x08, + 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, + 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, + 0x08, 0x08, 0x08, 0x08, 0xbc, 0xe1, 0x4b, 0x92, 0x0e, 0xb4, 0x7b, 0x18, + 0x86, 0x61, 0x18, 0xc6, 0x18, 0x63, 0x8c, 0x31, 0xc6, 0x18, 0x63, 0x8c, + 0x31, 0x66, 0x66, 0x66, 0x66, 0xb6, 0x6d, 0x01, 0x00, 0x00, 0x00, 0x04, + 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x20, 0x73, 0x6f, 0x64, 0x61, + 0x6c, 0x65, 0x73, 0x20, 0x74, 0x6f, 0x72, 0x74, 0x6f, 0x72, 0x20, 0x65, + 0x6c, 0x65, 0x69, 0x66, 0x65, 0x6e, 0x64, 0x2e, 0x20, 0x41, 0x6c, 0x69}; if (dictBuffer==NULL || samplesSizes==NULL) { free(dictBuffer); @@ -1865,19 +1878,7 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "OK : %u \n", (unsigned)dictHeaderSize); DISPLAYLEVEL(3, "test%3i : check dict header size correctness : ", testNb++); - { unsigned char const dictBufferFixed[144] = { 0x37, 0xa4, 0x30, 0xec, 0x63, 0x00, 0x00, 0x00, 0x08, 0x10, 0x00, 0x1f, - 0x0f, 0x00, 0x28, 0xe5, 0x03, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x80, 0x0f, 0x9e, 0x0f, 0x00, 0x00, 0x24, 0x40, 0x80, 0x00, 0x01, - 0x02, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0x04, 0xde, 0x08, - 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, - 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, 0x08, - 0x08, 0x08, 0x08, 0x08, 0xbc, 0xe1, 0x4b, 0x92, 0x0e, 0xb4, 0x7b, 0x18, - 0x86, 0x61, 0x18, 0xc6, 0x18, 0x63, 0x8c, 0x31, 0xc6, 0x18, 0x63, 0x8c, - 0x31, 0x66, 0x66, 0x66, 0x66, 0xb6, 0x6d, 0x01, 0x00, 0x00, 0x00, 0x04, - 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x20, 0x73, 0x6f, 0x64, 0x61, - 0x6c, 0x65, 0x73, 0x20, 0x74, 0x6f, 0x72, 0x74, 0x6f, 0x72, 0x20, 0x65, - 0x6c, 0x65, 0x69, 0x66, 0x65, 0x6e, 0x64, 0x2e, 0x20, 0x41, 0x6c, 0x69 }; - dictHeaderSize = ZDICT_getDictHeaderSize(dictBufferFixed, 144); + { dictHeaderSize = ZDICT_getDictHeaderSize(dictBufferFixed, dictBufferFixedSize); if (dictHeaderSize != 115) goto _output_error; } DISPLAYLEVEL(3, "OK : %u \n", (unsigned)dictHeaderSize); @@ -2331,6 +2332,67 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : ZSTD_decompressDCtx() with multiple ddicts : ", testNb++); + { + const size_t numDicts = 128; + const size_t numFrames = 4; + size_t i; + ZSTD_DCtx* dctx = ZSTD_createDCtx(); + ZSTD_DDict** ddictTable = (ZSTD_DDict**)malloc(sizeof(ZSTD_DDict*)*numDicts); + ZSTD_CDict** cdictTable = (ZSTD_CDict**)malloc(sizeof(ZSTD_CDict*)*numDicts); + U32 dictIDSeed = seed; + /* Create new compressed buffer that will hold frames with differing dictIDs */ + char* dictBufferMulti = (char*)malloc(sizeof(char) * dictBufferFixedSize); /* Modifiable copy of fixed full dict buffer */ + + ZSTD_memcpy(dictBufferMulti, dictBufferFixed, dictBufferFixedSize); + cSize = 0; + /* Create a bunch of DDicts with random dict IDs */ + for (i = 0; i < numDicts; ++i) { + U32 currDictID = FUZ_rand(&dictIDSeed); + MEM_writeLE32(dictBufferMulti+ZSTD_FRAMEIDSIZE, currDictID); + ddictTable[i] = ZSTD_createDDict(dictBufferMulti, dictBufferFixedSize); + cdictTable[i] = ZSTD_createCDict(dictBufferMulti, dictBufferFixedSize, 3); + if (!ddictTable[i] || !cdictTable[i] || ZSTD_getDictID_fromCDict(cdictTable[i]) != ZSTD_getDictID_fromDDict(ddictTable[i])) { + goto _output_error; + } + } + /* Compress a few frames using random CDicts */ + { + size_t off = 0; + /* only use the first half so we don't push against size limit of compressedBuffer */ + size_t const segSize = (CNBuffSize / 2) / numFrames; + for (i = 0; i < numFrames; i++) { + size_t dictIdx = FUZ_rand(&dictIDSeed) % numDicts; + ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters); + { CHECK_NEWV(r, ZSTD_compress_usingCDict(cctx, + (BYTE*)compressedBuffer + off, CNBuffSize - off, + (BYTE*)CNBuffer + segSize * (size_t)i, segSize, + cdictTable[dictIdx])); + off += r; + } + } + cSize = off; + } + + /* We should succeed to decompression even though different dicts were used on different frames */ + ZSTD_DCtx_reset(dctx, ZSTD_reset_session_and_parameters); + ZSTD_DCtx_setParameter(dctx, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts); + /* Reference every single ddict we made */ + for (i = 0; i < numDicts; ++i) { + CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictTable[i])); + } + CHECK_Z( ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize) ); + ZSTD_freeDCtx(dctx); + for (i = 0; i < numDicts; ++i) { + ZSTD_freeCDict(cdictTable[i]); + ZSTD_freeDDict(ddictTable[i]); + } + free(dictBufferMulti); + free(ddictTable); + free(cdictTable); + } + DISPLAYLEVEL(3, "OK \n"); + ZSTD_freeCCtx(cctx); free(dictBuffer); free(samplesSizes); From c2c9b8a7eca5b3080deb9e4a8dc180f5721ad7ca Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 7 Jan 2021 11:53:35 -0500 Subject: [PATCH 08/11] Address comments, clean up interface/internals --- lib/decompress/zstd_decompress.c | 35 ++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 72abba3b9..14f94f31b 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -62,6 +62,7 @@ #include "../common/fse.h" #define HUF_STATIC_LINKING_ONLY #include "../common/huf.h" +#include "../common/xxhash.h" /* XXH64_reset, XXH64_update, XXH64_digest, XXH64 */ #include "../common/zstd_internal.h" /* blockProperties_t */ #include "zstd_decompress_internal.h" /* ZSTD_DCtx */ #include "zstd_ddict.h" /* ZSTD_DDictDictContent */ @@ -77,7 +78,13 @@ * Multiple DDicts Hashset internals * *************************************/ -#define DDICT_HASHSET_MAX_LOAD_FACTOR 0.75 +#define DDICT_HASHSET_MAX_LOAD_FACTOR_COUNT_MULT 4 +#define DDICT_HASHSET_MAX_LOAD_FACTOR_SIZE_MULT 3 /* These two constants represent SIZE_MULT/COUNT_MULT load factor without using a float. + * Currently, that means a 0.75 load factor. + * So, if count * COUNT_MULT / size * SIZE_MULT != 0, then we've exceeded + * the load factor of the ddict hash set. + */ + #define DDICT_HASHSET_TABLE_BASE_SIZE 64 #define DDICT_HASHSET_RESIZE_FACTOR 2 @@ -90,17 +97,19 @@ static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, U32 d return hash & (hashSet->ddictPtrTableSize - 1); } -/* Adds ddict to a hashset resizing it - * Returns 0 if successful (ddict may or may not have actually been added), or a zstd error code if something went wrong +/* Adds DDict to a hashset without resizing it. + * If inserting a DDict with a dictID that already exists in the set, replaces the one in the set. + * Returns 0 if successful, or a zstd error code if something went wrong. */ -static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, U32 dictID) { +static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict) { + U32 dictID = ZSTD_getDictID_fromDDict(ddict); size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); RETURN_ERROR_IF(hashSet->ddictPtrCount == hashSet->ddictPtrTableSize, GENERIC, "Hash set is full!"); DEBUGLOG(4, "Hashed index: for dictID: %u is %zu", dictID, idx); while (hashSet->ddictPtrTable[idx] != NULL) { /* Replace existing ddict if inserting ddict with same dictID */ if (ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]) == dictID) { - DEBUGLOG(2, "DictID already exists, replacing rather than adding"); + DEBUGLOG(4, "DictID already exists, replacing rather than adding"); hashSet->ddictPtrTable[idx] = ddict; return 0; } @@ -134,7 +143,7 @@ static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMe hashSet->ddictPtrCount = 0; for (i = 0; i < oldTableSize; ++i) { if (oldTable[i] != NULL) { - FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, oldTable[i], ZSTD_getDictID_fromDDict(oldTable[i])), ""); + FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, oldTable[i]), ""); } } ZSTD_customFree(oldTable, customMem); @@ -154,10 +163,7 @@ static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, /* currDictID == 0 implies a NULL ddict entry */ break; } else { - if (idx == hashSet->ddictPtrTableSize - 1) { - idx = 0; - continue; - } + idx &= (hashSet->ddictPtrTableSize - 1); /* Goes to start of table when we reach the end */ idx++; } } @@ -197,12 +203,12 @@ static void ZSTD_freeDDictHashSet(ZSTD_DDictHashSet* hashSet, ZSTD_customMem cus /* Public function: Adds a DDict into the ZSTD_DDictHashSet, possibly triggering a resize of the hash set. * Returns 0 on success, or a ZSTD error. */ -static size_t ZSTD_DDictHashSet_addDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, U32 dictID, ZSTD_customMem customMem) { +static size_t ZSTD_DDictHashSet_addDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, ZSTD_customMem customMem) { DEBUGLOG(4, "Adding dict ID: %d to hashset with - Count: %zu Tablesize: %zu", dictID, hashSet->ddictPtrCount, hashSet->ddictPtrTableSize); - if ((float)hashSet->ddictPtrCount / (float)hashSet->ddictPtrTableSize >= DDICT_HASHSET_MAX_LOAD_FACTOR) { + if (hashSet->ddictPtrCount * DDICT_HASHSET_MAX_LOAD_FACTOR_COUNT_MULT / hashSet->ddictPtrTableSize * DDICT_HASHSET_MAX_LOAD_FACTOR_SIZE_MULT != 0) { FORWARD_IF_ERROR(ZSTD_DDictHashSet_expand(hashSet, customMem), ""); } - FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, ddict, dictID), ""); + FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, ddict), ""); return 0; } @@ -1568,8 +1574,7 @@ size_t ZSTD_DCtx_refDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) } } assert(!dctx->staticSize); /* Impossible: ddictSet cannot have been allocated if static dctx */ - FORWARD_IF_ERROR(ZSTD_DDictHashSet_addDDict(dctx->ddictSet, ddict, - ZSTD_getDictID_fromDDict(ddict), dctx->customMem), ""); + FORWARD_IF_ERROR(ZSTD_DDictHashSet_addDDict(dctx->ddictSet, ddict, dctx->customMem), ""); } } return 0; From 17222654bff8172f95cbb0b364fdc5a873477db6 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 7 Jan 2021 12:07:35 -0500 Subject: [PATCH 09/11] Add streaming decompression to unit test --- tests/fuzzer.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index a67113edd..b808ad2d2 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -2382,6 +2382,14 @@ static int basicUnitTests(U32 const seed, double compressibility) CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictTable[i])); } CHECK_Z( ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize) ); + /* Streaming decompression should also work */ + { + ZSTD_inBuffer in = {compressedBuffer, cSize, 0}; + ZSTD_outBuffer out = {decodedBuffer, CNBuffSize, 0}; + while (in.pos < in.size) { + CHECK_Z(ZSTD_decompressStream(dctx, &out, &in)); + } + } ZSTD_freeDCtx(dctx); for (i = 0; i < numDicts; ++i) { ZSTD_freeCDict(cdictTable[i]); From 4f7584e7a337cf2da7b2768d006fc83f4d5e413e Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 7 Jan 2021 12:09:46 -0500 Subject: [PATCH 10/11] Allow freestanding lib script regex to detect XXH64( --- contrib/freestanding_lib/freestanding.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/freestanding_lib/freestanding.py b/contrib/freestanding_lib/freestanding.py index f8f206944..32d4fcbea 100755 --- a/contrib/freestanding_lib/freestanding.py +++ b/contrib/freestanding_lib/freestanding.py @@ -576,7 +576,7 @@ class Freestanding(object): ) if self._xxh64_prefix is not None: replacements.append( - (re.compile(r"([^\w]|^)(?PXXH64)_"), self._xxh64_prefix) + (re.compile(r"([^\w]|^)(?PXXH64)[\(_]"), self._xxh64_prefix) ) for filepath in self._dst_lib_file_paths(): file = FileLines(filepath) From 9ae0dd93368e3526b6e57e143425d395cdff171c Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 7 Jan 2021 12:13:27 -0500 Subject: [PATCH 11/11] Fix Visual and staticanalyze warnings --- lib/decompress/zstd_decompress.c | 19 +++++++++---------- tests/fuzzer.c | 1 - 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 14f94f31b..3701170cd 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -92,7 +92,7 @@ * Returns an index between [0, hashSet->ddictPtrTableSize] */ static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, U32 dictID) { - U32 hash = XXH64(&dictID, sizeof(U32), 0); + const U64 hash = XXH64(&dictID, sizeof(U32), 0); /* DDict ptr table size is a multiple of 2, use size - 1 as mask to get index within [0, hashSet->ddictPtrTableSize) */ return hash & (hashSet->ddictPtrTableSize - 1); } @@ -102,8 +102,9 @@ static size_t ZSTD_DDictHashSet_getIndex(const ZSTD_DDictHashSet* hashSet, U32 d * Returns 0 if successful, or a zstd error code if something went wrong. */ static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict) { - U32 dictID = ZSTD_getDictID_fromDDict(ddict); + const U32 dictID = ZSTD_getDictID_fromDDict(ddict); size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); + const size_t idxRangeMask = hashSet->ddictPtrTableSize - 1; RETURN_ERROR_IF(hashSet->ddictPtrCount == hashSet->ddictPtrTableSize, GENERIC, "Hash set is full!"); DEBUGLOG(4, "Hashed index: for dictID: %u is %zu", dictID, idx); while (hashSet->ddictPtrTable[idx] != NULL) { @@ -113,10 +114,7 @@ static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const Z hashSet->ddictPtrTable[idx] = ddict; return 0; } - if (idx == hashSet->ddictPtrTableSize - 1) { - idx = 0; - continue; - } + idx &= idxRangeMask; idx++; } DEBUGLOG(4, "Final idx after probing for dictID %u is: %zu", dictID, idx); @@ -146,7 +144,7 @@ static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMe FORWARD_IF_ERROR(ZSTD_DDictHashSet_emplaceDDict(hashSet, oldTable[i]), ""); } } - ZSTD_customFree(oldTable, customMem); + ZSTD_customFree((void*)oldTable, customMem); DEBUGLOG(4, "Finished re-hash"); return 0; } @@ -156,6 +154,7 @@ static size_t ZSTD_DDictHashSet_expand(ZSTD_DDictHashSet* hashSet, ZSTD_customMe */ static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, U32 dictID) { size_t idx = ZSTD_DDictHashSet_getIndex(hashSet, dictID); + const size_t idxRangeMask = hashSet->ddictPtrTableSize - 1; DEBUGLOG(4, "Hashed index: for dictID: %u is %zu", dictID, idx); for (;;) { size_t currDictID = ZSTD_getDictID_fromDDict(hashSet->ddictPtrTable[idx]); @@ -163,7 +162,7 @@ static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, /* currDictID == 0 implies a NULL ddict entry */ break; } else { - idx &= (hashSet->ddictPtrTableSize - 1); /* Goes to start of table when we reach the end */ + idx &= idxRangeMask; /* Goes to start of table when we reach the end */ idx++; } } @@ -193,7 +192,7 @@ static ZSTD_DDictHashSet* ZSTD_createDDictHashSet(ZSTD_customMem customMem) { static void ZSTD_freeDDictHashSet(ZSTD_DDictHashSet* hashSet, ZSTD_customMem customMem) { DEBUGLOG(4, "Freeing ddict hash set"); if (hashSet && hashSet->ddictPtrTable) { - ZSTD_customFree(hashSet->ddictPtrTable, customMem); + ZSTD_customFree((void*)hashSet->ddictPtrTable, customMem); } if (hashSet) { ZSTD_customFree(hashSet, customMem); @@ -204,7 +203,7 @@ static void ZSTD_freeDDictHashSet(ZSTD_DDictHashSet* hashSet, ZSTD_customMem cus * Returns 0 on success, or a ZSTD error. */ static size_t ZSTD_DDictHashSet_addDDict(ZSTD_DDictHashSet* hashSet, const ZSTD_DDict* ddict, ZSTD_customMem customMem) { - DEBUGLOG(4, "Adding dict ID: %d to hashset with - Count: %zu Tablesize: %zu", dictID, hashSet->ddictPtrCount, hashSet->ddictPtrTableSize); + DEBUGLOG(4, "Adding dict ID: %u to hashset with - Count: %zu Tablesize: %zu", ZSTD_getDictID_fromDDict(ddict), hashSet->ddictPtrCount, hashSet->ddictPtrTableSize); if (hashSet->ddictPtrCount * DDICT_HASHSET_MAX_LOAD_FACTOR_COUNT_MULT / hashSet->ddictPtrTableSize * DDICT_HASHSET_MAX_LOAD_FACTOR_SIZE_MULT != 0) { FORWARD_IF_ERROR(ZSTD_DDictHashSet_expand(hashSet, customMem), ""); } diff --git a/tests/fuzzer.c b/tests/fuzzer.c index b808ad2d2..157b798ac 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -2345,7 +2345,6 @@ static int basicUnitTests(U32 const seed, double compressibility) char* dictBufferMulti = (char*)malloc(sizeof(char) * dictBufferFixedSize); /* Modifiable copy of fixed full dict buffer */ ZSTD_memcpy(dictBufferMulti, dictBufferFixed, dictBufferFixedSize); - cSize = 0; /* Create a bunch of DDicts with random dict IDs */ for (i = 0; i < numDicts; ++i) { U32 currDictID = FUZ_rand(&dictIDSeed);