From 13c5ec3e413af3157b9bef5ed3eea42e5fb3ff18 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 2 Sep 2020 17:15:31 -0400 Subject: [PATCH 01/18] Only Allow Dedicated Dict Search for Dicts Loaded in 1 Chunk The load algorithm requires we do it all in one go. --- lib/compress/zstd_compress.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 3e253006a..5f72707f5 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2914,10 +2914,12 @@ static size_t ZSTD_loadDictionaryContent(ZSTD_matchState_t* ms, case ZSTD_greedy: case ZSTD_lazy: case ZSTD_lazy2: - if (chunk >= HASH_READ_SIZE && ms->dedicatedDictSearch) + if (chunk >= HASH_READ_SIZE && ms->dedicatedDictSearch) { + assert(chunk == remaining); /* must load everything in one go */ ZSTD_dedicatedDictSearch_lazy_loadDictionary(ms, ichunk-HASH_READ_SIZE); - else if (chunk >= HASH_READ_SIZE) + } else if (chunk >= HASH_READ_SIZE) { ZSTD_insertAndFindFirstIndex(ms, ichunk-HASH_READ_SIZE); + } break; case ZSTD_btlazy2: /* we want the dictionary table fully sorted */ @@ -3416,6 +3418,9 @@ static size_t ZSTD_initCDict_internal( assert(!ZSTD_checkCParams(cParams)); cdict->matchState.cParams = cParams; cdict->matchState.dedicatedDictSearch = params.enableDedicatedDictSearch; + if (cdict->matchState.dedicatedDictSearch && dictSize > ZSTD_CHUNKSIZE_MAX) { + cdict->matchState.dedicatedDictSearch = 0; + } if ((dictLoadMethod == ZSTD_dlm_byRef) || (!dictBuffer) || (!dictSize)) { cdict->dictContent = dictBuffer; } else { From 66509c7bf42dda5c76e2269f20392fd44cd0fd0f Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 2 Sep 2020 17:29:46 -0400 Subject: [PATCH 02/18] Only Insert Positions Inside the Chain Window --- lib/compress/zstd_lazy.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 5ce805326..c61f8aee4 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -480,8 +480,10 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B { U32 const target = (U32)(ip - ms->window.base); U32* const chainTable = ms->chainTable; - U32 const chainMask = (1 << ms->cParams.chainLog) - 1; + U32 const chainSize = 1 << ms->cParams.chainLog; + U32 const chainMask = chainSize - 1; U32 idx = ms->nextToUpdate; + U32 const minChain = chainSize > target ? 0 : target - chainSize; U32 bucketSize = 1 << ZSTD_LAZY_DDSS_BUCKET_LOG; for ( ; idx < target; idx++) { U32 i; @@ -493,7 +495,9 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B for (i = bucketSize - 1; i; i--) ms->hashTable[h + i] = ms->hashTable[h + i - 1]; /* Insert new position. */ - chainTable[idx & chainMask] = ms->hashTable[h]; + if (idx >= minChain) { + chainTable[idx & chainMask] = ms->hashTable[h]; + } ms->hashTable[h] = idx; } From 9b9feb84f2d8378d5f89d4a9541f034718189c73 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 3 Sep 2020 12:55:40 -0400 Subject: [PATCH 03/18] Lay Out Chain Table Chains Contiguously Rather than interleave all of the chain table entries, tying each entry's position to the corresponding position in the input, this commit changes the layout so that all the entries in a single chain are laid out next to each other. The last entry in the hash table's bucket for this hash is now a packed pointer of position + length of this chain. This cannot be merged as written, since it allocates temporary memory inside ZSTD_dedicatedDictSearch_lazy_loadDictionary(). --- lib/compress/zstd_lazy.c | 93 ++++++++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 27 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index c61f8aee4..f2de622d8 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -11,6 +11,8 @@ #include "zstd_compress_internal.h" #include "zstd_lazy.h" +#include + /*-************************************* * Binary Tree search @@ -479,12 +481,17 @@ U32 ZSTD_insertAndFindFirstIndex(ZSTD_matchState_t* ms, const BYTE* ip) { void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const BYTE* const ip) { U32 const target = (U32)(ip - ms->window.base); + U32* const hashTable = ms->hashTable; U32* const chainTable = ms->chainTable; U32 const chainSize = 1 << ms->cParams.chainLog; U32 const chainMask = chainSize - 1; U32 idx = ms->nextToUpdate; - U32 const minChain = chainSize > target ? 0 : target - chainSize; + U32 const minChain = chainSize < target ? target - chainSize : idx; U32 bucketSize = 1 << ZSTD_LAZY_DDSS_BUCKET_LOG; + U32* const chains = (U32*)malloc(chainSize * sizeof(U32)); + assert(chains != NULL); + assert(idx != 0); + assert(ms->cParams.chainLog <= 24); for ( ; idx < target; idx++) { U32 i; size_t const h = ZSTD_hashPtr( @@ -493,15 +500,34 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B ms->cParams.minMatch) << ZSTD_LAZY_DDSS_BUCKET_LOG; /* Shift hash cache down 1. */ for (i = bucketSize - 1; i; i--) - ms->hashTable[h + i] = ms->hashTable[h + i - 1]; + hashTable[h + i] = hashTable[h + i - 1]; /* Insert new position. */ if (idx >= minChain) { - chainTable[idx & chainMask] = ms->hashTable[h]; + chains[idx & chainMask] = hashTable[h]; + } + hashTable[h] = idx; + } + + { + U32 chainPos = 0; + size_t hashIdx; + for (hashIdx = 0; hashIdx < (1U << ms->cParams.hashLog); hashIdx += (1 << ZSTD_LAZY_DDSS_BUCKET_LOG)) { + U32 count = 0; + U32 i = hashTable[hashIdx + bucketSize - 1]; + while (i) { + chainTable[chainPos++] = i; + count++; + if (i < minChain || count >= 255) { + break; + } + i = chains[i & chainMask]; + } + hashTable[hashIdx + bucketSize - 1] = ((chainPos - count) << 8) + count; } - ms->hashTable[h] = idx; } ms->nextToUpdate = target; + free(chains); } @@ -574,22 +600,26 @@ size_t ZSTD_HcFindBestMatch_generic ( } if (dictMode == ZSTD_dedicatedDictSearch) { - const U32 ddsChainSize = (1 << dms->cParams.chainLog); - const U32 ddsChainMask = ddsChainSize - 1; const U32 ddsLowestIndex = dms->window.dictLimit; const BYTE* const ddsBase = dms->window.base; const BYTE* const ddsEnd = dms->window.nextSrc; const U32 ddsSize = (U32)(ddsEnd - ddsBase); const U32 ddsIndexDelta = dictLimit - ddsSize; - const U32 ddsMinChain = ddsSize > ddsChainSize ? ddsSize - ddsChainSize : 0; const U32 bucketSize = (1 << ZSTD_LAZY_DDSS_BUCKET_LOG); - const U32 bucketLimit = nbAttempts < bucketSize ? nbAttempts : bucketSize; + const U32 bucketLimit = nbAttempts < bucketSize - 1 ? nbAttempts : bucketSize - 1; U32 ddsAttempt; - for (ddsAttempt = 0; ddsAttempt < bucketSize; ddsAttempt++) { + for (ddsAttempt = 0; ddsAttempt < bucketSize - 1; ddsAttempt++) { PREFETCH_L1(ddsBase + dms->hashTable[ddsIdx + ddsAttempt]); } + { + U32 const chainPackedPointer = dms->hashTable[ddsIdx + bucketSize - 1]; + U32 const chainIndex = chainPackedPointer >> 8; + + PREFETCH_L1(&dms->chainTable[chainIndex]); + } + for (ddsAttempt = 0; ddsAttempt < bucketLimit; ddsAttempt++) { size_t currentMl=0; const BYTE* match; @@ -617,27 +647,36 @@ size_t ZSTD_HcFindBestMatch_generic ( } } - for ( ; (ddsAttempt < nbAttempts) & (matchIndex >= ddsMinChain); ddsAttempt++) { - size_t currentMl=0; - const BYTE* match; - matchIndex = dms->chainTable[matchIndex & ddsChainMask]; - match = ddsBase + matchIndex; + { + U32 const chainPackedPointer = dms->hashTable[ddsIdx + bucketSize - 1]; + U32 chainIndex = chainPackedPointer >> 8; + U32 const chainLength = chainPackedPointer & 0xFF; + U32 const chainAttempts = nbAttempts - ddsAttempt; + U32 const chainLimit = chainAttempts > chainLength ? chainLength : chainAttempts; + U32 chainAttempt = 0; - if (matchIndex < ddsLowestIndex) { - break; - } + for ( ; chainAttempt < chainLimit; chainAttempt++, chainIndex++) { + size_t currentMl=0; + const BYTE* match; + matchIndex = dms->chainTable[chainIndex]; + match = ddsBase + matchIndex; - assert(match+4 <= ddsEnd); - if (MEM_read32(match) == MEM_read32(ip)) { - /* assumption : matchIndex <= dictLimit-4 (by table construction) */ - currentMl = ZSTD_count_2segments(ip+4, match+4, iLimit, ddsEnd, prefixStart) + 4; - } + if (matchIndex < ddsLowestIndex) { + break; + } - /* save best solution */ - if (currentMl > ml) { - ml = currentMl; - *offsetPtr = curr - (matchIndex + ddsIndexDelta) + ZSTD_REP_MOVE; - if (ip+currentMl == iLimit) break; /* best possible, avoids read overflow on next attempt */ + assert(match+4 <= ddsEnd); + if (MEM_read32(match) == MEM_read32(ip)) { + /* assumption : matchIndex <= dictLimit-4 (by table construction) */ + currentMl = ZSTD_count_2segments(ip+4, match+4, iLimit, ddsEnd, prefixStart) + 4; + } + + /* save best solution */ + if (currentMl > ml) { + ml = currentMl; + *offsetPtr = curr - (matchIndex + ddsIndexDelta) + ZSTD_REP_MOVE; + if (ip+currentMl == iLimit) break; /* best possible, avoids read overflow on next attempt */ + } } } } else if (dictMode == ZSTD_dictMatchState) { From 20a020edbc7aac677552f273d1e9961c2a56d911 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 3 Sep 2020 13:34:38 -0400 Subject: [PATCH 04/18] Prefetch Chain Table Matches --- lib/compress/zstd_lazy.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index f2de622d8..0b70494fa 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -653,9 +653,13 @@ size_t ZSTD_HcFindBestMatch_generic ( U32 const chainLength = chainPackedPointer & 0xFF; U32 const chainAttempts = nbAttempts - ddsAttempt; U32 const chainLimit = chainAttempts > chainLength ? chainLength : chainAttempts; - U32 chainAttempt = 0; + U32 chainAttempt; - for ( ; chainAttempt < chainLimit; chainAttempt++, chainIndex++) { + for (chainAttempt = 0 ; chainAttempt < chainLimit; chainAttempt++) { + PREFETCH_L1(ddsBase + dms->chainTable[chainIndex + chainAttempt]); + } + + for (chainAttempt = 0 ; chainAttempt < chainLimit; chainAttempt++, chainIndex++) { size_t currentMl=0; const BYTE* match; matchIndex = dms->chainTable[chainIndex]; From f42c5bddd9b3b5e2741e93c866b988f97c9f296a Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 3 Sep 2020 13:58:11 -0400 Subject: [PATCH 05/18] Truncate Chain at Last Possible Attempt Make the chain table denser? --- lib/compress/zstd_lazy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 0b70494fa..edba73d05 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -487,7 +487,9 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B U32 const chainMask = chainSize - 1; U32 idx = ms->nextToUpdate; U32 const minChain = chainSize < target ? target - chainSize : idx; - U32 bucketSize = 1 << ZSTD_LAZY_DDSS_BUCKET_LOG; + U32 const bucketSize = 1 << ZSTD_LAZY_DDSS_BUCKET_LOG; + U32 const nbAttempts = (1 << ms->cParams.searchLog) - bucketSize + 1; + U32 const chainLimit = nbAttempts > 255 ? 255 : nbAttempts; U32* const chains = (U32*)malloc(chainSize * sizeof(U32)); assert(chains != NULL); assert(idx != 0); @@ -517,7 +519,7 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B while (i) { chainTable[chainPos++] = i; count++; - if (i < minChain || count >= 255) { + if (i < minChain || count >= chainLimit) { break; } i = chains[i & chainMask]; From 916238d9dca6fd3d1bef5f2b95299dae63faa2f1 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 3 Sep 2020 17:29:44 -0400 Subject: [PATCH 06/18] Avoid Malloc in Table Fill; Pack Tmp Structure into Hash Table --- lib/compress/zstd_lazy.c | 79 ++++++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index edba73d05..9fc787943 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -11,8 +11,6 @@ #include "zstd_compress_internal.h" #include "zstd_lazy.h" -#include - /*-************************************* * Binary Tree search @@ -488,48 +486,75 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B U32 idx = ms->nextToUpdate; U32 const minChain = chainSize < target ? target - chainSize : idx; U32 const bucketSize = 1 << ZSTD_LAZY_DDSS_BUCKET_LOG; - U32 const nbAttempts = (1 << ms->cParams.searchLog) - bucketSize + 1; + U32 const nbAttempts = 1 << ms->cParams.searchLog; U32 const chainLimit = nbAttempts > 255 ? 255 : nbAttempts; - U32* const chains = (U32*)malloc(chainSize * sizeof(U32)); - assert(chains != NULL); - assert(idx != 0); + + /* We know the hashtable is oversized by a factor of `bucketSize`. + * We are going to temporarily pretend `bucketSize == 1`, keeping only a + * single entry. We will use + * the rest of the space to construct a temporary chaintable. + */ + U32 const hashLog = ms->cParams.hashLog - ZSTD_LAZY_DDSS_BUCKET_LOG; + U32* const tmpHashTable = hashTable; + U32* const tmpChainTable = hashTable + (1 << hashLog); + + U32 hashIdx; + assert(ms->cParams.chainLog <= 24); + assert(ms->cParams.hashLog >= ms->cParams.chainLog + 2); + assert(idx != 0); + + /* fill tmp hash and tmp chain */ for ( ; idx < target; idx++) { - U32 i; - size_t const h = ZSTD_hashPtr( - ms->window.base + idx, - ms->cParams.hashLog - ZSTD_LAZY_DDSS_BUCKET_LOG, - ms->cParams.minMatch) << ZSTD_LAZY_DDSS_BUCKET_LOG; - /* Shift hash cache down 1. */ - for (i = bucketSize - 1; i; i--) - hashTable[h + i] = hashTable[h + i - 1]; - /* Insert new position. */ + U32 const h = ZSTD_hashPtr( + ms->window.base + idx, hashLog, ms->cParams.minMatch); if (idx >= minChain) { - chains[idx & chainMask] = hashTable[h]; + tmpChainTable[idx & chainMask] = hashTable[h]; } - hashTable[h] = idx; + tmpHashTable[h] = idx; } + /* sort chains into ddss chain table */ { U32 chainPos = 0; - size_t hashIdx; - for (hashIdx = 0; hashIdx < (1U << ms->cParams.hashLog); hashIdx += (1 << ZSTD_LAZY_DDSS_BUCKET_LOG)) { + for (hashIdx = 0; hashIdx < (1U << hashLog); hashIdx++) { U32 count = 0; - U32 i = hashTable[hashIdx + bucketSize - 1]; - while (i) { + U32 i = tmpHashTable[hashIdx]; + while (i >= minChain && count < chainLimit) { chainTable[chainPos++] = i; count++; - if (i < minChain || count >= chainLimit) { - break; - } - i = chains[i & chainMask]; + i = tmpChainTable[i & chainMask]; } - hashTable[hashIdx + bucketSize - 1] = ((chainPos - count) << 8) + count; + tmpHashTable[hashIdx] = ((chainPos - count) << 8) + count; } } + /* inflate hash table */ + for (hashIdx = (1 << hashLog); hashIdx; ) { + U32 const bucketIdx = --hashIdx << ZSTD_LAZY_DDSS_BUCKET_LOG; + U32 const chainPackedPointer = tmpHashTable[hashIdx]; + U32 const chainIdx = chainPackedPointer >> 8; + U32 const chainLength = chainPackedPointer & 0xFF; + U32 const cacheLength = chainLength < bucketSize - 1 ? chainLength : bucketSize - 1; + U32 i; + for (i = 0; i < cacheLength; i++) { + hashTable[bucketIdx + i] = chainTable[chainIdx + i]; + } + for (; i < bucketSize - 1; i++) { + hashTable[bucketIdx + i] = 0; + } + if (chainLength < bucketSize) { + hashTable[bucketIdx + bucketSize - 1] = 0; + } else { + U32 const newChainPointer = ((chainIdx + bucketSize - 1) << 8) + (chainLength - bucketSize + 1); + hashTable[bucketIdx + bucketSize - 1] = newChainPointer; + } + } + + /* densify chain table */ + /* TODO */ + ms->nextToUpdate = target; - free(chains); } From b2b0641ea03c8fe4eec3a80d04d8c7423534707f Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 3 Sep 2020 19:45:24 -0400 Subject: [PATCH 07/18] Rewrite Table Fill to Retain Cache Entries Beyond Chain Window --- lib/compress/zstd_lazy.c | 71 ++++++++++++++++++++++++---------------- 1 file changed, 43 insertions(+), 28 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 9fc787943..454c57176 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -478,7 +478,8 @@ U32 ZSTD_insertAndFindFirstIndex(ZSTD_matchState_t* ms, const BYTE* ip) { void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const BYTE* const ip) { - U32 const target = (U32)(ip - ms->window.base); + const BYTE* const base = ms->window.base; + U32 const target = (U32)(ip - base); U32* const hashTable = ms->hashTable; U32* const chainTable = ms->chainTable; U32 const chainSize = 1 << ms->cParams.chainLog; @@ -486,8 +487,9 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B U32 idx = ms->nextToUpdate; U32 const minChain = chainSize < target ? target - chainSize : idx; U32 const bucketSize = 1 << ZSTD_LAZY_DDSS_BUCKET_LOG; - U32 const nbAttempts = 1 << ms->cParams.searchLog; - U32 const chainLimit = nbAttempts > 255 ? 255 : nbAttempts; + U32 const cacheSize = bucketSize - 1; + U32 const chainAttempts = (1 << ms->cParams.searchLog) - cacheSize; + U32 const chainLimit = chainAttempts > 255 ? 255 : chainAttempts; /* We know the hashtable is oversized by a factor of `bucketSize`. * We are going to temporarily pretend `bucketSize == 1`, keeping only a @@ -501,13 +503,12 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B U32 hashIdx; assert(ms->cParams.chainLog <= 24); - assert(ms->cParams.hashLog >= ms->cParams.chainLog + 2); + assert(ms->cParams.hashLog >= ms->cParams.chainLog); assert(idx != 0); - /* fill tmp hash and tmp chain */ + /* fill conventional hash table and conventional chain table */ for ( ; idx < target; idx++) { - U32 const h = ZSTD_hashPtr( - ms->window.base + idx, hashLog, ms->cParams.minMatch); + U32 const h = ZSTD_hashPtr(base + idx, hashLog, ms->cParams.minMatch); if (idx >= minChain) { tmpChainTable[idx & chainMask] = hashTable[h]; } @@ -518,41 +519,55 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B { U32 chainPos = 0; for (hashIdx = 0; hashIdx < (1U << hashLog); hashIdx++) { - U32 count = 0; + U32 count; U32 i = tmpHashTable[hashIdx]; - while (i >= minChain && count < chainLimit) { - chainTable[chainPos++] = i; - count++; + for (count = 0; i >= minChain && count < cacheSize; count++) { + /* skip through the chain to the first position that won't be + * in the hash bucket */ i = tmpChainTable[i & chainMask]; } - tmpHashTable[hashIdx] = ((chainPos - count) << 8) + count; + if (count == cacheSize) { + for (count = 0; count < chainLimit;) { + chainTable[chainPos++] = i; + count++; + if (i < minChain) { + break; + } + i = tmpChainTable[i & chainMask]; + } + } else { + count = 0; + } + if (count) { + tmpHashTable[hashIdx] = ((chainPos - count) << 8) + count; + } else { + tmpHashTable[hashIdx] = 0; + } } + assert(chainPos <= chainSize); /* I believe this is guaranteed... */ } - /* inflate hash table */ + /* move chain pointers into the last entry of each hash bucket */ for (hashIdx = (1 << hashLog); hashIdx; ) { U32 const bucketIdx = --hashIdx << ZSTD_LAZY_DDSS_BUCKET_LOG; U32 const chainPackedPointer = tmpHashTable[hashIdx]; - U32 const chainIdx = chainPackedPointer >> 8; - U32 const chainLength = chainPackedPointer & 0xFF; - U32 const cacheLength = chainLength < bucketSize - 1 ? chainLength : bucketSize - 1; U32 i; - for (i = 0; i < cacheLength; i++) { - hashTable[bucketIdx + i] = chainTable[chainIdx + i]; - } - for (; i < bucketSize - 1; i++) { + for (i = 0; i < cacheSize; i++) { hashTable[bucketIdx + i] = 0; } - if (chainLength < bucketSize) { - hashTable[bucketIdx + bucketSize - 1] = 0; - } else { - U32 const newChainPointer = ((chainIdx + bucketSize - 1) << 8) + (chainLength - bucketSize + 1); - hashTable[bucketIdx + bucketSize - 1] = newChainPointer; - } + hashTable[bucketIdx + bucketSize - 1] = chainPackedPointer; } - /* densify chain table */ - /* TODO */ + /* fill the buckets of the hash table */ + for (idx = ms->nextToUpdate; idx < target; idx++) { + U32 const h = ZSTD_hashPtr(base + idx, hashLog, ms->cParams.minMatch) + << ZSTD_LAZY_DDSS_BUCKET_LOG; + U32 i; + /* Shift hash cache down 1. */ + for (i = cacheSize - 1; i; i--) + hashTable[h + i] = hashTable[h + i - 1]; + hashTable[h] = idx; + } ms->nextToUpdate = target; } From 06d240b8a769ce5261247f52c9194cba0a4b3d9f Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Fri, 4 Sep 2020 00:11:44 -0400 Subject: [PATCH 08/18] Use All Available Space in the Hash Table to Extent Chain Table Reach Rather than restrict our temp chain table to 2 ** chainLog entries, this commit uses all available space to reach further back to gather longer chains to pack into the DDSS chain table. --- lib/compress/zstd_lazy.c | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 454c57176..efe58b3cd 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -483,7 +483,6 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B U32* const hashTable = ms->hashTable; U32* const chainTable = ms->chainTable; U32 const chainSize = 1 << ms->cParams.chainLog; - U32 const chainMask = chainSize - 1; U32 idx = ms->nextToUpdate; U32 const minChain = chainSize < target ? target - chainSize : idx; U32 const bucketSize = 1 << ZSTD_LAZY_DDSS_BUCKET_LOG; @@ -493,24 +492,27 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B /* We know the hashtable is oversized by a factor of `bucketSize`. * We are going to temporarily pretend `bucketSize == 1`, keeping only a - * single entry. We will use - * the rest of the space to construct a temporary chaintable. + * single entry. We will use the rest of the space to construct a temporary + * chaintable. */ U32 const hashLog = ms->cParams.hashLog - ZSTD_LAZY_DDSS_BUCKET_LOG; U32* const tmpHashTable = hashTable; U32* const tmpChainTable = hashTable + (1 << hashLog); + U32 const tmpChainSize = ((1 << ZSTD_LAZY_DDSS_BUCKET_LOG) - 1) << hashLog; + U32 const tmpMinChain = tmpChainSize < target ? target - tmpChainSize : idx; U32 hashIdx; assert(ms->cParams.chainLog <= 24); assert(ms->cParams.hashLog >= ms->cParams.chainLog); assert(idx != 0); + assert(tmpMinChain <= minChain); /* fill conventional hash table and conventional chain table */ for ( ; idx < target; idx++) { U32 const h = ZSTD_hashPtr(base + idx, hashLog, ms->cParams.minMatch); - if (idx >= minChain) { - tmpChainTable[idx & chainMask] = hashTable[h]; + if (idx >= tmpMinChain) { + tmpChainTable[idx - tmpMinChain] = hashTable[h]; } tmpHashTable[h] = idx; } @@ -520,20 +522,38 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B U32 chainPos = 0; for (hashIdx = 0; hashIdx < (1U << hashLog); hashIdx++) { U32 count; + U32 countBeyondMinChain = 0; U32 i = tmpHashTable[hashIdx]; - for (count = 0; i >= minChain && count < cacheSize; count++) { + for (count = 0; i >= tmpMinChain && count < cacheSize; count++) { /* skip through the chain to the first position that won't be - * in the hash bucket */ - i = tmpChainTable[i & chainMask]; + * in the hash cache bucket */ + if (i < minChain) { + countBeyondMinChain++; + } + i = tmpChainTable[i - tmpMinChain]; } if (count == cacheSize) { for (count = 0; count < chainLimit;) { + if (i < minChain) { + countBeyondMinChain++; + if (countBeyondMinChain > cacheSize) { + /* only allow pulling `cacheSize` number of entries + * into the cache or chainTable beyond `minChain`, + * to replace the entries pulled out of the + * chainTable into the cache. This lets us reach + * back further without increasing the total number + * of entries in the chainTable, guaranteeing the + * DDSS chain table will fit into the space + * allocated for the regular one. */ + break; + } + } chainTable[chainPos++] = i; count++; - if (i < minChain) { + if (i < tmpMinChain) { break; } - i = tmpChainTable[i & chainMask]; + i = tmpChainTable[i - tmpMinChain]; } } else { count = 0; From ed4383277040ee2318dae7b1b83dd879404c8fc2 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Fri, 4 Sep 2020 00:31:00 -0400 Subject: [PATCH 09/18] Simplify Match Limit Checks Seems like a ~1.25% speedup. --- lib/compress/zstd_lazy.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index efe58b3cd..898f1c21f 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -535,8 +535,7 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B if (count == cacheSize) { for (count = 0; count < chainLimit;) { if (i < minChain) { - countBeyondMinChain++; - if (countBeyondMinChain > cacheSize) { + if (!i || countBeyondMinChain++ > cacheSize) { /* only allow pulling `cacheSize` number of entries * into the cache or chainTable beyond `minChain`, * to replace the entries pulled out of the @@ -688,10 +687,13 @@ size_t ZSTD_HcFindBestMatch_generic ( matchIndex = dms->hashTable[ddsIdx + ddsAttempt]; match = ddsBase + matchIndex; - if (matchIndex < ddsLowestIndex) { + if (!matchIndex) { return ml; } + /* guaranteed by table construction */ + (void)ddsLowestIndex; + assert(matchIndex >= ddsLowestIndex); assert(match+4 <= ddsEnd); if (MEM_read32(match) == MEM_read32(ip)) { /* assumption : matchIndex <= dictLimit-4 (by table construction) */ @@ -727,10 +729,8 @@ size_t ZSTD_HcFindBestMatch_generic ( matchIndex = dms->chainTable[chainIndex]; match = ddsBase + matchIndex; - if (matchIndex < ddsLowestIndex) { - break; - } - + /* guaranteed by table construction */ + assert(matchIndex >= ddsLowestIndex); assert(match+4 <= ddsEnd); if (MEM_read32(match) == MEM_read32(ip)) { /* assumption : matchIndex <= dictLimit-4 (by table construction) */ From efa33861f27b0339cf4a5238fa1e22fce08717ce Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Tue, 8 Sep 2020 17:39:37 -0400 Subject: [PATCH 10/18] Attempt to Fix MSVC Warnings --- lib/compress/zstd_lazy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 898f1c21f..9f11d4ca8 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -497,7 +497,7 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B */ U32 const hashLog = ms->cParams.hashLog - ZSTD_LAZY_DDSS_BUCKET_LOG; U32* const tmpHashTable = hashTable; - U32* const tmpChainTable = hashTable + (1 << hashLog); + U32* const tmpChainTable = hashTable + ((size_t)1 << hashLog); U32 const tmpChainSize = ((1 << ZSTD_LAZY_DDSS_BUCKET_LOG) - 1) << hashLog; U32 const tmpMinChain = tmpChainSize < target ? target - tmpChainSize : idx; @@ -510,7 +510,7 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B /* fill conventional hash table and conventional chain table */ for ( ; idx < target; idx++) { - U32 const h = ZSTD_hashPtr(base + idx, hashLog, ms->cParams.minMatch); + U32 const h = (U32)ZSTD_hashPtr(base + idx, hashLog, ms->cParams.minMatch); if (idx >= tmpMinChain) { tmpChainTable[idx - tmpMinChain] = hashTable[h]; } @@ -579,7 +579,7 @@ void ZSTD_dedicatedDictSearch_lazy_loadDictionary(ZSTD_matchState_t* ms, const B /* fill the buckets of the hash table */ for (idx = ms->nextToUpdate; idx < target; idx++) { - U32 const h = ZSTD_hashPtr(base + idx, hashLog, ms->cParams.minMatch) + U32 const h = (U32)ZSTD_hashPtr(base + idx, hashLog, ms->cParams.minMatch) << ZSTD_LAZY_DDSS_BUCKET_LOG; U32 i; /* Shift hash cache down 1. */ From 2cc2b40a1baff2f323a3576f42134b06965d2420 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Sep 2020 11:32:16 -0400 Subject: [PATCH 11/18] Test DDSS A Little More Thoroughly --- tests/fuzzer.c | 51 +++++++++++++++++++++++++++++--------------------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index c354c4212..2c37bb0f3 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -571,7 +571,7 @@ static int basicUnitTests(U32 const seed, double compressibility) r = ZSTD_decompress(decodedBuffer, CNBuffSize, compressedBuffer, cSize); if (!ZSTD_isError(r)) goto _output_error; if (ZSTD_getErrorCode(r) != ZSTD_error_checksum_wrong) goto _output_error; - + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_forceIgnoreChecksum, ZSTD_d_ignoreChecksum)); r = ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize-1); if (!ZSTD_isError(r)) goto _output_error; /* wrong checksum size should still throw error */ @@ -2926,7 +2926,7 @@ static int basicUnitTests(U32 const seed, double compressibility) { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); ZSTD_DCtx* const dctx = ZSTD_createDCtx(); - size_t dictSize = CNBuffSize > 110 KB ? 110 KB : CNBuffSize; + size_t dictSize = CNBuffSize; void* dict = (void*)malloc(dictSize); ZSTD_CCtx_params* cctx_params = ZSTD_createCCtxParams(); ZSTD_dictAttachPref_e const attachPrefs[] = { @@ -2934,10 +2934,13 @@ static int basicUnitTests(U32 const seed, double compressibility) ZSTD_dictForceAttach, ZSTD_dictForceCopy, ZSTD_dictForceLoad, - ZSTD_dictForceAttach + ZSTD_dictDefaultAttach, + ZSTD_dictForceAttach, + ZSTD_dictForceCopy, + ZSTD_dictForceLoad }; - int const enableDedicatedDictSearch[] = {0, 0, 0, 0, 1}; - int const cLevel = 6; + int const enableDedicatedDictSearch[] = {0, 0, 0, 0, 1, 1, 1, 1}; + int cLevel; int i; RDG_genBuffer(dict, dictSize, 0.5, 0.5, seed); @@ -2945,28 +2948,34 @@ static int basicUnitTests(U32 const seed, double compressibility) CHECK(cctx_params != NULL); - for (i = 0; i < 5; ++i) { - ZSTD_dictAttachPref_e const attachPref = attachPrefs[i]; - int const enableDDS = enableDedicatedDictSearch[i]; - ZSTD_CDict* cdict; + for (dictSize = CNBuffSize; dictSize; dictSize = dictSize >> 1) { + for (cLevel = 4; cLevel < 12; cLevel++) { + for (i = 0; i < 8; ++i) { + ZSTD_dictAttachPref_e const attachPref = attachPrefs[i]; + int const enableDDS = enableDedicatedDictSearch[i]; + ZSTD_CDict* cdict; - DISPLAYLEVEL(5, "\n iter %d ", i); + DISPLAYLEVEL(5, "\n dictSize %lu cLevel %d iter %d ", dictSize, cLevel, i); - ZSTD_CCtxParams_init(cctx_params, cLevel); - CHECK_Z(ZSTD_CCtxParams_setParameter(cctx_params, ZSTD_c_enableDedicatedDictSearch, enableDDS)); + ZSTD_CCtxParams_init(cctx_params, cLevel); + CHECK_Z(ZSTD_CCtxParams_setParameter(cctx_params, ZSTD_c_enableDedicatedDictSearch, enableDDS)); - cdict = ZSTD_createCDict_advanced2(dict, dictSize, ZSTD_dlm_byRef, ZSTD_dct_auto, cctx_params, ZSTD_defaultCMem); - CHECK(cdict != NULL); + cdict = ZSTD_createCDict_advanced2(dict, dictSize, ZSTD_dlm_byRef, ZSTD_dct_auto, cctx_params, ZSTD_defaultCMem); + CHECK(cdict != NULL); - CHECK_Z(ZSTD_CCtx_refCDict(cctx, cdict)); - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_forceAttachDict, attachPref)); + CHECK_Z(ZSTD_CCtx_refCDict(cctx, cdict)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_forceAttachDict, attachPref)); - cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize); - CHECK_Z(cSize); - CHECK_Z(ZSTD_decompress_usingDict(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize, dict, dictSize)); + cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize); + CHECK_Z(cSize); + CHECK_Z(ZSTD_decompress_usingDict(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize, dict, dictSize)); - CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters)); - ZSTD_freeCDict(cdict); + DISPLAYLEVEL(5, "compressed to %lu bytes ", cSize); + + CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters)); + ZSTD_freeCDict(cdict); + } + } } ZSTD_freeCCtx(cctx); From 0faefbf1b305524a8ccd5b4a0804429e62779ecb Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Sep 2020 11:33:12 -0400 Subject: [PATCH 12/18] Make DDSS Selection Override ForceCopy Directive --- lib/compress/zstd_compress.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 5f72707f5..eedd9b67f 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1647,13 +1647,13 @@ static int ZSTD_shouldAttachDict(const ZSTD_CDict* cdict, { size_t cutoff = attachDictSizeCutoffs[cdict->matchState.cParams.strategy]; int const dedicatedDictSearch = cdict->matchState.dedicatedDictSearch; - return ( dedicatedDictSearch - || pledgedSrcSize <= cutoff - || pledgedSrcSize == ZSTD_CONTENTSIZE_UNKNOWN - || params->attachDictPref == ZSTD_dictForceAttach ) - && params->attachDictPref != ZSTD_dictForceCopy - && !params->forceWindow; /* dictMatchState isn't correctly - * handled in _enforceMaxDist */ + return dedicatedDictSearch + || ( ( pledgedSrcSize <= cutoff + || pledgedSrcSize == ZSTD_CONTENTSIZE_UNKNOWN + || params->attachDictPref == ZSTD_dictForceAttach ) + && params->attachDictPref != ZSTD_dictForceCopy + && !params->forceWindow ); /* dictMatchState isn't correctly + * handled in _enforceMaxDist */ } static size_t From 032010fcc1c148d4799a2be75790e1c8fb044dab Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Sep 2020 16:36:28 -0400 Subject: [PATCH 13/18] Improve Documentation Slightly --- lib/zstd.h | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/lib/zstd.h b/lib/zstd.h index e42a4bd49..f8d5e84da 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1548,15 +1548,16 @@ ZSTDLIB_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const void* pre #define ZSTD_c_srcSizeHint ZSTD_c_experimentalParam7 /* Controls whether the new and experimental "dedicated dictionary search - * structure" can be used. + * structure" can be used. This feature is still rough around the edges, be + * prepared for surprising behavior! * * How to use it: * * When using a CDict, whether to use this feature or not is controlled at * CDict creation, and it must be set in a CCtxParams set passed into that - * construction. A compression will then use the feature or not based on how - * the CDict was constructed; the value of this param, set in the CCtx, will - * have no effect. + * construction (via ZSTD_createCDict_advanced2()). A compression will then + * use the feature or not based on how the CDict was constructed; the value of + * this param, set in the CCtx, will have no effect. * * However, when a dictionary buffer is passed into a CCtx, such as via * ZSTD_CCtx_loadDictionary(), this param can be set on the CCtx to control @@ -1578,10 +1579,13 @@ ZSTDLIB_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const void* pre * written as the compression goes along. This means we can choose a search * structure for the dictionary that is read-optimized. * - * This feature enables the use of that different structure. Note that this - * means that the CDict tables can no longer be copied into the CCtx, so - * the dict attachment mode ZSTD_dictForceCopy will no longer be useable. The - * dictionary can only be attached or reloaded. + * This feature enables the use of that different structure. + * + * Note that some of the members of the ZSTD_compressionParameters struct have + * different semantics and constraints in the dedicated search structure. It is + * highly recommended that you simply set a compression level in the CCtxParams + * you pass into the CDict creation call, and avoid messing with the cParams + * directly. * * Effects: * @@ -1589,9 +1593,13 @@ ZSTDLIB_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const void* pre * implementation supports this feature. Currently, that's limited to * ZSTD_greedy, ZSTD_lazy, and ZSTD_lazy2. * - * In general, you should expect compression to be faster, and CDict creation - * to be slightly slower. Eventually, we will probably make this mode the - * default. + * Note that this means that the CDict tables can no longer be copied into the + * CCtx, so the dict attachment mode ZSTD_dictForceCopy will no longer be + * useable. The dictionary can only be attached or reloaded. + * + * In general, you should expect compression to be faster--sometimes very much + * so--and CDict creation to be slightly slower. Eventually, we will probably + * make this mode the default. */ #define ZSTD_c_enableDedicatedDictSearch ZSTD_c_experimentalParam8 From 85a95840e429dce4e662cd11d63e4823af1df2e3 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Sep 2020 18:18:50 -0400 Subject: [PATCH 14/18] Further Consolidate Dict Mode Checks --- lib/compress/zstd_lazy.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 9f11d4ca8..f84215223 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -896,16 +896,13 @@ ZSTD_compressBlock_lazy_generic( const int isDMS = dictMode == ZSTD_dictMatchState; const int isDDS = dictMode == ZSTD_dedicatedDictSearch; + const int isDxS = isDMS || isDDS; const ZSTD_matchState_t* const dms = ms->dictMatchState; - const U32 dictLowestIndex = isDMS || isDDS ? - dms->window.dictLimit : 0; - const BYTE* const dictBase = isDMS || isDDS ? - dms->window.base : NULL; - const BYTE* const dictLowest = isDMS || isDDS ? - dictBase + dictLowestIndex : NULL; - const BYTE* const dictEnd = isDMS || isDDS ? - dms->window.nextSrc : NULL; - const U32 dictIndexDelta = isDMS || isDDS ? + const U32 dictLowestIndex = isDxS ? dms->window.dictLimit : 0; + const BYTE* const dictBase = isDxS ? dms->window.base : NULL; + const BYTE* const dictLowest = isDxS ? dictBase + dictLowestIndex : NULL; + const BYTE* const dictEnd = isDxS ? dms->window.nextSrc : NULL; + const U32 dictIndexDelta = isDxS ? prefixLowestIndex - (U32)(dictEnd - dictBase) : 0; const U32 dictAndPrefixLength = (U32)((ip - prefixLowest) + (dictEnd - dictLowest)); @@ -923,7 +920,7 @@ ZSTD_compressBlock_lazy_generic( if (offset_2 > maxRep) savedOffset = offset_2, offset_2 = 0; if (offset_1 > maxRep) savedOffset = offset_1, offset_1 = 0; } - if (isDMS || isDDS) { + if (isDxS) { /* dictMatchState repCode checks don't currently handle repCode == 0 * disabling. */ assert(offset_1 <= dictAndPrefixLength); @@ -943,7 +940,7 @@ ZSTD_compressBlock_lazy_generic( const BYTE* start=ip+1; /* check repCode */ - if (isDMS || isDDS) { + if (isDxS) { const U32 repIndex = (U32)(ip - base) + 1 - offset_1; const BYTE* repMatch = ((dictMode == ZSTD_dictMatchState || dictMode == ZSTD_dedicatedDictSearch) && repIndex < prefixLowestIndex) ? @@ -986,7 +983,7 @@ ZSTD_compressBlock_lazy_generic( if ((mlRep >= 4) && (gain2 > gain1)) matchLength = mlRep, offset = 0, start = ip; } - if (isDMS || isDDS) { + if (isDxS) { const U32 repIndex = (U32)(ip - base) - offset_1; const BYTE* repMatch = repIndex < prefixLowestIndex ? dictBase + (repIndex - dictIndexDelta) : @@ -1021,7 +1018,7 @@ ZSTD_compressBlock_lazy_generic( if ((mlRep >= 4) && (gain2 > gain1)) matchLength = mlRep, offset = 0, start = ip; } - if (isDMS || isDDS) { + if (isDxS) { const U32 repIndex = (U32)(ip - base) - offset_1; const BYTE* repMatch = repIndex < prefixLowestIndex ? dictBase + (repIndex - dictIndexDelta) : @@ -1059,7 +1056,7 @@ ZSTD_compressBlock_lazy_generic( && (start[-1] == (start-(offset-ZSTD_REP_MOVE))[-1]) ) /* only search for offset within prefix */ { start--; matchLength++; } } - if (isDMS || isDDS) { + if (isDxS) { U32 const matchIndex = (U32)((start-base) - (offset - ZSTD_REP_MOVE)); const BYTE* match = (matchIndex < prefixLowestIndex) ? dictBase + matchIndex - dictIndexDelta : base + matchIndex; const BYTE* const mStart = (matchIndex < prefixLowestIndex) ? dictLowest : prefixLowest; @@ -1075,7 +1072,7 @@ _storeSequence: } /* check immediate repcode */ - if (isDMS || isDDS) { + if (isDxS) { while (ip <= ilimit) { U32 const current2 = (U32)(ip-base); U32 const repIndex = current2 - offset_2; From c5fab8848ab9d643cabd6d84f9ae65b76e726fb5 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Sep 2020 18:22:49 -0400 Subject: [PATCH 15/18] Document searchFuncs Table --- lib/compress/zstd_lazy.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index f84215223..49ec1b09e 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -872,6 +872,12 @@ ZSTD_compressBlock_lazy_generic( ZSTD_matchState_t* ms, const BYTE* ip, const BYTE* iLimit, size_t* offsetPtr); + /** + * This table is indexed first by the four ZSTD_dictMode_e values, and then + * by the two searchMethod_e values. NULLs are placed for configurations + * that should never occur (extDict modes go to the other implementation + * below and there is no DDSS for binary tree search yet). + */ const searchMax_f searchFuncs[4][2] = { { ZSTD_HcFindBestMatch_selectMLS, From b6df3fd43846ac78edebc26d815ae8a7df8564af Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Sep 2020 19:19:39 -0400 Subject: [PATCH 16/18] Fix Debug Logging in 32-bit Build --- tests/fuzzer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 2c37bb0f3..579aeed94 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -2955,7 +2955,7 @@ static int basicUnitTests(U32 const seed, double compressibility) int const enableDDS = enableDedicatedDictSearch[i]; ZSTD_CDict* cdict; - DISPLAYLEVEL(5, "\n dictSize %lu cLevel %d iter %d ", dictSize, cLevel, i); + DISPLAYLEVEL(5, "\n dictSize %u cLevel %d iter %d ", (U32)dictSize, cLevel, i); ZSTD_CCtxParams_init(cctx_params, cLevel); CHECK_Z(ZSTD_CCtxParams_setParameter(cctx_params, ZSTD_c_enableDedicatedDictSearch, enableDDS)); @@ -2970,7 +2970,7 @@ static int basicUnitTests(U32 const seed, double compressibility) CHECK_Z(cSize); CHECK_Z(ZSTD_decompress_usingDict(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize, dict, dictSize)); - DISPLAYLEVEL(5, "compressed to %lu bytes ", cSize); + DISPLAYLEVEL(5, "compressed to %u bytes ", (U32)cSize); CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters)); ZSTD_freeCDict(cdict); From 6d3f816b3e232f4ab4ffdf9e14d39cb6cf796334 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Sep 2020 22:29:19 -0400 Subject: [PATCH 17/18] Test Fewer Dictionary Sizes --- tests/fuzzer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 579aeed94..a659f4872 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -2948,8 +2948,8 @@ static int basicUnitTests(U32 const seed, double compressibility) CHECK(cctx_params != NULL); - for (dictSize = CNBuffSize; dictSize; dictSize = dictSize >> 1) { - for (cLevel = 4; cLevel < 12; cLevel++) { + for (dictSize = CNBuffSize; dictSize; dictSize = dictSize >> 3) { + for (cLevel = 4; cLevel < 13; cLevel++) { for (i = 0; i < 8; ++i) { ZSTD_dictAttachPref_e const attachPref = attachPrefs[i]; int const enableDDS = enableDedicatedDictSearch[i]; From d6246d4a0fa169abda6f0f3cb682e68894c511fc Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Sep 2020 23:35:42 -0400 Subject: [PATCH 18/18] Print More During Fuzzer Test to Avoid CI Killing it Due to Timeout This is kind of hacky. And maybe this test doesn't need to be permanently as exhaustive as it is now. But while we're actively developing the DDSS, we should ensure it's compatible across many different modes. --- tests/fuzzer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index a659f4872..52086c7f3 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -2949,13 +2949,14 @@ static int basicUnitTests(U32 const seed, double compressibility) CHECK(cctx_params != NULL); for (dictSize = CNBuffSize; dictSize; dictSize = dictSize >> 3) { + DISPLAYLEVEL(3, "\n Testing with dictSize %u ", (U32)dictSize); for (cLevel = 4; cLevel < 13; cLevel++) { for (i = 0; i < 8; ++i) { ZSTD_dictAttachPref_e const attachPref = attachPrefs[i]; int const enableDDS = enableDedicatedDictSearch[i]; ZSTD_CDict* cdict; - DISPLAYLEVEL(5, "\n dictSize %u cLevel %d iter %d ", (U32)dictSize, cLevel, i); + DISPLAYLEVEL(5, "\n dictSize %u cLevel %d iter %d ", (U32)dictSize, cLevel, i); ZSTD_CCtxParams_init(cctx_params, cLevel); CHECK_Z(ZSTD_CCtxParams_setParameter(cctx_params, ZSTD_c_enableDedicatedDictSearch, enableDDS));