From 63a519dbf63c38f3b693e3007315a3a191377fec Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 11 Sep 2018 17:23:44 -0700 Subject: [PATCH 01/10] implemented first prefetch based on dictID. dictContent is prefetched up to 32 KB (no contentSize adaptation) --- NEWS | 5 ++ lib/common/compiler.h | 9 ++- lib/decompress/zstd_decompress.c | 132 ++++++++++++++++++++----------- 3 files changed, 98 insertions(+), 48 deletions(-) diff --git a/NEWS b/NEWS index e3bfb242d..1553f5ad8 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,8 @@ +v1.3.6 +perf: much faster dictionary builder, by @jenniferliu +api : reduced DDict size by 2 KB +misc: tests/paramgrill, a parameter optimizer, by @GeorgeLu97 + v1.3.5 perf: much faster dictionary compression, by @felixhandte perf: small quality improvement for dictionary generation, by @terrelln diff --git a/lib/common/compiler.h b/lib/common/compiler.h index 595bac204..a0687ddd6 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -97,12 +97,19 @@ # include /* https://msdn.microsoft.com/fr-fr/library/84szxsww(v=vs.90).aspx */ # define PREFETCH(ptr) _mm_prefetch((const char*)ptr, _MM_HINT_T0) # elif defined(__GNUC__) && ( (__GNUC__ >= 4) || ( (__GNUC__ == 3) && (__GNUC_MINOR__ >= 1) ) ) -# define PREFETCH(ptr) __builtin_prefetch(ptr, 0, 0) +# define PREFETCH(ptr) __builtin_prefetch(ptr, 0 /* rw==read */, 0 /* locality */) # else # define PREFETCH(ptr) /* disabled */ # endif #endif /* NO_PREFETCH */ +#define PREFETCH_AREA(ptr, size) { \ + size_t pos; \ + for (pos=0; pos /* For Visual 2005 */ diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index d78a286f6..ec581b6f1 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -40,7 +40,6 @@ # define ZSTD_MAXWINDOWSIZE_DEFAULT (((U32)1 << ZSTD_WINDOWLOG_DEFAULTMAX) + 1) #endif - /*! * NO_FORWARD_PROGRESS_MAX : * maximum allowed nb of calls to ZSTD_decompressStream() and ZSTD_decompress_generic() @@ -52,11 +51,12 @@ # define ZSTD_NO_FORWARD_PROGRESS_MAX 16 #endif + /*-******************************************************* * Dependencies *********************************************************/ #include /* memcpy, memmove, memset */ -#include "cpu.h" +#include "cpu.h" /* prefetch */ #include "mem.h" /* low level memory routines */ #define FSE_STATIC_LINKING_ONLY #include "fse.h" @@ -138,7 +138,6 @@ struct ZSTD_DCtx_s U32 fseEntropy; XXH64_state_t xxhState; size_t headerSize; - U32 dictID; ZSTD_format_e format; const BYTE* litPtr; ZSTD_customMem customMem; @@ -147,9 +146,13 @@ struct ZSTD_DCtx_s size_t staticSize; int bmi2; /* == 1 if the CPU supports BMI2 and 0 otherwise. CPU support is determined dynamically once per context lifetime. */ - /* streaming */ + /* dictionary */ ZSTD_DDict* ddictLocal; const ZSTD_DDict* ddict; + U32 dictID; + int ddictIsCold; /* if == 1 : dictionary is "new" for working context, and presumed "cold" (not in cpu cache) */ + + /* streaming */ ZSTD_dStreamStage streamStage; char* inBuff; size_t inBuffSize; @@ -200,6 +203,7 @@ static void ZSTD_initDCtx_internal(ZSTD_DCtx* dctx) dctx->maxWindowSize = ZSTD_MAXWINDOWSIZE_DEFAULT; dctx->ddict = NULL; dctx->ddictLocal = NULL; + dctx->ddictIsCold = 0; dctx->inBuff = NULL; dctx->inBuffSize = 0; dctx->outBuffSize = 0; @@ -574,6 +578,12 @@ size_t ZSTD_decodeLiteralsBlock(ZSTD_DCtx* dctx, { case set_repeat: if (dctx->litEntropy==0) return ERROR(dictionary_corrupted); + + /* prefetch huffman table if cold */ + if (dctx->ddictIsCold) { + PREFETCH_AREA(dctx->HUFptr, sizeof(dctx->entropy.hufTable)); + } + /* fall-through */ case set_compressed: if (srcSize < 5) return ERROR(corruption_detected); /* srcSize >= MIN_CBLOCK_SIZE == 3; here we need up to 5 for case 3 */ @@ -886,7 +896,8 @@ static size_t ZSTD_buildSeqTable(ZSTD_seqSymbol* DTableSpace, const ZSTD_seqSymb symbolEncodingType_e type, U32 max, U32 maxLog, const void* src, size_t srcSize, const U32* baseValue, const U32* nbAdditionalBits, - const ZSTD_seqSymbol* defaultTable, U32 flagRepeatTable) + const ZSTD_seqSymbol* defaultTable, U32 flagRepeatTable, + int ddictIsCold) { switch(type) { @@ -905,6 +916,12 @@ static size_t ZSTD_buildSeqTable(ZSTD_seqSymbol* DTableSpace, const ZSTD_seqSymb return 0; case set_repeat: if (!flagRepeatTable) return ERROR(corruption_detected); + /* prefetch FSE table if used */ + if (ddictIsCold) { + const void* const pStart = *DTablePtr; + size_t const pSize = sizeof(ZSTD_seqSymbol) * (SEQSYMBOL_TABLE_SIZE(maxLog)); + PREFETCH_AREA(pStart, pSize); + } return 0; case set_compressed : { U32 tableLog; @@ -989,7 +1006,8 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, LLtype, MaxLL, LLFSELog, ip, iend-ip, LL_base, LL_bits, - LL_defaultDTable, dctx->fseEntropy); + LL_defaultDTable, dctx->fseEntropy, + dctx->ddictIsCold); if (ZSTD_isError(llhSize)) return ERROR(corruption_detected); ip += llhSize; } @@ -998,7 +1016,8 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, OFtype, MaxOff, OffFSELog, ip, iend-ip, OF_base, OF_bits, - OF_defaultDTable, dctx->fseEntropy); + OF_defaultDTable, dctx->fseEntropy, + dctx->ddictIsCold); if (ZSTD_isError(ofhSize)) return ERROR(corruption_detected); ip += ofhSize; } @@ -1007,11 +1026,13 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, MLtype, MaxML, MLFSELog, ip, iend-ip, ML_base, ML_bits, - ML_defaultDTable, dctx->fseEntropy); + ML_defaultDTable, dctx->fseEntropy, + dctx->ddictIsCold); if (ZSTD_isError(mlhSize)) return ERROR(corruption_detected); ip += mlhSize; } } + dctx->ddictIsCold = 0; return ip-istart; } @@ -1679,7 +1700,8 @@ static size_t ZSTD_decompressBlock_internal(ZSTD_DCtx* dctx, /* isLongOffset must be true if there are long offsets. * Offsets are long if they are larger than 2^STREAM_ACCUMULATOR_MIN. * We don't expect that to be the case in 64-bit mode. - * In block mode, window size is not known, so we have to be conservative. (note: but it could be evaluated from current-lowLimit) + * In block mode, window size is not known, so we have to be conservative. + * (note: but it could be evaluated from current-lowLimit) */ ZSTD_longOffset_e const isLongOffset = (ZSTD_longOffset_e)(MEM_32bits() && (!frame || dctx->fParams.windowSize > (1ULL << STREAM_ACCUMULATOR_MIN))); DEBUGLOG(5, "ZSTD_decompressBlock_internal (size : %u)", (U32)srcSize); @@ -2193,8 +2215,8 @@ static size_t ZSTD_refDictContent(ZSTD_DCtx* dctx, const void* dict, size_t dict return 0; } -/* ZSTD_loadEntropy() : - * dict : must point at beginning of a valid zstd dictionary +/*! ZSTD_loadEntropy() : + * dict : must point at beginning of a valid zstd dictionary. * @return : size of entropy tables read */ static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, const void* const dict, size_t const dictSize) @@ -2206,13 +2228,11 @@ static size_t ZSTD_loadEntropy(ZSTD_entropyDTables_t* entropy, assert(MEM_readLE32(dict) == ZSTD_MAGIC_DICTIONARY); /* dict must be valid */ dictPtr += 8; /* skip header = magic + dictID */ - ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, LLTable) == 0); - ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, OFTable) == sizeof(entropy->LLTable)); - ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, MLTable) == sizeof(entropy->LLTable) + sizeof(entropy->OFTable)); - ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, hufTable) == sizeof(entropy->LLTable) + sizeof(entropy->OFTable) + sizeof(entropy->MLTable)); - ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, hufTable) >= HUF_DECOMPRESS_WORKSPACE_SIZE); - { void* const workspace = entropy; /* use fse tables as temporary workspace; implies fse table precede huffTable at beginning of entropy */ - size_t const workspaceSize = offsetof(ZSTD_entropyDTables_t, hufTable); + ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, OFTable) == offsetof(ZSTD_entropyDTables_t, LLTable) + sizeof(entropy->LLTable)); + ZSTD_STATIC_ASSERT(offsetof(ZSTD_entropyDTables_t, MLTable) == offsetof(ZSTD_entropyDTables_t, OFTable) + sizeof(entropy->OFTable)); + ZSTD_STATIC_ASSERT(sizeof(entropy->LLTable) + sizeof(entropy->OFTable) + sizeof(entropy->MLTable) >= HUF_DECOMPRESS_WORKSPACE_SIZE); + { void* const workspace = &entropy->LLTable; /* use fse tables as temporary workspace; implies fse tables are grouped together */ + size_t const workspaceSize = sizeof(entropy->LLTable) + sizeof(entropy->OFTable) + sizeof(entropy->MLTable); size_t const hSize = HUF_readDTableX2_wksp(entropy->hufTable, dictPtr, dictEnd - dictPtr, workspace, workspaceSize); @@ -2292,7 +2312,6 @@ static size_t ZSTD_decompress_insertDictionary(ZSTD_DCtx* dctx, const void* dict return ZSTD_refDictContent(dctx, dict, dictSize); } -/* Note : this function cannot fail */ size_t ZSTD_decompressBegin(ZSTD_DCtx* dctx) { assert(dctx != NULL); @@ -2346,28 +2365,44 @@ static size_t ZSTD_DDictDictSize(const ZSTD_DDict* ddict) return ddict->dictSize; } -size_t ZSTD_decompressBegin_usingDDict(ZSTD_DCtx* dstDCtx, const ZSTD_DDict* ddict) +size_t ZSTD_decompressBegin_usingDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) { - CHECK_F( ZSTD_decompressBegin(dstDCtx) ); - if (ddict) { /* support begin on NULL */ - dstDCtx->dictID = ddict->dictID; - dstDCtx->prefixStart = ddict->dictContent; - dstDCtx->virtualStart = ddict->dictContent; - dstDCtx->dictEnd = (const BYTE*)ddict->dictContent + ddict->dictSize; - dstDCtx->previousDstEnd = dstDCtx->dictEnd; + DEBUGLOG(4, "ZSTD_decompressBegin_usingDDict"); + assert(dctx != NULL); + if (ddict) { + dctx->ddictIsCold = (dctx->dictID != ddict->dictID); + DEBUGLOG(4, "DDict is %s", + dctx->ddictIsCold ? "~cold~" : "hot!"); + } + CHECK_F( ZSTD_decompressBegin(dctx) ); + if (ddict) { /* NULL ddict is equivalent to no dictionary */ + dctx->dictID = ddict->dictID; + dctx->prefixStart = ddict->dictContent; + dctx->virtualStart = ddict->dictContent; + dctx->dictEnd = (const BYTE*)ddict->dictContent + ddict->dictSize; + dctx->previousDstEnd = dctx->dictEnd; if (ddict->entropyPresent) { - dstDCtx->litEntropy = 1; - dstDCtx->fseEntropy = 1; - dstDCtx->LLTptr = ddict->entropy.LLTable; - dstDCtx->MLTptr = ddict->entropy.MLTable; - dstDCtx->OFTptr = ddict->entropy.OFTable; - dstDCtx->HUFptr = ddict->entropy.hufTable; - dstDCtx->entropy.rep[0] = ddict->entropy.rep[0]; - dstDCtx->entropy.rep[1] = ddict->entropy.rep[1]; - dstDCtx->entropy.rep[2] = ddict->entropy.rep[2]; + dctx->litEntropy = 1; + dctx->fseEntropy = 1; + dctx->LLTptr = ddict->entropy.LLTable; + dctx->MLTptr = ddict->entropy.MLTable; + dctx->OFTptr = ddict->entropy.OFTable; + dctx->HUFptr = ddict->entropy.hufTable; + dctx->entropy.rep[0] = ddict->entropy.rep[0]; + dctx->entropy.rep[1] = ddict->entropy.rep[1]; + dctx->entropy.rep[2] = ddict->entropy.rep[2]; + + /* prefetch dictionary content */ + if (dctx->ddictIsCold) { + size_t const dictSize = ddict->dictSize; + size_t const pSize = MIN(dictSize, 32 KB); /* proposed heuristic : 8 x frameContentSize => need to know frameContentSize */ + const void* const pStart = (const char*)ddict->dictContent + dictSize - pSize; + PREFETCH_AREA(pStart, pSize); + } + } else { - dstDCtx->litEntropy = 0; - dstDCtx->fseEntropy = 0; + dctx->litEntropy = 0; + dctx->fseEntropy = 0; } } return 0; @@ -2604,12 +2639,15 @@ size_t ZSTD_freeDStream(ZSTD_DStream* zds) } -/* *** Initialization *** */ +/* *** Initialization *** */ size_t ZSTD_DStreamInSize(void) { return ZSTD_BLOCKSIZE_MAX + ZSTD_blockHeaderSize; } size_t ZSTD_DStreamOutSize(void) { return ZSTD_BLOCKSIZE_MAX; } -size_t ZSTD_DCtx_loadDictionary_advanced(ZSTD_DCtx* dctx, const void* dict, size_t dictSize, ZSTD_dictLoadMethod_e dictLoadMethod, ZSTD_dictContentType_e dictContentType) +size_t ZSTD_DCtx_loadDictionary_advanced(ZSTD_DCtx* dctx, + const void* dict, size_t dictSize, + ZSTD_dictLoadMethod_e dictLoadMethod, + ZSTD_dictContentType_e dictContentType) { if (dctx->streamStage != zdss_init) return ERROR(stage_wrong); ZSTD_freeDDict(dctx->ddictLocal); @@ -2663,13 +2701,6 @@ size_t ZSTD_initDStream(ZSTD_DStream* zds) return ZSTD_initDStream_usingDict(zds, NULL, 0); } -size_t ZSTD_DCtx_refDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) -{ - if (dctx->streamStage != zdss_init) return ERROR(stage_wrong); - dctx->ddict = ddict; - return 0; -} - /* ZSTD_initDStream_usingDDict() : * ddict will just be referenced, and must outlive decompression session * this function cannot fail */ @@ -2708,6 +2739,13 @@ size_t ZSTD_setDStreamParameter(ZSTD_DStream* dctx, return 0; } +size_t ZSTD_DCtx_refDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) +{ + if (dctx->streamStage != zdss_init) return ERROR(stage_wrong); + dctx->ddict = ddict; + return 0; +} + size_t ZSTD_DCtx_setMaxWindowSize(ZSTD_DCtx* dctx, size_t maxWindowSize) { if (dctx->streamStage != zdss_init) return ERROR(stage_wrong); From 4de344d50582caebb652199fcab83f3f823a411e Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 12 Sep 2018 10:29:47 -0700 Subject: [PATCH 02/10] added conditional prefetch depending on amount of work to do. --- lib/common/compiler.h | 16 ++++++----- lib/decompress/zstd_decompress.c | 49 +++++++++++++++++--------------- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/lib/common/compiler.h b/lib/common/compiler.h index a0687ddd6..e68b81bf6 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -95,19 +95,21 @@ #else # if defined(_MSC_VER) && (defined(_M_X64) || defined(_M_I86)) /* _mm_prefetch() is not defined outside of x86/x64 */ # include /* https://msdn.microsoft.com/fr-fr/library/84szxsww(v=vs.90).aspx */ -# define PREFETCH(ptr) _mm_prefetch((const char*)ptr, _MM_HINT_T0) +# define PREFETCH(ptr) _mm_prefetch((const char*)ptr, _MM_HINT_T1) # elif defined(__GNUC__) && ( (__GNUC__ >= 4) || ( (__GNUC__ == 3) && (__GNUC_MINOR__ >= 1) ) ) -# define PREFETCH(ptr) __builtin_prefetch(ptr, 0 /* rw==read */, 0 /* locality */) +# define PREFETCH(ptr) __builtin_prefetch(ptr, 0 /* rw==read */, 2 /* locality */) # else # define PREFETCH(ptr) /* disabled */ # endif #endif /* NO_PREFETCH */ -#define PREFETCH_AREA(ptr, size) { \ - size_t pos; \ - for (pos=0; poslitEntropy==0) return ERROR(dictionary_corrupted); - - /* prefetch huffman table if cold */ - if (dctx->ddictIsCold) { - PREFETCH_AREA(dctx->HUFptr, sizeof(dctx->entropy.hufTable)); - } - /* fall-through */ + case set_compressed: if (srcSize < 5) return ERROR(corruption_detected); /* srcSize >= MIN_CBLOCK_SIZE == 3; here we need up to 5 for case 3 */ { size_t lhSize, litSize, litCSize; @@ -616,6 +611,11 @@ size_t ZSTD_decodeLiteralsBlock(ZSTD_DCtx* dctx, if (litSize > ZSTD_BLOCKSIZE_MAX) return ERROR(corruption_detected); if (litCSize + lhSize > srcSize) return ERROR(corruption_detected); + /* prefetch huffman table if cold */ + if (dctx->ddictIsCold && (litSize > 256 /* heuristic */)) { + PREFETCH_AREA(dctx->HUFptr, sizeof(dctx->entropy.hufTable)); + } + if (HUF_isError((litEncType==set_repeat) ? ( singleStream ? HUF_decompress1X_usingDTable_bmi2(dctx->litBuffer, litSize, istart+lhSize, litCSize, dctx->HUFptr, dctx->bmi2) : @@ -897,7 +897,7 @@ static size_t ZSTD_buildSeqTable(ZSTD_seqSymbol* DTableSpace, const ZSTD_seqSymb const void* src, size_t srcSize, const U32* baseValue, const U32* nbAdditionalBits, const ZSTD_seqSymbol* defaultTable, U32 flagRepeatTable, - int ddictIsCold) + int ddictIsCold, int nbSeq) { switch(type) { @@ -917,7 +917,8 @@ static size_t ZSTD_buildSeqTable(ZSTD_seqSymbol* DTableSpace, const ZSTD_seqSymb case set_repeat: if (!flagRepeatTable) return ERROR(corruption_detected); /* prefetch FSE table if used */ - if (ddictIsCold) { + if (ddictIsCold && (nbSeq > 16 /* heuristic */)) { + //if (ddictIsCold) { const void* const pStart = *DTablePtr; size_t const pSize = sizeof(ZSTD_seqSymbol) * (SEQSYMBOL_TABLE_SIZE(maxLog)); PREFETCH_AREA(pStart, pSize); @@ -974,25 +975,27 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, const BYTE* const istart = (const BYTE* const)src; const BYTE* const iend = istart + srcSize; const BYTE* ip = istart; + int nbSeq; DEBUGLOG(5, "ZSTD_decodeSeqHeaders"); /* check */ if (srcSize < MIN_SEQUENCES_SIZE) return ERROR(srcSize_wrong); /* SeqHead */ - { int nbSeq = *ip++; - if (!nbSeq) { *nbSeqPtr=0; return 1; } - if (nbSeq > 0x7F) { - if (nbSeq == 0xFF) { - if (ip+2 > iend) return ERROR(srcSize_wrong); - nbSeq = MEM_readLE16(ip) + LONGNBSEQ, ip+=2; - } else { - if (ip >= iend) return ERROR(srcSize_wrong); - nbSeq = ((nbSeq-0x80)<<8) + *ip++; - } + nbSeq = *ip++; + if (!nbSeq) { *nbSeqPtr=0; return 1; } + if (nbSeq > 0x7F) { + if (nbSeq == 0xFF) { + if (ip+2 > iend) return ERROR(srcSize_wrong); + nbSeq = MEM_readLE16(ip) + LONGNBSEQ, ip+=2; + } else { + if (ip >= iend) return ERROR(srcSize_wrong); + nbSeq = ((nbSeq-0x80)<<8) + *ip++; } - *nbSeqPtr = nbSeq; } + *nbSeqPtr = nbSeq; + DEBUGLOG(2, "nbSeqs=%i", nbSeq); + /* FSE table descriptors */ if (ip+4 > iend) return ERROR(srcSize_wrong); /* minimum possible size */ @@ -1007,7 +1010,7 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, ip, iend-ip, LL_base, LL_bits, LL_defaultDTable, dctx->fseEntropy, - dctx->ddictIsCold); + dctx->ddictIsCold, nbSeq); if (ZSTD_isError(llhSize)) return ERROR(corruption_detected); ip += llhSize; } @@ -1017,7 +1020,7 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, ip, iend-ip, OF_base, OF_bits, OF_defaultDTable, dctx->fseEntropy, - dctx->ddictIsCold); + dctx->ddictIsCold, nbSeq); if (ZSTD_isError(ofhSize)) return ERROR(corruption_detected); ip += ofhSize; } @@ -1027,7 +1030,7 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, ip, iend-ip, ML_base, ML_bits, ML_defaultDTable, dctx->fseEntropy, - dctx->ddictIsCold); + dctx->ddictIsCold, nbSeq); if (ZSTD_isError(mlhSize)) return ERROR(corruption_detected); ip += mlhSize; } @@ -2395,7 +2398,7 @@ size_t ZSTD_decompressBegin_usingDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) /* prefetch dictionary content */ if (dctx->ddictIsCold) { size_t const dictSize = ddict->dictSize; - size_t const pSize = MIN(dictSize, 32 KB); /* proposed heuristic : 8 x frameContentSize => need to know frameContentSize */ + size_t const pSize = MIN(dictSize, 2 KB); /* very conservative; would need to know Nb of Copies in dictionary, or frameContentSize as a proxy */ const void* const pStart = (const char*)ddict->dictContent + dictSize - pSize; PREFETCH_AREA(pStart, pSize); } From c49ccbc8e73c3d86758208f4d07a15a2ce6810af Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 12 Sep 2018 11:28:45 -0700 Subject: [PATCH 03/10] largeNbDicts : can select a nb of blocks will automatically truncate or repeat input as needed, to create the requested nb of blocks. default: nb of files, eventually increased appropriately if blockSize is set --- contrib/largeNbDicts/largeNbDicts.c | 60 ++++++++++++++++++----------- 1 file changed, 37 insertions(+), 23 deletions(-) diff --git a/contrib/largeNbDicts/largeNbDicts.c b/contrib/largeNbDicts/largeNbDicts.c index d094065a4..db9e730b3 100644 --- a/contrib/largeNbDicts/largeNbDicts.c +++ b/contrib/largeNbDicts/largeNbDicts.c @@ -19,8 +19,8 @@ /*--- Dependencies ---*/ #include /* size_t */ -#include /* malloc, free */ -#include /* printf */ +#include /* malloc, free, abort */ +#include /* fprintf */ #include /* assert */ #include "util.h" @@ -49,9 +49,9 @@ /*--- Macros ---*/ -#define CONTROL(c) assert(c) +#define CONTROL(c) { if (!(c)) abort(); } #undef MIN -#define MIN(a,b) ((a) < (b) ? (a) : (b)) +#define MIN(a,b) ((a) < (b) ? (a) : (b)) /*--- Display Macros ---*/ @@ -226,42 +226,50 @@ void shrinkSizes(slice_collection_t collection, } -slice_collection_t splitSlices(slice_collection_t srcSlices, size_t blockSize) +/* splitSlices() : + * nbSlices : if == 0, nbSlices is automatically determined from srcSlices and blockSize. + * otherwise, creates exactly nbSlices slices, + * by either truncating input (when smaller) + * or repeating input from beginning */ +static slice_collection_t +splitSlices(slice_collection_t srcSlices, size_t blockSize, size_t nbSlices) { if (blockSize==0) blockSize = (size_t)(-1); /* means "do not cut" */ - size_t nbBlocks = 0; + size_t nbSrcBlocks = 0; for (size_t ssnb=0; ssnb < srcSlices.nbSlices; ssnb++) { size_t pos = 0; while (pos <= srcSlices.capacities[ssnb]) { - nbBlocks++; + nbSrcBlocks++; pos += blockSize; } } - void** const sliceTable = (void**)malloc(nbBlocks * sizeof(*sliceTable)); - size_t* const capacities = (size_t*)malloc(nbBlocks * sizeof(*capacities)); + if (nbSlices == 0) nbSlices = nbSrcBlocks; + + void** const sliceTable = (void**)malloc(nbSlices * sizeof(*sliceTable)); + size_t* const capacities = (size_t*)malloc(nbSlices * sizeof(*capacities)); if (sliceTable == NULL || capacities == NULL) { free(sliceTable); free(capacities); return kNullCollection; } - size_t blockNb = 0; - for (size_t ssnb=0; ssnb < srcSlices.nbSlices; ssnb++) { + size_t ssnb = 0; + for (size_t sliceNb=0; sliceNb < nbSlices; ) { + ssnb = (ssnb + 1) % srcSlices.nbSlices; size_t pos = 0; char* const ptr = (char*)srcSlices.slicePtrs[ssnb]; - while (pos < srcSlices.capacities[ssnb]) { + while (pos < srcSlices.capacities[ssnb] && sliceNb < nbSlices) { size_t const size = MIN(blockSize, srcSlices.capacities[ssnb] - pos); - sliceTable[blockNb] = ptr + pos; - capacities[blockNb] = size; - blockNb++; + sliceTable[sliceNb] = ptr + pos; + capacities[sliceNb] = size; + sliceNb++; pos += blockSize; } } - assert(blockNb == nbBlocks); slice_collection_t result; - result.nbSlices = nbBlocks; + result.nbSlices = nbSlices; result.slicePtrs = sliceTable; result.capacities = capacities; return result; @@ -329,6 +337,7 @@ static buffer_collection_t createBufferCollection_fromFiles(const char* const * fileNamesTable, unsigned nbFiles) { U64 const totalSizeToLoad = UTIL_getTotalFileSize(fileNamesTable, nbFiles); + assert(totalSizeToLoad != UTIL_FILESIZE_UNKNOWN); assert(totalSizeToLoad <= BENCH_SIZE_MAX); size_t const loadedSize = (size_t)totalSizeToLoad; assert(loadedSize > 0); @@ -565,7 +574,9 @@ static int benchMem(slice_collection_t dstBlocks, * @return : 0 is success, 1+ otherwise */ int bench(const char** fileNameTable, unsigned nbFiles, const char* dictionary, - size_t blockSize, int clevel, unsigned nbDictMax, int nbRounds) + size_t blockSize, int clevel, + unsigned nbDictMax, unsigned nbBlocks, + int nbRounds) { int result = 0; @@ -577,8 +588,8 @@ int bench(const char** fileNameTable, unsigned nbFiles, DISPLAYLEVEL(3, "created src buffer of size %.1f MB \n", (double)srcSize / (1 MB)); - slice_collection_t const srcSlices = splitSlices(srcs.slices, blockSize); - unsigned const nbBlocks = (unsigned)(srcSlices.nbSlices); + slice_collection_t const srcSlices = splitSlices(srcs.slices, blockSize, nbBlocks); + nbBlocks = (unsigned)(srcSlices.nbSlices); DISPLAYLEVEL(3, "split input into %u blocks ", nbBlocks); if (blockSize) DISPLAYLEVEL(3, "of max size %u bytes ", (unsigned)blockSize); @@ -596,10 +607,10 @@ int bench(const char** fileNameTable, unsigned nbFiles, buffer_t dstBuffer = createBuffer(dstBufferCapacity); CONTROL(dstBuffer.ptr != NULL); - void** const sliceTable = (void**)malloc(nbBlocks * sizeof(*sliceTable)); + void** const sliceTable = malloc(nbBlocks * sizeof(*sliceTable)); CONTROL(sliceTable != NULL); - { char* const ptr = (char*)dstBuffer.ptr; + { char* const ptr = dstBuffer.ptr; size_t pos = 0; for (size_t snb=0; snb < nbBlocks; snb++) { sliceTable[snb] = ptr + pos; @@ -727,6 +738,7 @@ int usage(const char* exeName) DISPLAY ("-# : use compression level # (default: %u) \n", CLEVEL_DEFAULT); DISPLAY ("-D # : use # as a dictionary (default: create one) \n"); DISPLAY ("-i# : nb benchmark rounds (default: %u) \n", BENCH_TIME_DEFAULT_S); + DISPLAY ("--nbBlocks=#: use # blocks for bench (default: one per file) \n"); DISPLAY ("--nbDicts=# : create # dictionaries for bench (default: one per block) \n"); DISPLAY ("-h : help (this text) \n"); return 0; @@ -755,6 +767,7 @@ int main (int argc, const char** argv) int cLevel = CLEVEL_DEFAULT; size_t blockSize = BLOCKSIZE_DEFAULT; size_t nbDicts = 0; /* determine nbDicts automatically: 1 dictionary per block */ + size_t nbBlocks = 0; /* determine nbBlocks automatically, from source and blockSize */ for (int argNb = 1; argNb < argc ; argNb++) { const char* argument = argv[argNb]; @@ -766,6 +779,7 @@ int main (int argc, const char** argv) if (longCommandWArg(&argument, "-B")) { blockSize = readU32FromChar(&argument); continue; } if (longCommandWArg(&argument, "--blockSize=")) { blockSize = readU32FromChar(&argument); continue; } if (longCommandWArg(&argument, "--nbDicts=")) { nbDicts = readU32FromChar(&argument); continue; } + if (longCommandWArg(&argument, "--nbBlocks=")) { nbBlocks = readU32FromChar(&argument); continue; } if (longCommandWArg(&argument, "--clevel=")) { cLevel = readU32FromChar(&argument); continue; } if (longCommandWArg(&argument, "-")) { cLevel = readU32FromChar(&argument); continue; } /* anything that's not a command is a filename */ @@ -783,7 +797,7 @@ int main (int argc, const char** argv) filenameTable = UTIL_createFileList(nameTable, nameIdx, &buffer_containing_filenames, &nbFiles, 1 /* follow_links */); } - int result = bench(filenameTable, nbFiles, dictionary, blockSize, cLevel, nbDicts, nbRounds); + int result = bench(filenameTable, nbFiles, dictionary, blockSize, cLevel, nbDicts, nbBlocks, nbRounds); free(buffer_containing_filenames); free(nameTable); From 5fb5ed3b314fca86f6c4a0f960cdd106108f6e08 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 12 Sep 2018 12:32:09 -0700 Subject: [PATCH 04/10] adjust heuristic decisions --- lib/decompress/zstd_decompress.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 3c08c19f4..5080b9439 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -612,7 +612,7 @@ size_t ZSTD_decodeLiteralsBlock(ZSTD_DCtx* dctx, if (litCSize + lhSize > srcSize) return ERROR(corruption_detected); /* prefetch huffman table if cold */ - if (dctx->ddictIsCold && (litSize > 256 /* heuristic */)) { + if (dctx->ddictIsCold && (litSize > 768 /* heuristic */)) { PREFETCH_AREA(dctx->HUFptr, sizeof(dctx->entropy.hufTable)); } @@ -917,8 +917,7 @@ static size_t ZSTD_buildSeqTable(ZSTD_seqSymbol* DTableSpace, const ZSTD_seqSymb case set_repeat: if (!flagRepeatTable) return ERROR(corruption_detected); /* prefetch FSE table if used */ - if (ddictIsCold && (nbSeq > 16 /* heuristic */)) { - //if (ddictIsCold) { + if (ddictIsCold && (nbSeq > 24 /* heuristic */)) { const void* const pStart = *DTablePtr; size_t const pSize = sizeof(ZSTD_seqSymbol) * (SEQSYMBOL_TABLE_SIZE(maxLog)); PREFETCH_AREA(pStart, pSize); From 44d3b83bb1cdfc7560f27f0a2902d2b283c11bd6 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 12 Sep 2018 15:35:21 -0700 Subject: [PATCH 05/10] conditional dict content prefetching based on nbSeq. --- lib/decompress/zstd_decompress.c | 38 +++++++++++++++++--------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 5080b9439..7e2b60e81 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -68,6 +68,9 @@ # include "zstd_legacy.h" #endif +static const void* ZSTD_DDictDictContent(const ZSTD_DDict* ddict); +static size_t ZSTD_DDictDictSize(const ZSTD_DDict* ddict); + /*-************************************* * Errors @@ -148,7 +151,7 @@ struct ZSTD_DCtx_s /* dictionary */ ZSTD_DDict* ddictLocal; - const ZSTD_DDict* ddict; + const ZSTD_DDict* ddict; /* set by ZSTD_initDStream_usingDDict(), or ZSTD_DCtx_refDDict() */ U32 dictID; int ddictIsCold; /* if == 1 : dictionary is "new" for working context, and presumed "cold" (not in cpu cache) */ @@ -993,8 +996,6 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, } } *nbSeqPtr = nbSeq; - DEBUGLOG(2, "nbSeqs=%i", nbSeq); - /* FSE table descriptors */ if (ip+4 > iend) return ERROR(srcSize_wrong); /* minimum possible size */ @@ -1034,7 +1035,16 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, ip += mlhSize; } } - dctx->ddictIsCold = 0; + + /* prefetch dictionary content */ + if (dctx->ddictIsCold) { + size_t const dictSize = (const char*)dctx->prefixStart - (const char*)dctx->virtualStart; + size_t const pSize = MIN(dictSize, (size_t)(64*nbSeq)); + const void* const pStart = (const char*)dctx->dictEnd - pSize; + DEBUGLOG(2, "dictSize: %zu ; prefetchSize: %zu", dictSize, pSize); + PREFETCH_AREA(pStart, pSize); + dctx->ddictIsCold = 0; + } return ip-istart; } @@ -1911,9 +1921,6 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx, return op-ostart; } -static const void* ZSTD_DDictDictContent(const ZSTD_DDict* ddict); -static size_t ZSTD_DDictDictSize(const ZSTD_DDict* ddict); - static size_t ZSTD_decompressMultiFrame(ZSTD_DCtx* dctx, void* dst, size_t dstCapacity, const void* src, size_t srcSize, @@ -1922,6 +1929,8 @@ static size_t ZSTD_decompressMultiFrame(ZSTD_DCtx* dctx, { void* const dststart = dst; int moreThan1Frame = 0; + + DEBUGLOG(5, "ZSTD_decompressMultiFrame"); assert(dict==NULL || ddict==NULL); /* either dict or ddict set, not both */ if (ddict) { @@ -2359,11 +2368,13 @@ struct ZSTD_DDict_s { static const void* ZSTD_DDictDictContent(const ZSTD_DDict* ddict) { + assert(ddict != NULL); return ddict->dictContent; } static size_t ZSTD_DDictDictSize(const ZSTD_DDict* ddict) { + assert(ddict != NULL); return ddict->dictSize; } @@ -2372,8 +2383,8 @@ size_t ZSTD_decompressBegin_usingDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) DEBUGLOG(4, "ZSTD_decompressBegin_usingDDict"); assert(dctx != NULL); if (ddict) { - dctx->ddictIsCold = (dctx->dictID != ddict->dictID); - DEBUGLOG(4, "DDict is %s", + dctx->ddictIsCold = (dctx->dictEnd != (const char*)ddict->dictContent + ddict->dictSize); + DEBUGLOG(2, "DDict is %s", dctx->ddictIsCold ? "~cold~" : "hot!"); } CHECK_F( ZSTD_decompressBegin(dctx) ); @@ -2393,15 +2404,6 @@ size_t ZSTD_decompressBegin_usingDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) dctx->entropy.rep[0] = ddict->entropy.rep[0]; dctx->entropy.rep[1] = ddict->entropy.rep[1]; dctx->entropy.rep[2] = ddict->entropy.rep[2]; - - /* prefetch dictionary content */ - if (dctx->ddictIsCold) { - size_t const dictSize = ddict->dictSize; - size_t const pSize = MIN(dictSize, 2 KB); /* very conservative; would need to know Nb of Copies in dictionary, or frameContentSize as a proxy */ - const void* const pStart = (const char*)ddict->dictContent + dictSize - pSize; - PREFETCH_AREA(pStart, pSize); - } - } else { dctx->litEntropy = 0; dctx->fseEntropy = 0; From 2618253da213a08ab571d8ec38fc963d78882ba4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 12 Sep 2018 16:15:37 -0700 Subject: [PATCH 06/10] fixed PREFETCH() macro for corner cases and platforms without this instruction --- lib/common/compiler.h | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/common/compiler.h b/lib/common/compiler.h index e68b81bf6..31eb1ccdf 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -91,25 +91,27 @@ /* prefetch * can be disabled, by declaring NO_PREFETCH macro */ #if defined(NO_PREFETCH) -# define PREFETCH(ptr) /* disabled */ +# define PREFETCH(ptr) (void)(ptr) /* disabled */ #else # if defined(_MSC_VER) && (defined(_M_X64) || defined(_M_I86)) /* _mm_prefetch() is not defined outside of x86/x64 */ # include /* https://msdn.microsoft.com/fr-fr/library/84szxsww(v=vs.90).aspx */ -# define PREFETCH(ptr) _mm_prefetch((const char*)ptr, _MM_HINT_T1) +# define PREFETCH(ptr) _mm_prefetch((const char*)(ptr), _MM_HINT_T1) # elif defined(__GNUC__) && ( (__GNUC__ >= 4) || ( (__GNUC__ == 3) && (__GNUC_MINOR__ >= 1) ) ) -# define PREFETCH(ptr) __builtin_prefetch(ptr, 0 /* rw==read */, 2 /* locality */) +# define PREFETCH(ptr) __builtin_prefetch((ptr), 0 /* rw==read */, 2 /* locality */) # else -# define PREFETCH(ptr) /* disabled */ +# define PREFETCH(ptr) (void)(ptr) /* disabled */ # endif #endif /* NO_PREFETCH */ #define CACHELINE_SIZE 64 -#define PREFETCH_AREA(ptr, size) { \ - size_t pos; \ - for (pos=0; pos Date: Wed, 12 Sep 2018 16:40:28 -0700 Subject: [PATCH 07/10] clean traces --- lib/decompress/zstd_decompress.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 7e2b60e81..e71e961f4 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1041,7 +1041,6 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr, size_t const dictSize = (const char*)dctx->prefixStart - (const char*)dctx->virtualStart; size_t const pSize = MIN(dictSize, (size_t)(64*nbSeq)); const void* const pStart = (const char*)dctx->dictEnd - pSize; - DEBUGLOG(2, "dictSize: %zu ; prefetchSize: %zu", dictSize, pSize); PREFETCH_AREA(pStart, pSize); dctx->ddictIsCold = 0; } @@ -2384,7 +2383,7 @@ size_t ZSTD_decompressBegin_usingDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) assert(dctx != NULL); if (ddict) { dctx->ddictIsCold = (dctx->dictEnd != (const char*)ddict->dictContent + ddict->dictSize); - DEBUGLOG(2, "DDict is %s", + DEBUGLOG(4, "DDict is %s", dctx->ddictIsCold ? "~cold~" : "hot!"); } CHECK_F( ZSTD_decompressBegin(dctx) ); From 674dd21bd0b82efbcebc3c4bff188d8dbfafde96 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 12 Sep 2018 16:54:57 -0700 Subject: [PATCH 08/10] final parameter tuning --- doc/zstd_manual.html | 7 +++++-- lib/decompress/zstd_decompress.c | 3 ++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/doc/zstd_manual.html b/doc/zstd_manual.html index bd792008b..e168fa2e1 100644 --- a/doc/zstd_manual.html +++ b/doc/zstd_manual.html @@ -181,7 +181,8 @@ size_t ZSTD_freeDCtx(ZSTD_DCtx* dctx);

When compressing multiple messages / blocks with the same dictionary, it's recommended to load it just once. ZSTD_createCDict() will create a digested dictionary, ready to start future compression operations without startup delay. ZSTD_CDict can be created once and shared by multiple threads concurrently, since its usage is read-only. - `dictBuffer` can be released after ZSTD_CDict creation, since its content is copied within CDict + `dictBuffer` can be released after ZSTD_CDict creation, since its content is copied within CDict + Note : A ZSTD_CDict can be created with an empty dictionary, but it is inefficient for small data. 


size_t      ZSTD_freeCDict(ZSTD_CDict* CDict);
@@ -195,7 +196,9 @@ size_t     ZSTD_freeDCtx(ZSTD_DCtx* dctx);
 
  Compression using a digested Dictionary.
   Faster startup than ZSTD_compress_usingDict(), recommended when same dictionary is used multiple times.
   Note that compression level is decided during dictionary creation.
-  Frame parameters are hardcoded (dictID=yes, contentSize=yes, checksum=no) 
+  Frame parameters are hardcoded (dictID=yes, contentSize=yes, checksum=no)
+  Note : ZSTD_compress_usingCDict() can be used with a ZSTD_CDict created from an empty dictionary.
+         But it is inefficient for small data, and it is recommended to use ZSTD_compressCCtx(). 
 

ZSTD_DDict* ZSTD_createDDict(const void* dictBuffer, size_t dictSize);
diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c
index e71e961f4..0f02540fc 100644
--- a/lib/decompress/zstd_decompress.c
+++ b/lib/decompress/zstd_decompress.c
@@ -1039,7 +1039,8 @@ size_t ZSTD_decodeSeqHeaders(ZSTD_DCtx* dctx, int* nbSeqPtr,
     /* prefetch dictionary content */
     if (dctx->ddictIsCold) {
         size_t const dictSize = (const char*)dctx->prefixStart - (const char*)dctx->virtualStart;
-        size_t const pSize = MIN(dictSize, (size_t)(64*nbSeq));
+        size_t const psmin = MIN(dictSize, (size_t)(64*nbSeq) /* heuristic */ );
+        size_t const pSize = MIN(psmin, 128 KB /* protection */ );
         const void* const pStart = (const char*)dctx->dictEnd - pSize;
         PREFETCH_AREA(pStart, pSize);
         dctx->ddictIsCold = 0;

From d195eec97e124eff513e4a254c1faa47ba450c9f Mon Sep 17 00:00:00 2001
From: Yann Collet 
Date: Thu, 13 Sep 2018 12:29:52 -0700
Subject: [PATCH 09/10] fixed msan error

cold dictionary is detected through a comparison with dictEnd,
which was not initialized at the beginning of first DCtx usage.
---
 lib/decompress/zstd_decompress.c |  1 +
 tests/zstreamtest.c              | 28 ++++++++++++++--------------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c
index 0f02540fc..9d7408336 100644
--- a/lib/decompress/zstd_decompress.c
+++ b/lib/decompress/zstd_decompress.c
@@ -206,6 +206,7 @@ static void ZSTD_initDCtx_internal(ZSTD_DCtx* dctx)
     dctx->maxWindowSize = ZSTD_MAXWINDOWSIZE_DEFAULT;
     dctx->ddict       = NULL;
     dctx->ddictLocal  = NULL;
+    dctx->dictEnd     = NULL;
     dctx->ddictIsCold = 0;
     dctx->inBuff      = NULL;
     dctx->inBuffSize  = 0;
diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c
index 0e0fbe0c9..96136a625 100644
--- a/tests/zstreamtest.c
+++ b/tests/zstreamtest.c
@@ -135,34 +135,34 @@ typedef struct {
     size_t filled;
 } buffer_t;
 
-static const buffer_t g_nullBuffer = { NULL, 0 , 0 };
+static const buffer_t kBuffNull = { NULL, 0 , 0 };
+
+static void FUZ_freeDictionary(buffer_t dict)
+{
+    free(dict.start);
+}
 
 static buffer_t FUZ_createDictionary(const void* src, size_t srcSize, size_t blockSize, size_t requestedDictSize)
 {
-    buffer_t dict = { NULL, 0, 0 };
+    buffer_t dict = kBuffNull;
     size_t const nbBlocks = (srcSize + (blockSize-1)) / blockSize;
-    size_t* const blockSizes = (size_t*) malloc(nbBlocks * sizeof(size_t));
-    if (!blockSizes) return dict;
+    size_t* const blockSizes = (size_t*)malloc(nbBlocks * sizeof(size_t));
+    if (!blockSizes) return kBuffNull;
     dict.start = malloc(requestedDictSize);
-    if (!dict.start) { free(blockSizes); return dict; }
+    if (!dict.start) { free(blockSizes); return kBuffNull; }
     {   size_t nb;
         for (nb=0; nb
Date: Thu, 13 Sep 2018 16:44:04 -0700
Subject: [PATCH 10/10] updated code comments, based on @terrelln review

---
 lib/common/compiler.h            | 8 +++++++-
 lib/decompress/zstd_decompress.c | 3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/common/compiler.h b/lib/common/compiler.h
index 31eb1ccdf..07f875e4d 100644
--- a/lib/common/compiler.h
+++ b/lib/common/compiler.h
@@ -89,7 +89,13 @@
 #endif
 
 /* prefetch
- * can be disabled, by declaring NO_PREFETCH macro */
+ * can be disabled, by declaring NO_PREFETCH macro
+ * All prefetch invocations use a single default locality 2,
+ * generating instruction prefetcht1,
+ * which, according to Intel, means "load data into L2 cache".
+ * This is a good enough "middle ground" for the time being,
+ * though in theory, it would be better to specialize locality depending on data being prefetched.
+ * Tests could not determine any sensible difference based on locality value. */
 #if defined(NO_PREFETCH)
 #  define PREFETCH(ptr)     (void)(ptr)  /* disabled */
 #else
diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c
index 9d7408336..1382c9c7f 100644
--- a/lib/decompress/zstd_decompress.c
+++ b/lib/decompress/zstd_decompress.c
@@ -56,7 +56,8 @@
 *  Dependencies
 *********************************************************/
 #include       /* memcpy, memmove, memset */
-#include "cpu.h"         /* prefetch */
+#include "compiler.h"    /* prefetch */
+#include "cpu.h"         /* bmi2 */
 #include "mem.h"         /* low level memory routines */
 #define FSE_STATIC_LINKING_ONLY
 #include "fse.h"