From 761f40d1c60f2ed07638d52f2be4c6ebebc884bc Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 27 Oct 2020 09:41:32 -0400 Subject: [PATCH 01/13] Clarify and modify ZSTD_Sequence definition --- lib/zstd.h | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/zstd.h b/lib/zstd.h index 75d5bb237..c91245663 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1114,21 +1114,24 @@ ZSTDLIB_API size_t ZSTD_sizeof_DDict(const ZSTD_DDict* ddict); typedef struct ZSTD_CCtx_params_s ZSTD_CCtx_params; typedef struct { - unsigned int matchPos; /* Match pos in dst */ - /* If seqDef.offset > 3, then this is seqDef.offset - 3 - * If seqDef.offset < 3, then this is the corresponding repeat offset - * But if seqDef.offset < 3 and litLength == 0, this is the - * repeat offset before the corresponding repeat offset - * And if seqDef.offset == 3 and litLength == 0, this is the - * most recent repeat offset - 1 - */ - unsigned int offset; - unsigned int litLength; /* Literal length */ - unsigned int matchLength; /* Match length */ - /* 0 when seq not rep and seqDef.offset otherwise - * when litLength == 0 this will be <= 4, otherwise <= 3 like normal - */ - unsigned int rep; + unsigned int offset; /* The offset of the match. + * If == 0, then represents a block of literals, determined by litLength + */ + + unsigned int litLength; /* Literal length */ + unsigned int matchLength; /* Match length. */ + unsigned int rep; /* Represents which repeat offset is used. Ranges from [0, 3]. + * If rep == 0, then this sequence does not contain a repeat offset. + * Otherwise: + * If litLength != 0: + * rep == 1 --> offset == repeat offset 1 + * rep == 2 --> offset == repeat offset 2 + * rep == 3 --> offset == repeat offset 3 + * If litLength == 0: + * rep == 1 --> offset == repeat offset 2 + * rep == 2 --> offset == repeat offset 3 + * rep == 3 --> offset == repeat offset 1 - 1 + */ } ZSTD_Sequence; typedef struct { From 3a11c7eb035ff949175d99376284b49c8d6ea01a Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 27 Oct 2020 10:07:26 -0400 Subject: [PATCH 02/13] Modify ZSTD_copyBlockSequences to agree with new API --- lib/compress/zstd_compress.c | 48 ++++++++++++++++++++---------------- lib/zstd.h | 31 ++++++++++++----------- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index b1bb9fa10..119b90941 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2442,17 +2442,19 @@ static size_t ZSTD_buildSeqStore(ZSTD_CCtx* zc, const void* src, size_t srcSize) static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) { const seqStore_t* seqStore = ZSTD_getSeqStore(zc); - const seqDef* seqs = seqStore->sequencesStart; - size_t seqsSize = seqStore->sequences - seqs; + const seqDef* seqStoreSeqs = seqStore->sequencesStart; + size_t seqStoreSeqSize = seqStore->sequences - seqStoreSeqs; ZSTD_Sequence* outSeqs = &zc->seqCollector.seqStart[zc->seqCollector.seqIndex]; - size_t i; size_t position; int repIdx; + size_t i; + size_t position; + int repIdx; assert(zc->seqCollector.seqIndex + 1 < zc->seqCollector.maxSequences); - for (i = 0, position = 0; i < seqsSize; ++i) { - outSeqs[i].offset = seqs[i].offset; - outSeqs[i].litLength = seqs[i].litLength; - outSeqs[i].matchLength = seqs[i].matchLength + MINMATCH; + for (i = 0, position = 0; i < seqStoreSeqSize; ++i) { + outSeqs[i].offset = seqStoreSeqs[i].offset - ZSTD_REP_NUM; + outSeqs[i].litLength = seqStoreSeqs[i].litLength; + outSeqs[i].matchLength = seqStoreSeqs[i].matchLength + MINMATCH; if (i == seqStore->longLengthPos) { if (seqStore->longLengthID == 1) { @@ -2462,32 +2464,36 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) } } - if (outSeqs[i].offset <= ZSTD_REP_NUM) { - outSeqs[i].rep = outSeqs[i].offset; - repIdx = (unsigned int)i - outSeqs[i].offset; + /* Repcode handling: + * If litLength != 0: + * rep == 1 --> offset == repeat offset 1 + * rep == 2 --> offset == repeat offset 2 + * rep == 3 --> offset == repeat offset 3 + * If litLength == 0: + * rep == 1 --> offset == repeat offset 2 + * rep == 2 --> offset == repeat offset 3 + * rep == 3 --> offset == repeat offset 1 - 1 + */ + if (seqStoreSeqs[i].offset <= ZSTD_REP_NUM) { + outSeqs[i].rep = seqStoreSeqs[i].offset; + repIdx = (unsigned int)i - seqStoreSeqs[i].offset; - if (outSeqs[i].litLength == 0) { - if (outSeqs[i].offset < 3) { + if (seqStoreSeqs[i].litLength == 0) { + if (seqStoreSeqs[i].offset < 3) { --repIdx; } else { repIdx = (unsigned int)i - 1; } - ++outSeqs[i].rep; } assert(repIdx >= -3); outSeqs[i].offset = repIdx >= 0 ? outSeqs[repIdx].offset : repStartValue[-repIdx - 1]; - if (outSeqs[i].rep == 4) { + if (outSeqs[i].rep == 3 && outSeqs[i].litLength == 0) { --outSeqs[i].offset; } - } else { - outSeqs[i].offset -= ZSTD_REP_NUM; } - - position += outSeqs[i].litLength; - outSeqs[i].matchPos = (unsigned int)position; - position += outSeqs[i].matchLength; + position += outSeqs[i].litLength + outSeqs[i].matchLength; } - zc->seqCollector.seqIndex += seqsSize; + zc->seqCollector.seqIndex += seqStoreSeqSize; } size_t ZSTD_getSequences(ZSTD_CCtx* zc, ZSTD_Sequence* outSeqs, diff --git a/lib/zstd.h b/lib/zstd.h index c91245663..7668729a1 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1114,24 +1114,25 @@ ZSTDLIB_API size_t ZSTD_sizeof_DDict(const ZSTD_DDict* ddict); typedef struct ZSTD_CCtx_params_s ZSTD_CCtx_params; typedef struct { - unsigned int offset; /* The offset of the match. - * If == 0, then represents a block of literals, determined by litLength - */ + unsigned int offset; /* The offset of the match. + * If == 0, then represents a block of literals, determined by litLength + */ unsigned int litLength; /* Literal length */ unsigned int matchLength; /* Match length. */ - unsigned int rep; /* Represents which repeat offset is used. Ranges from [0, 3]. - * If rep == 0, then this sequence does not contain a repeat offset. - * Otherwise: - * If litLength != 0: - * rep == 1 --> offset == repeat offset 1 - * rep == 2 --> offset == repeat offset 2 - * rep == 3 --> offset == repeat offset 3 - * If litLength == 0: - * rep == 1 --> offset == repeat offset 2 - * rep == 2 --> offset == repeat offset 3 - * rep == 3 --> offset == repeat offset 1 - 1 - */ + + unsigned int rep; /* Represents which repeat offset is used. Ranges from [0, 3]. + * If rep == 0, then this sequence does not contain a repeat offset. + * Otherwise: + * If litLength != 0: + * rep == 1 --> offset == repeat offset 1 + * rep == 2 --> offset == repeat offset 2 + * rep == 3 --> offset == repeat offset 3 + * If litLength == 0: + * rep == 1 --> offset == repeat offset 2 + * rep == 2 --> offset == repeat offset 3 + * rep == 3 --> offset == repeat offset 1 - 1 + */ } ZSTD_Sequence; typedef struct { From 96b0ff78865c3952827e573268103ff93e01560e Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 27 Oct 2020 10:36:06 -0400 Subject: [PATCH 03/13] Improve documentation regarding various operations in copyBlockSequences --- lib/compress/zstd_compress.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 119b90941..b3ada04f4 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2452,10 +2452,13 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) assert(zc->seqCollector.seqIndex + 1 < zc->seqCollector.maxSequences); for (i = 0, position = 0; i < seqStoreSeqSize; ++i) { - outSeqs[i].offset = seqStoreSeqs[i].offset - ZSTD_REP_NUM; outSeqs[i].litLength = seqStoreSeqs[i].litLength; outSeqs[i].matchLength = seqStoreSeqs[i].matchLength + MINMATCH; + /* matchLength and litLength are stored with U16. longLengthPos + * and longLengthID to allow us to represent a single litLength or matchLength + * in the seqStore that has a value larger than U16 (if it exists). + */ if (i == seqStore->longLengthPos) { if (seqStore->longLengthID == 1) { outSeqs[i].litLength += 0x10000; @@ -2465,14 +2468,15 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) } /* Repcode handling: + * See docs/format.md for more detail about repeat offset codes * If litLength != 0: - * rep == 1 --> offset == repeat offset 1 - * rep == 2 --> offset == repeat offset 2 - * rep == 3 --> offset == repeat offset 3 + * rep == 1 --> offset == repeat_offset_1 + * rep == 2 --> offset == repeat_offset_2 + * rep == 3 --> offset == repeat_offset_3 * If litLength == 0: - * rep == 1 --> offset == repeat offset 2 - * rep == 2 --> offset == repeat offset 3 - * rep == 3 --> offset == repeat offset 1 - 1 + * rep == 1 --> offset == repeat_offset_2 + * rep == 2 --> offset == repeat_offset_3 + * rep == 3 --> offset == repeat_offset_1 - 1 */ if (seqStoreSeqs[i].offset <= ZSTD_REP_NUM) { outSeqs[i].rep = seqStoreSeqs[i].offset; @@ -2486,10 +2490,15 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) } } assert(repIdx >= -3); + /* Use default repcodes if repcode references an offset that doesn't exist yet + * This can only occur within the first two sequences. + */ outSeqs[i].offset = repIdx >= 0 ? outSeqs[repIdx].offset : repStartValue[-repIdx - 1]; if (outSeqs[i].rep == 3 && outSeqs[i].litLength == 0) { --outSeqs[i].offset; } + } else { + outSeqs[i].offset = seqStoreSeqs[i].offset - ZSTD_REP_NUM; } position += outSeqs[i].litLength + outSeqs[i].matchLength; } From 9171f920cd790cc3f7b3ecc817efe88af7b063c7 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 27 Oct 2020 10:43:37 -0400 Subject: [PATCH 04/13] Improve documentation of seqStore_t --- lib/common/zstd_internal.h | 15 ++++++++++----- lib/compress/zstd_compress.c | 4 ---- lib/zstd.h | 26 +++++++++++++------------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index bfa98efaa..f9972766d 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -341,23 +341,28 @@ MEM_STATIC size_t ZSTD_limitCopy(void* dst, size_t dstCapacity, const void* src, * Private declarations *********************************************/ typedef struct seqDef_s { - U32 offset; + U32 offset; /* Offset code of the sequence */ U16 litLength; U16 matchLength; } seqDef; typedef struct { seqDef* sequencesStart; - seqDef* sequences; + seqDef* sequences; /* ptr to end of sequences */ BYTE* litStart; - BYTE* lit; + BYTE* lit; /* ptr to end of literals */ BYTE* llCode; BYTE* mlCode; BYTE* ofCode; size_t maxNbSeq; size_t maxNbLit; - U32 longLengthID; /* 0 == no longLength; 1 == Lit.longLength; 2 == Match.longLength; */ - U32 longLengthPos; + + /* longLengthPos and longLengthID to allow us to represent either a single litLength or matchLength + * in the seqStore that has a value larger than U16 (if it exists). To do so, we increment + * the existing value of the literal or match by 0x10000. + */ + U32 longLengthID; /* 0 == no longLength; 1 == Represent the long literal; 2 == Represent the long match; */ + U32 longLengthPos; /* Index of the sequence to apply long length modification to */ } seqStore_t; typedef struct { diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index b3ada04f4..9d5f20a61 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2455,10 +2455,6 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) outSeqs[i].litLength = seqStoreSeqs[i].litLength; outSeqs[i].matchLength = seqStoreSeqs[i].matchLength + MINMATCH; - /* matchLength and litLength are stored with U16. longLengthPos - * and longLengthID to allow us to represent a single litLength or matchLength - * in the seqStore that has a value larger than U16 (if it exists). - */ if (i == seqStore->longLengthPos) { if (seqStore->longLengthID == 1) { outSeqs[i].litLength += 0x10000; diff --git a/lib/zstd.h b/lib/zstd.h index 7668729a1..1c61bd001 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1115,24 +1115,24 @@ typedef struct ZSTD_CCtx_params_s ZSTD_CCtx_params; typedef struct { unsigned int offset; /* The offset of the match. - * If == 0, then represents a block of literals, determined by litLength - */ + * If == 0, then represents a block of literals, determined by litLength + */ unsigned int litLength; /* Literal length */ unsigned int matchLength; /* Match length. */ unsigned int rep; /* Represents which repeat offset is used. Ranges from [0, 3]. - * If rep == 0, then this sequence does not contain a repeat offset. - * Otherwise: - * If litLength != 0: - * rep == 1 --> offset == repeat offset 1 - * rep == 2 --> offset == repeat offset 2 - * rep == 3 --> offset == repeat offset 3 - * If litLength == 0: - * rep == 1 --> offset == repeat offset 2 - * rep == 2 --> offset == repeat offset 3 - * rep == 3 --> offset == repeat offset 1 - 1 - */ + * If rep == 0, then this sequence does not contain a repeat offset. + * Otherwise: + * If litLength != 0: + * rep == 1 --> offset == repeat offset 1 + * rep == 2 --> offset == repeat offset 2 + * rep == 3 --> offset == repeat offset 3 + * If litLength == 0: + * rep == 1 --> offset == repeat offset 2 + * rep == 2 --> offset == repeat offset 3 + * rep == 3 --> offset == repeat offset 1 - 1 + */ } ZSTD_Sequence; typedef struct { From 1d221ecc038b05a106b83b6cfc221f329d6680a9 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 27 Oct 2020 11:19:48 -0400 Subject: [PATCH 05/13] Add support for representing last literals in the extracted seqs --- lib/compress/zstd_compress.c | 17 +++++++++++++++-- lib/zstd.h | 5 +++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 9d5f20a61..00daa4c70 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2444,6 +2444,9 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) const seqStore_t* seqStore = ZSTD_getSeqStore(zc); const seqDef* seqStoreSeqs = seqStore->sequencesStart; size_t seqStoreSeqSize = seqStore->sequences - seqStoreSeqs; + size_t seqStoreLiteralsSize = (size_t)(seqStore->lit - seqStore->litStart); + size_t literalsRead = 0; + size_t lastLLSize; ZSTD_Sequence* outSeqs = &zc->seqCollector.seqStart[zc->seqCollector.seqIndex]; size_t i; @@ -2451,7 +2454,10 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) int repIdx; assert(zc->seqCollector.seqIndex + 1 < zc->seqCollector.maxSequences); - for (i = 0, position = 0; i < seqStoreSeqSize; ++i) { + /* Ensure we have enough space for last literals "sequence" */ + assert(zc->seqCollector.maxSequences >= seqStoreSeqSize + 1); + for (i = 0; i < seqStoreSeqSize; ++i) { + literalsRead += seqStoreSeqs[i].litLength; outSeqs[i].litLength = seqStoreSeqs[i].litLength; outSeqs[i].matchLength = seqStoreSeqs[i].matchLength + MINMATCH; @@ -2496,8 +2502,15 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) } else { outSeqs[i].offset = seqStoreSeqs[i].offset - ZSTD_REP_NUM; } - position += outSeqs[i].litLength + outSeqs[i].matchLength; } + + /* Insert last literals (if any exist) in the block as a sequence with ml == off == 0 */ + lastLLSize = seqStoreLiteralsSize - literalsRead; + if (lastLLSize) { + outSeqs[i].litLength = lastLLSize; + outSeqs[i].matchLength = outSeqs[i].offset = outSeqs[i].rep = 0; + } + zc->seqCollector.seqIndex += seqStoreSeqSize; } diff --git a/lib/zstd.h b/lib/zstd.h index 1c61bd001..d2f20be2d 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1115,7 +1115,7 @@ typedef struct ZSTD_CCtx_params_s ZSTD_CCtx_params; typedef struct { unsigned int offset; /* The offset of the match. - * If == 0, then represents a block of literals, determined by litLength + * If == 0, then represents a section of literals of litLength size */ unsigned int litLength; /* Literal length */ @@ -1278,7 +1278,8 @@ ZSTDLIB_API unsigned long long ZSTD_decompressBound(const void* src, size_t srcS ZSTDLIB_API size_t ZSTD_frameHeaderSize(const void* src, size_t srcSize); /*! ZSTD_getSequences() : - * Extract sequences from the sequence store + * Extract sequences from the sequence store. Any last literals in the block will be represented as a sequence + * with offset == 0, matchLength == 0, litLength == last literals size. * zc can be used to insert custom compression params. * This function invokes ZSTD_compress2 * @return : number of sequences extracted From dc448563e9d1ac768fb82436d0b3a3336e597572 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 27 Oct 2020 12:28:46 -0400 Subject: [PATCH 06/13] Add test compatibility with last literals in sequences --- lib/compress/zstd_compress.c | 4 ++-- lib/zstd.h | 25 +++++++++++++++---------- tests/fuzzer.c | 20 +++++++++++--------- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 00daa4c70..4091662a9 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2506,11 +2506,11 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) /* Insert last literals (if any exist) in the block as a sequence with ml == off == 0 */ lastLLSize = seqStoreLiteralsSize - literalsRead; - if (lastLLSize) { + if (lastLLSize > 0) { outSeqs[i].litLength = lastLLSize; outSeqs[i].matchLength = outSeqs[i].offset = outSeqs[i].rep = 0; + seqStoreSeqSize++; } - zc->seqCollector.seqIndex += seqStoreSeqSize; } diff --git a/lib/zstd.h b/lib/zstd.h index d2f20be2d..0c243e017 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1115,23 +1115,28 @@ typedef struct ZSTD_CCtx_params_s ZSTD_CCtx_params; typedef struct { unsigned int offset; /* The offset of the match. - * If == 0, then represents a section of literals of litLength size + * If offset == 0 and matchLength == 0, + * then this sequence represents last literals in the block of litLength size. */ - unsigned int litLength; /* Literal length */ - unsigned int matchLength; /* Match length. */ + unsigned int litLength; /* Literal length of the sequence. */ + unsigned int matchLength; /* Match length of the sequence. */ + + /* Note: Users of this API may provide a sequence with matchLength == litLength == offset == 0. + * In this case, we will treat the "sequence" as a marker for a block boundary. + */ unsigned int rep; /* Represents which repeat offset is used. Ranges from [0, 3]. * If rep == 0, then this sequence does not contain a repeat offset. - * Otherwise: + * If rep > 0: * If litLength != 0: - * rep == 1 --> offset == repeat offset 1 - * rep == 2 --> offset == repeat offset 2 - * rep == 3 --> offset == repeat offset 3 + * rep == 1 --> offset == repeat_offset_1 + * rep == 2 --> offset == repeat_offset_2 + * rep == 3 --> offset == repeat_offset_3 * If litLength == 0: - * rep == 1 --> offset == repeat offset 2 - * rep == 2 --> offset == repeat offset 3 - * rep == 3 --> offset == repeat offset 1 - 1 + * rep == 1 --> offset == repeat_offset_2 + * rep == 2 --> offset == repeat_offset_3 + * rep == 3 --> offset == repeat_offset_1 - 1 */ } ZSTD_Sequence; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index e54714fdf..038fae1aa 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -310,22 +310,23 @@ static void FUZ_decodeSequences(BYTE* dst, ZSTD_Sequence* seqs, size_t seqsSize, { size_t i; size_t j; - for(i = 0; i < seqsSize - 1; ++i) { - assert(dst + seqs[i].litLength + seqs[i].matchLength < dst + size); - assert(src + seqs[i].litLength + seqs[i].matchLength < src + size); + for(i = 0; i < seqsSize; ++i) { + assert(dst + seqs[i].litLength + seqs[i].matchLength <= dst + size); + assert(src + seqs[i].litLength + seqs[i].matchLength <= src + size); memcpy(dst, src, seqs[i].litLength); dst += seqs[i].litLength; src += seqs[i].litLength; size -= seqs[i].litLength; - for (j = 0; j < seqs[i].matchLength; ++j) - dst[j] = dst[j - seqs[i].offset]; - dst += seqs[i].matchLength; - src += seqs[i].matchLength; - size -= seqs[i].matchLength; + if (seqs[i].offset != 0) { + for (j = 0; j < seqs[i].matchLength; ++j) + dst[j] = dst[j - seqs[i].offset]; + dst += seqs[i].matchLength; + src += seqs[i].matchLength; + size -= seqs[i].matchLength; + } } - memcpy(dst, src, size); } /*============================================= @@ -2666,6 +2667,7 @@ static int basicUnitTests(U32 const seed, double compressibility) ZSTD_freeCCtx(cctx); free(seqs); } + DISPLAYLEVEL(3, "OK \n"); /* Multiple blocks of zeros test */ #define LONGZEROSLENGTH 1000000 /* 1MB of zeros */ From 3163909d14d2901179562faa03e2e90d7e19a71d Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 27 Oct 2020 12:58:12 -0400 Subject: [PATCH 07/13] Remove unused variable position --- lib/compress/zstd_compress.c | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 4091662a9..e2a80accd 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2450,7 +2450,6 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) ZSTD_Sequence* outSeqs = &zc->seqCollector.seqStart[zc->seqCollector.seqIndex]; size_t i; - size_t position; int repIdx; assert(zc->seqCollector.seqIndex + 1 < zc->seqCollector.maxSequences); @@ -2469,17 +2468,6 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) } } - /* Repcode handling: - * See docs/format.md for more detail about repeat offset codes - * If litLength != 0: - * rep == 1 --> offset == repeat_offset_1 - * rep == 2 --> offset == repeat_offset_2 - * rep == 3 --> offset == repeat_offset_3 - * If litLength == 0: - * rep == 1 --> offset == repeat_offset_2 - * rep == 2 --> offset == repeat_offset_3 - * rep == 3 --> offset == repeat_offset_1 - 1 - */ if (seqStoreSeqs[i].offset <= ZSTD_REP_NUM) { outSeqs[i].rep = seqStoreSeqs[i].offset; repIdx = (unsigned int)i - seqStoreSeqs[i].offset; From 3ed5d053d8ed13f5eeb569c01771ea990ead07c8 Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Tue, 27 Oct 2020 17:45:23 -0400 Subject: [PATCH 08/13] Clarify comments in zstd.h some more --- lib/common/zstd_internal.h | 2 +- lib/compress/zstd_compress.c | 4 +--- lib/zstd.h | 21 +++++++++++++++------ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index f9972766d..48a2a7b53 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -359,7 +359,7 @@ typedef struct { /* longLengthPos and longLengthID to allow us to represent either a single litLength or matchLength * in the seqStore that has a value larger than U16 (if it exists). To do so, we increment - * the existing value of the literal or match by 0x10000. + * the existing value of the litLength or matchLength by 0x10000. */ U32 longLengthID; /* 0 == no longLength; 1 == Represent the long literal; 2 == Represent the long match; */ U32 longLengthPos; /* Index of the sequence to apply long length modification to */ diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index e2a80accd..5cd243a69 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2480,9 +2480,6 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) } } assert(repIdx >= -3); - /* Use default repcodes if repcode references an offset that doesn't exist yet - * This can only occur within the first two sequences. - */ outSeqs[i].offset = repIdx >= 0 ? outSeqs[repIdx].offset : repStartValue[-repIdx - 1]; if (outSeqs[i].rep == 3 && outSeqs[i].litLength == 0) { --outSeqs[i].offset; @@ -2493,6 +2490,7 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) } /* Insert last literals (if any exist) in the block as a sequence with ml == off == 0 */ + assert(seqStoreLiteralsSize >= literalsRead); lastLLSize = seqStoreLiteralsSize - literalsRead; if (lastLLSize > 0) { outSeqs[i].litLength = lastLLSize; diff --git a/lib/zstd.h b/lib/zstd.h index 0c243e017..b57e1de0f 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1114,20 +1114,25 @@ ZSTDLIB_API size_t ZSTD_sizeof_DDict(const ZSTD_DDict* ddict); typedef struct ZSTD_CCtx_params_s ZSTD_CCtx_params; typedef struct { - unsigned int offset; /* The offset of the match. - * If offset == 0 and matchLength == 0, - * then this sequence represents last literals in the block of litLength size. + unsigned int offset; /* The offset of the match. (NOT the same as the offset code) + * If offset == 0 and matchLength == 0, this sequence represents the last + * literals in the block of litLength size. */ unsigned int litLength; /* Literal length of the sequence. */ unsigned int matchLength; /* Match length of the sequence. */ /* Note: Users of this API may provide a sequence with matchLength == litLength == offset == 0. - * In this case, we will treat the "sequence" as a marker for a block boundary. + * In this case, we will treat the sequence as a marker for a block boundary. */ - unsigned int rep; /* Represents which repeat offset is used. Ranges from [0, 3]. - * If rep == 0, then this sequence does not contain a repeat offset. + unsigned int rep; /* Represents which repeat offset is represented by the field 'offset'. + * Ranges from [0, 3]. + * + * Repeat offsets are essentially previous offsets from previous sequences sorted in + * recency order. For more detail, see doc/zstd_compression_format.md + * + * If rep == 0, then 'offset' does not contain a repeat offset. * If rep > 0: * If litLength != 0: * rep == 1 --> offset == repeat_offset_1 @@ -1137,6 +1142,10 @@ typedef struct { * rep == 1 --> offset == repeat_offset_2 * rep == 2 --> offset == repeat_offset_3 * rep == 3 --> offset == repeat_offset_1 - 1 + * + * Note: This field is optional. ZSTD_getSequence() will calculate the value of + * 'rep', but repeat offsets do not necessarily need to be calculated from an external + * sequence provider's perspective. */ } ZSTD_Sequence; From 59624f3163b14526cf616420793a6d9c61c76e9f Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Wed, 28 Oct 2020 16:25:09 -0400 Subject: [PATCH 09/13] Remove implicit typecast to appease appVeyor windows build --- lib/compress/zstd_compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 5cd243a69..93bfed4da 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2493,7 +2493,7 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) assert(seqStoreLiteralsSize >= literalsRead); lastLLSize = seqStoreLiteralsSize - literalsRead; if (lastLLSize > 0) { - outSeqs[i].litLength = lastLLSize; + outSeqs[i].litLength = (U32)lastLLSize; outSeqs[i].matchLength = outSeqs[i].offset = outSeqs[i].rep = 0; seqStoreSeqSize++; } From 69bd5f065461535fb9718cbe6804a82afd2a67ba Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 29 Oct 2020 14:49:37 -0400 Subject: [PATCH 10/13] Correct literalsRead calculation to include longLength --- lib/compress/zstd_compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 93bfed4da..f18880cab 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2456,7 +2456,6 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) /* Ensure we have enough space for last literals "sequence" */ assert(zc->seqCollector.maxSequences >= seqStoreSeqSize + 1); for (i = 0; i < seqStoreSeqSize; ++i) { - literalsRead += seqStoreSeqs[i].litLength; outSeqs[i].litLength = seqStoreSeqs[i].litLength; outSeqs[i].matchLength = seqStoreSeqs[i].matchLength + MINMATCH; @@ -2487,6 +2486,7 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) } else { outSeqs[i].offset = seqStoreSeqs[i].offset - ZSTD_REP_NUM; } + literalsRead += outSeqs[i].litLength; } /* Insert last literals (if any exist) in the block as a sequence with ml == off == 0 */ From 32cac2627ac2f17d025ee0fced7f53c857f9472f Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Thu, 29 Oct 2020 16:41:17 -0400 Subject: [PATCH 11/13] Emit last literals of 0 size as well, to indicate block boundary --- lib/compress/zstd_compress.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index f18880cab..d5185dbde 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2489,14 +2489,16 @@ static void ZSTD_copyBlockSequences(ZSTD_CCtx* zc) literalsRead += outSeqs[i].litLength; } - /* Insert last literals (if any exist) in the block as a sequence with ml == off == 0 */ + /* Insert last literals (if any exist) in the block as a sequence with ml == off == 0. + * If there are no last literals, then we'll emit (of: 0, ml: 0, ll: 0), which is a marker + * for the block boundary, according to the API. + */ assert(seqStoreLiteralsSize >= literalsRead); lastLLSize = seqStoreLiteralsSize - literalsRead; - if (lastLLSize > 0) { - outSeqs[i].litLength = (U32)lastLLSize; - outSeqs[i].matchLength = outSeqs[i].offset = outSeqs[i].rep = 0; - seqStoreSeqSize++; - } + outSeqs[i].litLength = (U32)lastLLSize; + outSeqs[i].matchLength = outSeqs[i].offset = outSeqs[i].rep = 0; + seqStoreSeqSize++; + zc->seqCollector.seqIndex += seqStoreSeqSize; } From 536e89c723c5f40cbaccf28990e8546b55bcf9ba Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Fri, 30 Oct 2020 12:13:19 -0400 Subject: [PATCH 12/13] Sequence extractor should update CBlockState --- lib/compress/zstd_compress.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index d5185dbde..c96413245 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2574,6 +2574,7 @@ static size_t ZSTD_compressBlock_internal(ZSTD_CCtx* zc, if (zc->seqCollector.collectSequences) { ZSTD_copyBlockSequences(zc); + ZSTD_confirmRepcodesAndEntropyTables(zc); return 0; } From f0da97642a97dbc8f1a25d678fabaf9bc56733ef Mon Sep 17 00:00:00 2001 From: senhuang42 Date: Fri, 30 Oct 2020 12:30:50 -0400 Subject: [PATCH 13/13] Specify that getSequences() will always emit block boundary sequences --- lib/zstd.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/zstd.h b/lib/zstd.h index b57e1de0f..2ebaa775e 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1143,7 +1143,7 @@ typedef struct { * rep == 2 --> offset == repeat_offset_3 * rep == 3 --> offset == repeat_offset_1 - 1 * - * Note: This field is optional. ZSTD_getSequence() will calculate the value of + * Note: This field is optional. ZSTD_getSequences() will calculate the value of * 'rep', but repeat offsets do not necessarily need to be calculated from an external * sequence provider's perspective. */ @@ -1292,8 +1292,9 @@ ZSTDLIB_API unsigned long long ZSTD_decompressBound(const void* src, size_t srcS ZSTDLIB_API size_t ZSTD_frameHeaderSize(const void* src, size_t srcSize); /*! ZSTD_getSequences() : - * Extract sequences from the sequence store. Any last literals in the block will be represented as a sequence - * with offset == 0, matchLength == 0, litLength == last literals size. + * Extract sequences from the sequence store. + * Each block will end with a dummy sequence with offset == 0, matchLength == 0, and litLength == length of last literals. + * * zc can be used to insert custom compression params. * This function invokes ZSTD_compress2 * @return : number of sequences extracted