From 8bed4012bdd7fcf65795611f8c6ab1a89337d015 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 8 Nov 2018 12:36:39 -0800 Subject: [PATCH 1/4] fixed decompression-only benchmark --- lib/decompress/zstd_decompress_block.c | 41 ++++++++++++++++---------- programs/bench.c | 40 +++++++++++++------------ 2 files changed, 47 insertions(+), 34 deletions(-) diff --git a/lib/decompress/zstd_decompress_block.c b/lib/decompress/zstd_decompress_block.c index 652ddd194..68298772e 100644 --- a/lib/decompress/zstd_decompress_block.c +++ b/lib/decompress/zstd_decompress_block.c @@ -1026,17 +1026,18 @@ ZSTD_decompressSequencesLong_body( /* Regen sequences */ if (nbSeq) { #define STORED_SEQS 4 -#define STOSEQ_MASK (STORED_SEQS-1) +#define STORED_SEQS_MASK (STORED_SEQS-1) #define ADVANCED_SEQS 4 seq_t sequences[STORED_SEQS]; int const seqAdvance = MIN(nbSeq, ADVANCED_SEQS); seqState_t seqState; int seqNb; dctx->fseEntropy = 1; - { U32 i; for (i=0; ientropy.rep[i]; } + { int i; for (i=0; ientropy.rep[i]; } seqState.prefixStart = prefixStart; seqState.pos = (size_t)(op-prefixStart); seqState.dictEnd = dictEnd; + assert(iend > ip); CHECK_E(BIT_initDStream(&seqState.DStream, ip, iend-ip), corruption_detected); ZSTD_initFseState(&seqState.stateLL, &seqState.DStream, dctx->LLTptr); ZSTD_initFseState(&seqState.stateOffb, &seqState.DStream, dctx->OFTptr); @@ -1051,10 +1052,10 @@ ZSTD_decompressSequencesLong_body( /* decode and decompress */ for ( ; (BIT_reloadDStream(&(seqState.DStream)) <= BIT_DStream_completed) && (seqNbentropy.rep[i] = (U32)(seqState.prevOffset[i]); } #undef STORED_SEQS -#undef STOSEQ_MASK +#undef STORED_SEQS_MASK #undef ADVANCED_SEQS } @@ -1118,9 +1119,10 @@ ZSTD_decompressSequencesLong_bmi2(ZSTD_DCtx* dctx, #endif typedef size_t (*ZSTD_decompressSequences_t)( - ZSTD_DCtx *dctx, void *dst, size_t maxDstSize, - const void *seqStart, size_t seqSize, int nbSeq, - const ZSTD_longOffset_e isLongOffset); + ZSTD_DCtx* dctx, + void* dst, size_t maxDstSize, + const void* seqStart, size_t seqSize, int nbSeq, + const ZSTD_longOffset_e isLongOffset); static size_t ZSTD_decompressSequences(ZSTD_DCtx* dctx, void* dst, size_t maxDstSize, @@ -1136,10 +1138,17 @@ ZSTD_decompressSequences(ZSTD_DCtx* dctx, void* dst, size_t maxDstSize, return ZSTD_decompressSequences_default(dctx, dst, maxDstSize, seqStart, seqSize, nbSeq, isLongOffset); } -static size_t ZSTD_decompressSequencesLong(ZSTD_DCtx* dctx, - void* dst, size_t maxDstSize, - const void* seqStart, size_t seqSize, int nbSeq, - const ZSTD_longOffset_e isLongOffset) + +/* ZSTD_decompressSequencesLong() : + * decompression function triggered when a minimum share of offsets is considered "long", + * aka out of cache. + * note : "long" definition seems overloaded here, sometimes meaning "wider than bitstream register", and sometimes mearning "farther than memory cache distance". + * This function will try to mitigate main memory latency through the use of prefetching */ +static size_t +ZSTD_decompressSequencesLong(ZSTD_DCtx* dctx, + void* dst, size_t maxDstSize, + const void* seqStart, size_t seqSize, int nbSeq, + const ZSTD_longOffset_e isLongOffset) { DEBUGLOG(5, "ZSTD_decompressSequencesLong"); #if DYNAMIC_BMI2 @@ -1150,6 +1159,8 @@ static size_t ZSTD_decompressSequencesLong(ZSTD_DCtx* dctx, return ZSTD_decompressSequencesLong_default(dctx, dst, maxDstSize, seqStart, seqSize, nbSeq, isLongOffset); } + + /* ZSTD_getLongOffsetsShare() : * condition : offTable must be valid * @return : "share" of long offsets (arbitrarily defined as > (1<<23)) @@ -1188,7 +1199,7 @@ ZSTD_decompressBlock_internal(ZSTD_DCtx* dctx, * 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))); + 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); if (srcSize >= ZSTD_BLOCKSIZE_MAX) return ERROR(srcSize_wrong); @@ -1208,7 +1219,7 @@ ZSTD_decompressBlock_internal(ZSTD_DCtx* dctx, ip += seqHSize; srcSize -= seqHSize; - if ( (!frame || dctx->fParams.windowSize > (1<<24)) + if ( (!frame || (dctx->fParams.windowSize > (1<<24))) && (nbSeq>0) ) { /* could probably use a larger nbSeq limit */ U32 const shareLongOffsets = ZSTD_getLongOffsetsShare(dctx->OFTptr); U32 const minShare = MEM_64bits() ? 7 : 20; /* heuristic values, correspond to 2.73% and 7.81% */ diff --git a/programs/bench.c b/programs/bench.c index 326c1c1c5..caa803572 100644 --- a/programs/bench.c +++ b/programs/bench.c @@ -522,22 +522,24 @@ static BMK_benchOutcome_t BMK_benchOutcome_setValidResult(BMK_benchResult_t resu /* benchMem with no allocation */ -static BMK_benchOutcome_t BMK_benchMemAdvancedNoAlloc( - const void** srcPtrs, size_t* srcSizes, - void** cPtrs, size_t* cCapacities, size_t* cSizes, - void** resPtrs, size_t* resSizes, - void** resultBufferPtr, void* compressedBuffer, - size_t maxCompressedSize, - BMK_timedFnState_t* timeStateCompress, - BMK_timedFnState_t* timeStateDecompress, +static BMK_benchOutcome_t +BMK_benchMemAdvancedNoAlloc( + const void** srcPtrs, size_t* srcSizes, + void** cPtrs, size_t* cCapacities, size_t* cSizes, + void** resPtrs, size_t* resSizes, + void** resultBufferPtr, void* compressedBuffer, + size_t maxCompressedSize, + BMK_timedFnState_t* timeStateCompress, + BMK_timedFnState_t* timeStateDecompress, - const void* srcBuffer, size_t srcSize, - const size_t* fileSizes, unsigned nbFiles, - const int cLevel, const ZSTD_compressionParameters* comprParams, - const void* dictBuffer, size_t dictBufferSize, - ZSTD_CCtx* cctx, ZSTD_DCtx* dctx, - int displayLevel, const char* displayName, - const BMK_advancedParams_t* adv) + const void* srcBuffer, size_t srcSize, + const size_t* fileSizes, unsigned nbFiles, + const int cLevel, + const ZSTD_compressionParameters* comprParams, + const void* dictBuffer, size_t dictBufferSize, + ZSTD_CCtx* cctx, ZSTD_DCtx* dctx, + int displayLevel, const char* displayName, + const BMK_advancedParams_t* adv) { size_t const blockSize = ((adv->blockSize>=32 && (adv->mode != BMK_decodeOnly)) ? adv->blockSize : srcSize) + (!srcSize); /* avoid div by 0 */ BMK_benchResult_t benchResult; @@ -599,6 +601,7 @@ static BMK_benchOutcome_t BMK_benchMemAdvancedNoAlloc( cPtr += cCapacities[nbBlocks]; resPtr += thisBlockSize; remaining -= thisBlockSize; + if (BMK_decodeOnly) { assert(nbBlocks==0); cSizes[nbBlocks] = thisBlockSize; } } } } @@ -633,7 +636,6 @@ static BMK_benchOutcome_t BMK_benchMemAdvancedNoAlloc( DISPLAYLEVEL(2, "%2s-%-17.17s :%10u ->\r", marks[markNb], displayName, (U32)srcSize); while (!(compressionCompleted && decompressionCompleted)) { - if (!compressionCompleted) { BMK_runOutcome_t const cOutcome = BMK_benchTimedFn( timeStateCompress, @@ -659,7 +661,6 @@ static BMK_benchOutcome_t BMK_benchMemAdvancedNoAlloc( } } { int const ratioAccuracy = (ratio < 10.) ? 3 : 2; - markNb = (markNb+1) % NB_MARKS; DISPLAYLEVEL(2, "%2s-%-17.17s :%10u ->%10u (%5.*f),%6.*f MB/s\r", marks[markNb], displayName, (U32)srcSize, (U32)cSize, @@ -690,7 +691,6 @@ static BMK_benchOutcome_t BMK_benchMemAdvancedNoAlloc( } { int const ratioAccuracy = (ratio < 10.) ? 3 : 2; - markNb = (markNb+1) % NB_MARKS; DISPLAYLEVEL(2, "%2s-%-17.17s :%10u ->%10u (%5.*f),%6.*f MB/s ,%6.1f MB/s \r", marks[markNb], displayName, (U32)srcSize, (U32)benchResult.cSize, @@ -700,6 +700,7 @@ static BMK_benchOutcome_t BMK_benchMemAdvancedNoAlloc( } decompressionCompleted = BMK_isCompleted_TimedFn(timeStateDecompress); } + markNb = (markNb+1) % NB_MARKS; } /* while (!(compressionCompleted && decompressionCompleted)) */ /* CRC Checking */ @@ -707,7 +708,8 @@ static BMK_benchOutcome_t BMK_benchMemAdvancedNoAlloc( U64 const crcCheck = XXH64(resultBuffer, srcSize, 0); if ((adv->mode == BMK_both) && (crcOrig!=crcCheck)) { size_t u; - DISPLAY("!!! WARNING !!! %14s : Invalid Checksum : %x != %x \n", displayName, (unsigned)crcOrig, (unsigned)crcCheck); + DISPLAY("!!! WARNING !!! %14s : Invalid Checksum : %x != %x \n", + displayName, (unsigned)crcOrig, (unsigned)crcCheck); for (u=0; u Date: Thu, 8 Nov 2018 12:47:46 -0800 Subject: [PATCH 2/4] improve long-range decoder speed on enwik9 at level 22 (which is almost a worst case scenario), speed improves by +7% on my laptop (415 -> 445 MB/s) --- lib/common/compiler.h | 18 ++++++++---------- lib/decompress/zstd_decompress_block.c | 2 +- programs/bench.c | 6 +++++- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/common/compiler.h b/lib/common/compiler.h index 07f875e4d..cc830b2b4 100644 --- a/lib/common/compiler.h +++ b/lib/common/compiler.h @@ -89,23 +89,21 @@ #endif /* prefetch - * 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. */ + * can be disabled, by declaring NO_PREFETCH build macro */ #if defined(NO_PREFETCH) +# define PREFETCH_L1(ptr) (void)(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_L1(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 */, 2 /* locality */) +# define PREFETCH_L1(ptr) __builtin_prefetch((ptr), 0 /* rw==read */, 3 /* locality */) +# define PREFETCH(ptr) __builtin_prefetch((ptr), 0 /* rw==read */, 2 /* locality */) # else -# define PREFETCH(ptr) (void)(ptr) /* disabled */ +# define PREFETCH_L1(ptr) (void)(ptr) /* disabled */ +# define PREFETCH(ptr) (void)(ptr) /* disabled */ # endif #endif /* NO_PREFETCH */ diff --git a/lib/decompress/zstd_decompress_block.c b/lib/decompress/zstd_decompress_block.c index 68298772e..ef424877b 100644 --- a/lib/decompress/zstd_decompress_block.c +++ b/lib/decompress/zstd_decompress_block.c @@ -1054,7 +1054,7 @@ ZSTD_decompressSequencesLong_body( seq_t const sequence = ZSTD_decodeSequenceLong(&seqState, isLongOffset); size_t const oneSeqSize = ZSTD_execSequenceLong(op, oend, sequences[(seqNb-ADVANCED_SEQS) & STORED_SEQS_MASK], &litPtr, litEnd, prefixStart, dictStart, dictEnd); if (ZSTD_isError(oneSeqSize)) return oneSeqSize; - PREFETCH(sequence.match); /* note : it's safe to invoke PREFETCH() on any memory address, including invalid ones */ + PREFETCH_L1(sequence.match); PREFETCH_L1(sequence.match + sequence.matchLength - 1); /* note : it's safe to invoke PREFETCH() on any memory address, including invalid ones */ sequences[seqNb & STORED_SEQS_MASK] = sequence; op += oneSeqSize; } diff --git a/programs/bench.c b/programs/bench.c index caa803572..2bbaa9d0a 100644 --- a/programs/bench.c +++ b/programs/bench.c @@ -601,7 +601,11 @@ BMK_benchMemAdvancedNoAlloc( cPtr += cCapacities[nbBlocks]; resPtr += thisBlockSize; remaining -= thisBlockSize; - if (BMK_decodeOnly) { assert(nbBlocks==0); cSizes[nbBlocks] = thisBlockSize; } + if (BMK_decodeOnly) { + assert(nbBlocks==0); + cSizes[nbBlocks] = thisBlockSize; + benchResult.cSize = thisBlockSize; + } } } } From f40c6b43951cb81e3a027c03e5ed2827648194e6 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 8 Nov 2018 12:51:42 -0800 Subject: [PATCH 3/4] added a simple test case for decode-only benchmark mode as it was broken and no one noticed --- tests/playTests.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/playTests.sh b/tests/playTests.sh index b86a0dc40..f35f5fee6 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -541,6 +541,9 @@ $ECHO "bench negative level" $ZSTD -bi0 --fast tmp1 $ECHO "with recursive and quiet modes" $ZSTD -rqi1b1e2 tmp1 +$ECHO "benchmark decompression only" +$ZSTD -f tmp1 +$ZSTD -b -d -i1 tmp1.zst $ECHO "\n===> zstd compatibility tests " From 20fb9e7f36f004d6509cfb3b07be7fdeafee0f63 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 8 Nov 2018 12:57:34 -0800 Subject: [PATCH 4/4] reduced assertion strength one limit case can apparently be generated during fuzzer tests --- lib/decompress/zstd_decompress_block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/decompress/zstd_decompress_block.c b/lib/decompress/zstd_decompress_block.c index ef424877b..71bb8e97b 100644 --- a/lib/decompress/zstd_decompress_block.c +++ b/lib/decompress/zstd_decompress_block.c @@ -1037,7 +1037,7 @@ ZSTD_decompressSequencesLong_body( seqState.prefixStart = prefixStart; seqState.pos = (size_t)(op-prefixStart); seqState.dictEnd = dictEnd; - assert(iend > ip); + assert(iend >= ip); CHECK_E(BIT_initDStream(&seqState.DStream, ip, iend-ip), corruption_detected); ZSTD_initFseState(&seqState.stateLL, &seqState.DStream, dctx->LLTptr); ZSTD_initFseState(&seqState.stateOffb, &seqState.DStream, dctx->OFTptr);