Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lazy parameters adaptation (part 1 - ZSTD_c_stableInBuffer) #2974

Merged
merged 13 commits into from
Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/common/error_private.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const char* ERR_getErrorString(ERR_enum code)
case PREFIX(tableLog_tooLarge): return "tableLog requires too much memory : unsupported";
case PREFIX(maxSymbolValue_tooLarge): return "Unsupported max Symbol Value : too large";
case PREFIX(maxSymbolValue_tooSmall): return "Specified maxSymbolValue is too small";
case PREFIX(stabilityCondition_notRespected): return "pledged buffer stability condition is not respected";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ZSTD_checkBufferStability() is currently using srcBuffer_wrong and dstBuffer_wrong. If you're going to add this code, can you please switch them over too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good idea

case PREFIX(dictionary_corrupted): return "Dictionary is corrupted";
case PREFIX(dictionary_wrong): return "Dictionary mismatch";
case PREFIX(dictionaryCreation_failed): return "Cannot create Dictionary from provided samples";
Expand Down
158 changes: 108 additions & 50 deletions lib/compress/zstd_compress.c

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions lib/compress/zstd_compress_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ struct ZSTD_CCtx_s {

/* Stable in/out buffer verification */
ZSTD_inBuffer expectedInBuffer;
size_t stableIn_notConsumed; /* nb bytes within stable input buffer that are said to be consumed but are not */
size_t expectedOutBufferSize;

/* Dictionary */
Expand Down
30 changes: 15 additions & 15 deletions lib/zstd.h
Original file line number Diff line number Diff line change
Expand Up @@ -1829,32 +1829,32 @@ ZSTDLIB_STATIC_API size_t ZSTD_CCtx_refPrefix_advanced(ZSTD_CCtx* cctx, const vo
* Experimental parameter.
* Default is 0 == disabled. Set to 1 to enable.
*
* Tells the compressor that the ZSTD_inBuffer will ALWAYS be the same
* between calls, except for the modifications that zstd makes to pos (the
* caller must not modify pos). This is checked by the compressor, and
* compression will fail if it ever changes. This means the only flush
* mode that makes sense is ZSTD_e_end, so zstd will error if ZSTD_e_end
* is not used. The data in the ZSTD_inBuffer in the range [src, src + pos)
* MUST not be modified during compression or you will get data corruption.
* Tells the compressor that input data presented with ZSTD_inBuffer
* will ALWAYS be the same between calls.
* Technically, the @src pointer must never be changed,
* and the @pos field can only be updated by zstd.
* However, it's possible to increase the @size field,
* allowing scenarios where more data can be appended after compressions starts.
* These conditions are checked by the compressor,
* and compression will fail if they are not respected.
* Also, data in the ZSTD_inBuffer within the range [src, src + pos)
* MUST not be modified during compression or it will result in data corruption.
*
* When this flag is enabled zstd won't allocate an input window buffer,
* because the user guarantees it can reference the ZSTD_inBuffer until
* the frame is complete. But, it will still allocate an output buffer
* large enough to fit a block (see ZSTD_c_stableOutBuffer). This will also
* avoid the memcpy() from the input buffer to the input window buffer.
*
* NOTE: ZSTD_compressStream2() will error if ZSTD_e_end is not used.
* That means this flag cannot be used with ZSTD_compressStream().
*
* NOTE: So long as the ZSTD_inBuffer always points to valid memory, using
* this flag is ALWAYS memory safe, and will never access out-of-bounds
* memory. However, compression WILL fail if you violate the preconditions.
* memory. However, compression WILL fail if conditions are not respected.
*
* WARNING: The data in the ZSTD_inBuffer in the range [dst, dst + pos) MUST
* not be modified during compression or you will get data corruption. This
* is because zstd needs to reference data in the ZSTD_inBuffer to find
* WARNING: The data in the ZSTD_inBuffer in the range [src, src + pos) MUST
* not be modified during compression or it will result in data corruption.
* This is because zstd needs to reference data in the ZSTD_inBuffer to find
* matches. Normally zstd maintains its own window buffer for this purpose,
* but passing this flag tells zstd to use the user provided buffer.
* but passing this flag tells zstd to rely on user provided buffer instead.
*/
#define ZSTD_c_stableInBuffer ZSTD_c_experimentalParam9

Expand Down
1 change: 1 addition & 0 deletions lib/zstd_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ typedef enum {
ZSTD_error_tableLog_tooLarge = 44,
ZSTD_error_maxSymbolValue_tooLarge = 46,
ZSTD_error_maxSymbolValue_tooSmall = 48,
ZSTD_error_stabilityCondition_notRespected = 50,
ZSTD_error_stage_wrong = 60,
ZSTD_error_init_missing = 62,
ZSTD_error_memory_allocation = 64,
Expand Down
2 changes: 1 addition & 1 deletion tests/fuzzer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,7 @@ static int basicUnitTests(U32 const seed, double compressibility)

DISPLAYLEVEL(3, "test%3i : compress a NULL input with each level : ", testNb++);
{ int level = -1;
ZSTD_CCtx* cctx = ZSTD_createCCtx();
ZSTD_CCtx* const cctx = ZSTD_createCCtx();
if (!cctx) goto _output_error;
for (level = -1; level <= ZSTD_maxCLevel(); ++level) {
CHECK_Z( ZSTD_compress(compressedBuffer, compressedBufferSize, NULL, 0, level) );
Expand Down
114 changes: 81 additions & 33 deletions tests/zstreamtest.c
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,7 @@ static int basicUnitTests(U32 seed, double compressibility)
DISPLAYLEVEL(3, "OK (error detected : %s) \n", ZSTD_getErrorName(r));
} }

/* Complex context re-use scenario */
/* Compression state re-use scenario */
DISPLAYLEVEL(3, "test%3i : context re-use : ", testNb++);
ZSTD_freeCStream(zc);
zc = ZSTD_createCStream();
Expand All @@ -634,8 +634,7 @@ static int basicUnitTests(U32 seed, double compressibility)
CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) );
if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */
DISPLAYLEVEL(5, "end1 ");
{ size_t const r = ZSTD_endStream(zc, &outBuff);
if (r != 0) goto _output_error; } /* error, or some data not flushed */
if (ZSTD_endStream(zc, &outBuff) != 0) goto _output_error; /* error, or some data not flushed */
}
/* use 2 */
{ size_t const inSize = 1025; /* will not continue, because tables auto-adjust and are therefore different size */
Expand All @@ -653,8 +652,7 @@ static int basicUnitTests(U32 seed, double compressibility)
CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) );
if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */
DISPLAYLEVEL(5, "end2 ");
{ size_t const r = ZSTD_endStream(zc, &outBuff);
if (r != 0) goto _output_error; } /* error, or some data not flushed */
if (ZSTD_endStream(zc, &outBuff) != 0) goto _output_error; /* error, or some data not flushed */
}
DISPLAYLEVEL(3, "OK \n");

Expand Down Expand Up @@ -771,11 +769,12 @@ static int basicUnitTests(U32 seed, double compressibility)
}

/* Compression with ZSTD_c_stable{In,Out}Buffer */
{ ZSTD_CCtx* cctx = ZSTD_createCCtx();
{ ZSTD_CCtx* const cctx = ZSTD_createCCtx();
ZSTD_inBuffer in;
ZSTD_outBuffer out;
size_t cctxSize1;
size_t cctxSize2;
assert(cctx != NULL);
in.src = CNBuffer;
in.size = CNBufferSize;
out.dst = compressedBuffer;
Expand All @@ -786,30 +785,33 @@ static int basicUnitTests(U32 seed, double compressibility)
CHECK(!(cSize < ZSTD_compressBound(CNBufferSize)), "cSize too large for test");
CHECK_Z(cSize = ZSTD_compress2(cctx, compressedBuffer, cSize + 4, CNBuffer, CNBufferSize));
CHECK_Z(cctxSize1 = ZSTD_sizeof_CCtx(cctx));
{ ZSTD_CCtx* cctx2 = ZSTD_createCCtx();
/* @cctxSize2 : sizeof_CCtx when doing full streaming (no stable in/out) */
{ ZSTD_CCtx* const cctx2 = ZSTD_createCCtx();
assert(cctx2 != NULL);
in.pos = out.pos = 0;
CHECK_Z(ZSTD_compressStream2(cctx2, &out, &in, ZSTD_e_continue));
CHECK(!(ZSTD_compressStream2(cctx2, &out, &in, ZSTD_e_end) == 0), "Not finished");
CHECK_Z(cctxSize2 = ZSTD_sizeof_CCtx(cctx2));
ZSTD_freeCCtx(cctx2);
}
{ ZSTD_CCtx* cctx3 = ZSTD_createCCtx();
/* @cctxSize1 : sizeof_CCtx when doing single-shot compression (no streaming) */
{ ZSTD_CCtx* const cctx1 = ZSTD_createCCtx();
ZSTD_parameters params = ZSTD_getParams(0, CNBufferSize, 0);
size_t cSize3;
assert(cctx1 != NULL);
params.fParams.checksumFlag = 1;
cSize3 = ZSTD_compress_advanced(cctx3, compressedBuffer, compressedBufferSize, CNBuffer, CNBufferSize, NULL, 0, params);
cSize3 = ZSTD_compress_advanced(cctx1, compressedBuffer, compressedBufferSize, CNBuffer, CNBufferSize, NULL, 0, params);
CHECK_Z(cSize3);
CHECK(!(cSize == cSize3), "Must be same compressed size");
CHECK(!(cctxSize1 == ZSTD_sizeof_CCtx(cctx3)), "Must be same CCtx size");
ZSTD_freeCCtx(cctx3);
CHECK(!(cctxSize1 == ZSTD_sizeof_CCtx(cctx1)), "Must be same CCtx size");
ZSTD_freeCCtx(cctx1);
}
CHECK(!(cctxSize1 < cctxSize2), "Stable buffers means less allocated size");
CHECK_Z(ZSTD_decompress(decodedBuffer, CNBufferSize, compressedBuffer, cSize));
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : ZSTD_compress2() doesn't modify user parameters : ", testNb++);
{
int stableInBuffer;
{ int stableInBuffer;
int stableOutBuffer;
CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_stableInBuffer, &stableInBuffer));
CHECK_Z(ZSTD_CCtx_getParameter(cctx, ZSTD_c_stableOutBuffer, &stableOutBuffer));
Expand Down Expand Up @@ -866,30 +868,73 @@ static int basicUnitTests(U32 seed, double compressibility)
in.pos = 0;
{ size_t const ret = ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_end);
CHECK(!ZSTD_isError(ret), "Must error");
CHECK(!(ZSTD_getErrorCode(ret) == ZSTD_error_srcBuffer_wrong), "Must be this error");
CHECK(!(ZSTD_getErrorCode(ret) == ZSTD_error_stabilityCondition_notRespected), "Must be this error");
}
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : ZSTD_compressStream2() ZSTD_c_stableInBuffer with continue and flush : ", testNb++);
in.src = CNBuffer;
in.size = CNBufferSize;
in.pos = 0;
out.pos = 0;
out.size = compressedBufferSize;
CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_only));
{ size_t const ret = ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue);
CHECK(!ZSTD_isError(ret), "Must error");
CHECK(!(ZSTD_getErrorCode(ret) == ZSTD_error_srcBuffer_wrong), "Must be this error");
}
CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_only));
{ size_t const ret = ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_flush);
CHECK(!ZSTD_isError(ret), "Must error");
CHECK(!(ZSTD_getErrorCode(ret) == ZSTD_error_srcBuffer_wrong), "Must be this error");
}
/* stableSrc + streaming */
DISPLAYLEVEL(3, "test%3i : ZSTD_c_stableInBuffer compatibility with compressStream, flushStream and endStream : ", testNb++);
CHECK_Z( ZSTD_initCStream(cctx, 1) );
CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_stableInBuffer, 1) );
{ ZSTD_inBuffer inBuf;
ZSTD_outBuffer outBuf;
const size_t nonZeroStartPos = 18;
const size_t inputSize = 500;
inBuf.src = CNBuffer;
inBuf.size = 100;
inBuf.pos = nonZeroStartPos;
outBuf.dst = (char*)(compressedBuffer)+cSize;
outBuf.size = ZSTD_compressBound(inputSize);
outBuf.pos = 0;
CHECK_Z( ZSTD_compressStream(cctx, &outBuf, &inBuf) );
inBuf.size = 200;
CHECK_Z( ZSTD_compressStream(cctx, &outBuf, &inBuf) );
CHECK_Z( ZSTD_flushStream(cctx, &outBuf) );
inBuf.size = nonZeroStartPos + inputSize;
CHECK_Z( ZSTD_compressStream(cctx, &outBuf, &inBuf) );
CHECK(ZSTD_endStream(cctx, &outBuf) != 0, "compression should be successful and fully flushed");
{ const void* const realSrcStart = (const char*)inBuf.src + nonZeroStartPos;
void* const verifBuf = (char*)outBuf.dst + outBuf.pos;
const size_t decSize = ZSTD_decompress(verifBuf, inputSize, outBuf.dst, outBuf.pos);
CHECK_Z(decSize);
CHECK(decSize != inputSize, "regenerated %zu bytes, instead of %zu", decSize, inputSize);
CHECK(memcmp(realSrcStart, verifBuf, inputSize) != 0, "regenerated data different from original");
} }
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : ZSTD_compressStream2() ZSTD_c_stableInBuffer allocated size : ", testNb++);
/* stableSrc + streaming */
DISPLAYLEVEL(3, "test%3i : ZSTD_c_stableInBuffer compatibility with compressStream2, using different end directives : ", testNb++);
CHECK_Z( ZSTD_initCStream(cctx, 1) );
CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_stableInBuffer, 1) );
{ ZSTD_inBuffer inBuf;
ZSTD_outBuffer outBuf;
const size_t nonZeroStartPos = 18;
const size_t inputSize = 500;
inBuf.src = CNBuffer;
inBuf.size = 100;
inBuf.pos = nonZeroStartPos;
outBuf.dst = (char*)(compressedBuffer)+cSize;
outBuf.size = ZSTD_compressBound(inputSize);
outBuf.pos = 0;
CHECK_Z( ZSTD_compressStream2(cctx, &outBuf, &inBuf, ZSTD_e_continue) );
inBuf.size = 200;
CHECK_Z( ZSTD_compressStream2(cctx, &outBuf, &inBuf, ZSTD_e_continue) );
CHECK_Z( ZSTD_compressStream2(cctx, &outBuf, &inBuf, ZSTD_e_flush) );
inBuf.size = nonZeroStartPos + inputSize;
CHECK_Z( ZSTD_compressStream2(cctx, &outBuf, &inBuf, ZSTD_e_continue) );
CHECK( ZSTD_compressStream2(cctx, &outBuf, &inBuf, ZSTD_e_end) != 0, "compression should be successful and fully flushed");
{ const void* const realSrcStart = (const char*)inBuf.src + nonZeroStartPos;
void* const verifBuf = (char*)outBuf.dst + outBuf.pos;
const size_t decSize = ZSTD_decompress(verifBuf, inputSize, outBuf.dst, outBuf.pos);
CHECK_Z(decSize);
CHECK(decSize != inputSize, "regenerated %zu bytes, instead of %zu", decSize, inputSize);
CHECK(memcmp(realSrcStart, verifBuf, inputSize) != 0, "regenerated data different from original");
} }
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : ZSTD_compressStream2() with ZSTD_c_stableInBuffer: context size : ", testNb++);
{ size_t const cctxSize = ZSTD_sizeof_CCtx(cctx);
DISPLAYLEVEL(4, "cctxSize1=%zu; cctxSize=%zu; cctxSize2=%zu : ", cctxSize1, cctxSize, cctxSize2);
CHECK(!(cctxSize1 < cctxSize), "Must be bigger than single-pass");
CHECK(!(cctxSize < cctxSize2), "Must be smaller than streaming");
cctxSize1 = cctxSize;
Expand All @@ -900,8 +945,10 @@ static int basicUnitTests(U32 seed, double compressibility)
CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters));
CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 1));
CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_stableOutBuffer, 1));
in.src = CNBuffer;
in.pos = out.pos = 0;
in.size = MIN(CNBufferSize, 10);
out.size = compressedBufferSize;
CHECK_Z(ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_flush));
in.pos = 0;
in.size = CNBufferSize - in.size;
Expand All @@ -916,12 +963,13 @@ static int basicUnitTests(U32 seed, double compressibility)
in.pos = out.pos = 0;
{ size_t const ret = ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue);
CHECK(!ZSTD_isError(ret), "Must have errored");
CHECK(!(ZSTD_getErrorCode(ret) == ZSTD_error_dstBuffer_wrong), "Must be this error");
CHECK(!(ZSTD_getErrorCode(ret) == ZSTD_error_stabilityCondition_notRespected), "Must be this error");
}
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : ZSTD_compressStream2() ZSTD_c_stableOutBuffer allocated size : ", testNb++);
DISPLAYLEVEL(3, "test%3i : ZSTD_compressStream2() with ZSTD_c_stableOutBuffer: context size : ", testNb++);
{ size_t const cctxSize = ZSTD_sizeof_CCtx(cctx);
DISPLAYLEVEL(4, "cctxSize1=%zu; cctxSize=%zu; cctxSize2=%zu : ", cctxSize1, cctxSize, cctxSize2);
CHECK(!(cctxSize1 < cctxSize), "Must be bigger than single-pass and stableInBuffer");
CHECK(!(cctxSize < cctxSize2), "Must be smaller than streaming");
}
Expand Down