From 327cf6fac115fab8ff32adaad2a2727286abf2a4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 28 May 2019 15:26:52 -0700 Subject: [PATCH 1/3] nextToUpdate3 does not need to be maintained outside of zstd_opt.c It's re-synchronized with nextToUpdate at beginning of each block. It only needs to be tracked from within zstd_opt block parser. Made the logic clear, so that no code tried to maintain this variable. An even better solution would be to make nextToUpdate3 an internal variable of ZSTD_compressBlock_opt_generic(). That would make it possible to remove it from ZSTD_matchState_t, thus restricting its visibility to only where it's actually useful. This would require deeper changes though, since the matchState is the natural structure to transport parameters into and inside the parser. --- lib/compress/zstd_compress.c | 3 --- lib/compress/zstd_compress_internal.h | 4 ++-- lib/compress/zstd_lazy.c | 2 -- lib/compress/zstd_opt.c | 20 +++++++++----------- 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 0e522a711..43185eea1 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1290,7 +1290,6 @@ static void ZSTD_invalidateMatchState(ZSTD_matchState_t* ms) ZSTD_window_clear(&ms->window); ms->nextToUpdate = ms->window.dictLimit; - ms->nextToUpdate3 = ms->window.dictLimit; ms->loadedDictEnd = 0; ms->opt.litLengthSum = 0; /* force reset of btopt stats */ ms->dictMatchState = NULL; @@ -1679,7 +1678,6 @@ static size_t ZSTD_resetCCtx_byCopyingCDict(ZSTD_CCtx* cctx, ZSTD_matchState_t* dstMatchState = &cctx->blockState.matchState; dstMatchState->window = srcMatchState->window; dstMatchState->nextToUpdate = srcMatchState->nextToUpdate; - dstMatchState->nextToUpdate3= srcMatchState->nextToUpdate3; dstMatchState->loadedDictEnd= srcMatchState->loadedDictEnd; } @@ -1759,7 +1757,6 @@ static size_t ZSTD_copyCCtx_internal(ZSTD_CCtx* dstCCtx, ZSTD_matchState_t* dstMatchState = &dstCCtx->blockState.matchState; dstMatchState->window = srcMatchState->window; dstMatchState->nextToUpdate = srcMatchState->nextToUpdate; - dstMatchState->nextToUpdate3= srcMatchState->nextToUpdate3; dstMatchState->loadedDictEnd= srcMatchState->loadedDictEnd; } dstCCtx->dictID = srcCCtx->dictID; diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index bccaacac7..559ff7029 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -134,9 +134,9 @@ typedef struct { typedef struct ZSTD_matchState_t ZSTD_matchState_t; struct ZSTD_matchState_t { ZSTD_window_t window; /* State for window round buffer management */ - U32 loadedDictEnd; /* index of end of dictionary, within context's referential. When dict referential is copied into active context (i.e. not attached), effectively same value as dictSize, since referential starts at zero */ + U32 loadedDictEnd; /* index of end of dictionary, within context's referential. When dict referential is copied into active context (i.e. not attached), effectively same value as dictSize, since referential starts from zero */ U32 nextToUpdate; /* index from which to continue table update */ - U32 nextToUpdate3; /* index from which to continue table update of hashTable3 */ + U32 nextToUpdate3; /* index from which to continue table update of hashTable3. synchronized with nextToUpdate at block's start, so no need to maintain outside of parser. Used by opt parser only */ U32 hashLog3; /* dispatch table : larger == faster, more memory */ U32* hashTable; U32* hashTable3; diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 53f998a43..aa932b511 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -653,7 +653,6 @@ size_t ZSTD_compressBlock_lazy_generic( /* init */ ip += (dictAndPrefixLength == 0); - ms->nextToUpdate3 = ms->nextToUpdate; if (dictMode == ZSTD_noDict) { U32 const maxRep = (U32)(ip - prefixLowest); if (offset_2 > maxRep) savedOffset = offset_2, offset_2 = 0; @@ -933,7 +932,6 @@ size_t ZSTD_compressBlock_lazy_extDict_generic( U32 offset_1 = rep[0], offset_2 = rep[1]; /* init */ - ms->nextToUpdate3 = ms->nextToUpdate; ip += (ip == prefixStart); /* Match Loop */ diff --git a/lib/compress/zstd_opt.c b/lib/compress/zstd_opt.c index 3f2f2b7ba..faacc75a4 100644 --- a/lib/compress/zstd_opt.c +++ b/lib/compress/zstd_opt.c @@ -255,13 +255,13 @@ static U32 ZSTD_litLengthPrice(U32 const litLength, const optState_t* const optP * to provide a cost which is directly comparable to a match ending at same position */ static int ZSTD_litLengthContribution(U32 const litLength, const optState_t* const optPtr, int optLevel) { - if (optPtr->priceType >= zop_predef) return WEIGHT(litLength, optLevel); + if (optPtr->priceType >= zop_predef) return (int)WEIGHT(litLength, optLevel); /* dynamic statistics */ { U32 const llCode = ZSTD_LLcode(litLength); - int const contribution = (LL_bits[llCode] * BITCOST_MULTIPLIER) - + WEIGHT(optPtr->litLengthFreq[0], optLevel) /* note: log2litLengthSum cancel out */ - - WEIGHT(optPtr->litLengthFreq[llCode], optLevel); + int const contribution = (int)(LL_bits[llCode] * BITCOST_MULTIPLIER) + + (int)WEIGHT(optPtr->litLengthFreq[0], optLevel) /* note: log2litLengthSum cancel out */ + - (int)WEIGHT(optPtr->litLengthFreq[llCode], optLevel); #if 1 return contribution; #else @@ -278,7 +278,7 @@ static int ZSTD_literalsContribution(const BYTE* const literals, U32 const litLe const optState_t* const optPtr, int optLevel) { - int const contribution = ZSTD_rawLiteralsCost(literals, litLength, optPtr, optLevel) + int const contribution = (int)ZSTD_rawLiteralsCost(literals, litLength, optPtr, optLevel) + ZSTD_litLengthContribution(litLength, optPtr, optLevel); return contribution; } @@ -378,7 +378,7 @@ static U32 ZSTD_insertAndFindFirstIndexHash3 (ZSTD_matchState_t* ms, const BYTE* U32 const hashLog3 = ms->hashLog3; const BYTE* const base = ms->window.base; U32 idx = ms->nextToUpdate3; - U32 const target = ms->nextToUpdate3 = (U32)(ip - base); + U32 const target = (U32)(ip - base); size_t const hash3 = ZSTD_hash3Ptr(ip, hashLog3); assert(hashLog3 > 0); @@ -387,6 +387,7 @@ static U32 ZSTD_insertAndFindFirstIndexHash3 (ZSTD_matchState_t* ms, const BYTE* idx++; } + ms->nextToUpdate3 = target; return hashTable3[hash3]; } @@ -653,9 +654,7 @@ U32 ZSTD_insertBtAndGetAllMatches ( (ip+mlen == iLimit) ) { /* best possible length */ ms->nextToUpdate = current+1; /* skip insertion */ return 1; - } - } - } + } } } /* no dictMatchState lookup: dicts don't have a populated HC3 table */ } @@ -862,7 +861,7 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms, DEBUGLOG(5, "ZSTD_compressBlock_opt_generic: current=%u, prefix=%u, nextToUpdate=%u", (U32)(ip - base), ms->window.dictLimit, ms->nextToUpdate); assert(optLevel <= 2); - ms->nextToUpdate3 = ms->nextToUpdate; /* note : why a separate nextToUpdate3 stored into cctx->ms if it's synchtonized from nextToUpdate anyway ? */ + ms->nextToUpdate3 = ms->nextToUpdate; /* note : nextToUpdate3 always synchronized with nextToUpdate at beginning of block */ ZSTD_rescaleFreqs(optStatePtr, (const BYTE*)src, srcSize, optLevel); ip += (ip==prefixStart); @@ -1158,7 +1157,6 @@ ZSTD_initStats_ultra(ZSTD_matchState_t* ms, ms->window.dictLimit += (U32)srcSize; ms->window.lowLimit = ms->window.dictLimit; ms->nextToUpdate = ms->window.dictLimit; - ms->nextToUpdate3 = ms->window.dictLimit; /* re-inforce weight of collected statistics */ ZSTD_upscaleStats(&ms->opt); From 33dabc8c80d637dfed25ce5037d6373e47356ca6 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 28 May 2019 16:11:32 -0700 Subject: [PATCH 2/3] get bt matches : made it a bit clearer which parameters are input and output --- lib/compress/zstd_opt.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/compress/zstd_opt.c b/lib/compress/zstd_opt.c index faacc75a4..aaacf55ae 100644 --- a/lib/compress/zstd_opt.c +++ b/lib/compress/zstd_opt.c @@ -532,11 +532,11 @@ void ZSTD_updateTree(ZSTD_matchState_t* ms, const BYTE* ip, const BYTE* iend) { FORCE_INLINE_TEMPLATE U32 ZSTD_insertBtAndGetAllMatches ( + ZSTD_match_t* matches, /* store result (found matches) in this table (presumed large enough) */ ZSTD_matchState_t* ms, const BYTE* const ip, const BYTE* const iLimit, const ZSTD_dictMode_e dictMode, - U32 rep[ZSTD_REP_NUM], + const U32 rep[ZSTD_REP_NUM], U32 const ll0, /* tells if associated literal length is 0 or not. This value must be 0 or 1 */ - ZSTD_match_t* matches, const U32 lengthToBeat, U32 const mls /* template */) { @@ -759,10 +759,12 @@ U32 ZSTD_insertBtAndGetAllMatches ( FORCE_INLINE_TEMPLATE U32 ZSTD_BtGetAllMatches ( + ZSTD_match_t* matches, /* store result (match found, increasing size) in this table */ ZSTD_matchState_t* ms, const BYTE* ip, const BYTE* const iHighLimit, const ZSTD_dictMode_e dictMode, - U32 rep[ZSTD_REP_NUM], U32 const ll0, - ZSTD_match_t* matches, U32 const lengthToBeat) + const U32 rep[ZSTD_REP_NUM], + U32 const ll0, + U32 const lengthToBeat) { const ZSTD_compressionParameters* const cParams = &ms->cParams; U32 const matchLengthSearch = cParams->minMatch; @@ -771,12 +773,12 @@ FORCE_INLINE_TEMPLATE U32 ZSTD_BtGetAllMatches ( ZSTD_updateTree_internal(ms, ip, iHighLimit, matchLengthSearch, dictMode); switch(matchLengthSearch) { - case 3 : return ZSTD_insertBtAndGetAllMatches(ms, ip, iHighLimit, dictMode, rep, ll0, matches, lengthToBeat, 3); + case 3 : return ZSTD_insertBtAndGetAllMatches(matches, ms, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 3); default : - case 4 : return ZSTD_insertBtAndGetAllMatches(ms, ip, iHighLimit, dictMode, rep, ll0, matches, lengthToBeat, 4); - case 5 : return ZSTD_insertBtAndGetAllMatches(ms, ip, iHighLimit, dictMode, rep, ll0, matches, lengthToBeat, 5); + case 4 : return ZSTD_insertBtAndGetAllMatches(matches, ms, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 4); + case 5 : return ZSTD_insertBtAndGetAllMatches(matches, ms, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 5); case 7 : - case 6 : return ZSTD_insertBtAndGetAllMatches(ms, ip, iHighLimit, dictMode, rep, ll0, matches, lengthToBeat, 6); + case 6 : return ZSTD_insertBtAndGetAllMatches(matches, ms, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 6); } } @@ -872,7 +874,7 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms, /* find first match */ { U32 const litlen = (U32)(ip - anchor); U32 const ll0 = !litlen; - U32 const nbMatches = ZSTD_BtGetAllMatches(ms, ip, iend, dictMode, rep, ll0, matches, minMatch); + U32 const nbMatches = ZSTD_BtGetAllMatches(matches, ms, ip, iend, dictMode, rep, ll0, minMatch); if (!nbMatches) { ip++; continue; } /* initialize opt[0] */ @@ -969,7 +971,7 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms, U32 const litlen = (opt[cur].mlen == 0) ? opt[cur].litlen : 0; U32 const previousPrice = opt[cur].price; U32 const basePrice = previousPrice + ZSTD_litLengthPrice(0, optStatePtr, optLevel); - U32 const nbMatches = ZSTD_BtGetAllMatches(ms, inr, iend, dictMode, opt[cur].rep, ll0, matches, minMatch); + U32 const nbMatches = ZSTD_BtGetAllMatches(matches, ms, inr, iend, dictMode, opt[cur].rep, ll0, minMatch); U32 matchNb; if (!nbMatches) { DEBUGLOG(7, "rPos:%u : no match found", cur); @@ -1093,7 +1095,7 @@ _shortestPath: /* cur, last_pos, best_mlen, best_off have to be set */ } /* while (ip < ilimit) */ /* Return the last literals size */ - return iend - anchor; + return (size_t)(iend - anchor); } From 9719fd616cb96d7a5120190b1a751d2ca421db36 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 28 May 2019 16:18:12 -0700 Subject: [PATCH 3/3] removed nextToUpdate3 from ZSTD_window it's now a local variable of ZSTD_compressBlock_opt() --- lib/compress/zstd_compress_internal.h | 1 - lib/compress/zstd_opt.c | 26 +++++++++++++++----------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 559ff7029..ee9d9e31a 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -136,7 +136,6 @@ struct ZSTD_matchState_t { ZSTD_window_t window; /* State for window round buffer management */ U32 loadedDictEnd; /* index of end of dictionary, within context's referential. When dict referential is copied into active context (i.e. not attached), effectively same value as dictSize, since referential starts from zero */ U32 nextToUpdate; /* index from which to continue table update */ - U32 nextToUpdate3; /* index from which to continue table update of hashTable3. synchronized with nextToUpdate at block's start, so no need to maintain outside of parser. Used by opt parser only */ U32 hashLog3; /* dispatch table : larger == faster, more memory */ U32* hashTable; U32* hashTable3; diff --git a/lib/compress/zstd_opt.c b/lib/compress/zstd_opt.c index aaacf55ae..12b56c70d 100644 --- a/lib/compress/zstd_opt.c +++ b/lib/compress/zstd_opt.c @@ -372,12 +372,14 @@ MEM_STATIC U32 ZSTD_readMINMATCH(const void* memPtr, U32 length) /* Update hashTable3 up to ip (excluded) Assumption : always within prefix (i.e. not within extDict) */ -static U32 ZSTD_insertAndFindFirstIndexHash3 (ZSTD_matchState_t* ms, const BYTE* const ip) +static U32 ZSTD_insertAndFindFirstIndexHash3 (ZSTD_matchState_t* ms, + U32* nextToUpdate3, + const BYTE* const ip) { U32* const hashTable3 = ms->hashTable3; U32 const hashLog3 = ms->hashLog3; const BYTE* const base = ms->window.base; - U32 idx = ms->nextToUpdate3; + U32 idx = *nextToUpdate3; U32 const target = (U32)(ip - base); size_t const hash3 = ZSTD_hash3Ptr(ip, hashLog3); assert(hashLog3 > 0); @@ -387,7 +389,7 @@ static U32 ZSTD_insertAndFindFirstIndexHash3 (ZSTD_matchState_t* ms, const BYTE* idx++; } - ms->nextToUpdate3 = target; + *nextToUpdate3 = target; return hashTable3[hash3]; } @@ -534,6 +536,7 @@ FORCE_INLINE_TEMPLATE U32 ZSTD_insertBtAndGetAllMatches ( ZSTD_match_t* matches, /* store result (found matches) in this table (presumed large enough) */ ZSTD_matchState_t* ms, + U32* nextToUpdate3, const BYTE* const ip, const BYTE* const iLimit, const ZSTD_dictMode_e dictMode, const U32 rep[ZSTD_REP_NUM], U32 const ll0, /* tells if associated literal length is 0 or not. This value must be 0 or 1 */ @@ -628,7 +631,7 @@ U32 ZSTD_insertBtAndGetAllMatches ( /* HC3 match finder */ if ((mls == 3) /*static*/ && (bestLength < mls)) { - U32 const matchIndex3 = ZSTD_insertAndFindFirstIndexHash3(ms, ip); + U32 const matchIndex3 = ZSTD_insertAndFindFirstIndexHash3(ms, nextToUpdate3, ip); if ((matchIndex3 >= matchLow) & (current - matchIndex3 < (1<<18)) /*heuristic : longer distance likely too expensive*/ ) { size_t mlen; @@ -761,6 +764,7 @@ U32 ZSTD_insertBtAndGetAllMatches ( FORCE_INLINE_TEMPLATE U32 ZSTD_BtGetAllMatches ( ZSTD_match_t* matches, /* store result (match found, increasing size) in this table */ ZSTD_matchState_t* ms, + U32* nextToUpdate3, const BYTE* ip, const BYTE* const iHighLimit, const ZSTD_dictMode_e dictMode, const U32 rep[ZSTD_REP_NUM], U32 const ll0, @@ -773,12 +777,12 @@ FORCE_INLINE_TEMPLATE U32 ZSTD_BtGetAllMatches ( ZSTD_updateTree_internal(ms, ip, iHighLimit, matchLengthSearch, dictMode); switch(matchLengthSearch) { - case 3 : return ZSTD_insertBtAndGetAllMatches(matches, ms, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 3); + case 3 : return ZSTD_insertBtAndGetAllMatches(matches, ms, nextToUpdate3, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 3); default : - case 4 : return ZSTD_insertBtAndGetAllMatches(matches, ms, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 4); - case 5 : return ZSTD_insertBtAndGetAllMatches(matches, ms, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 5); + case 4 : return ZSTD_insertBtAndGetAllMatches(matches, ms, nextToUpdate3, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 4); + case 5 : return ZSTD_insertBtAndGetAllMatches(matches, ms, nextToUpdate3, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 5); case 7 : - case 6 : return ZSTD_insertBtAndGetAllMatches(matches, ms, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 6); + case 6 : return ZSTD_insertBtAndGetAllMatches(matches, ms, nextToUpdate3, ip, iHighLimit, dictMode, rep, ll0, lengthToBeat, 6); } } @@ -854,6 +858,7 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms, U32 const sufficient_len = MIN(cParams->targetLength, ZSTD_OPT_NUM -1); U32 const minMatch = (cParams->minMatch == 3) ? 3 : 4; + U32 nextToUpdate3 = ms->nextToUpdate; ZSTD_optimal_t* const opt = optStatePtr->priceTable; ZSTD_match_t* const matches = optStatePtr->matchTable; @@ -863,7 +868,6 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms, DEBUGLOG(5, "ZSTD_compressBlock_opt_generic: current=%u, prefix=%u, nextToUpdate=%u", (U32)(ip - base), ms->window.dictLimit, ms->nextToUpdate); assert(optLevel <= 2); - ms->nextToUpdate3 = ms->nextToUpdate; /* note : nextToUpdate3 always synchronized with nextToUpdate at beginning of block */ ZSTD_rescaleFreqs(optStatePtr, (const BYTE*)src, srcSize, optLevel); ip += (ip==prefixStart); @@ -874,7 +878,7 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms, /* find first match */ { U32 const litlen = (U32)(ip - anchor); U32 const ll0 = !litlen; - U32 const nbMatches = ZSTD_BtGetAllMatches(matches, ms, ip, iend, dictMode, rep, ll0, minMatch); + U32 const nbMatches = ZSTD_BtGetAllMatches(matches, ms, &nextToUpdate3, ip, iend, dictMode, rep, ll0, minMatch); if (!nbMatches) { ip++; continue; } /* initialize opt[0] */ @@ -971,7 +975,7 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms, U32 const litlen = (opt[cur].mlen == 0) ? opt[cur].litlen : 0; U32 const previousPrice = opt[cur].price; U32 const basePrice = previousPrice + ZSTD_litLengthPrice(0, optStatePtr, optLevel); - U32 const nbMatches = ZSTD_BtGetAllMatches(matches, ms, inr, iend, dictMode, opt[cur].rep, ll0, minMatch); + U32 const nbMatches = ZSTD_BtGetAllMatches(matches, ms, &nextToUpdate3, inr, iend, dictMode, opt[cur].rep, ll0, minMatch); U32 matchNb; if (!nbMatches) { DEBUGLOG(7, "rPos:%u : no match found", cur);