From 60059df051ce5d0005c22e0f7cf74bf16afee9a3 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 27 Sep 2017 15:16:27 -0700 Subject: [PATCH 01/19] shorter make test to avoid time out on travis CI Timed tests (fuzzer) are reduced Long tests are shortened (less data generated) --- tests/Makefile | 30 +++++++++++++------------- tests/playTests.sh | 52 +++++++++++++++++++++------------------------- 2 files changed, 39 insertions(+), 43 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 2746c1392..fc62b9e26 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -23,18 +23,18 @@ PRGDIR = ../programs PYTHON ?= python3 TESTARTEFACT := versionsTest namespaceTest -DEBUGLEVEL= 1 -DEBUGFLAGS= -g -DZSTD_DEBUG=$(DEBUGLEVEL) -CPPFLAGS += -I$(ZSTDDIR) -I$(ZSTDDIR)/common -I$(ZSTDDIR)/compress \ - -I$(ZSTDDIR)/dictBuilder -I$(ZSTDDIR)/deprecated -I$(PRGDIR) -CFLAGS ?= -O3 -CFLAGS += -Wall -Wextra -Wcast-qual -Wcast-align -Wshadow \ - -Wstrict-aliasing=1 -Wswitch-enum -Wdeclaration-after-statement \ - -Wstrict-prototypes -Wundef -Wformat-security \ - -Wvla -Wformat=2 -Winit-self -Wfloat-equal -Wwrite-strings \ - -Wredundant-decls -CFLAGS += $(DEBUGFLAGS) $(MOREFLAGS) -FLAGS = $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) +DEBUGLEVEL ?= 1 +DEBUGFLAGS = -g -DZSTD_DEBUG=$(DEBUGLEVEL) +CPPFLAGS += -I$(ZSTDDIR) -I$(ZSTDDIR)/common -I$(ZSTDDIR)/compress \ + -I$(ZSTDDIR)/dictBuilder -I$(ZSTDDIR)/deprecated -I$(PRGDIR) +CFLAGS ?= -O3 +CFLAGS += -Wall -Wextra -Wcast-qual -Wcast-align -Wshadow \ + -Wstrict-aliasing=1 -Wswitch-enum -Wdeclaration-after-statement \ + -Wstrict-prototypes -Wundef -Wformat-security \ + -Wvla -Wformat=2 -Winit-self -Wfloat-equal -Wwrite-strings \ + -Wredundant-decls +CFLAGS += $(DEBUGFLAGS) $(MOREFLAGS) +FLAGS = $(CPPFLAGS) $(CFLAGS) $(LDFLAGS) ZSTDCOMMON_FILES := $(ZSTDDIR)/common/*.c @@ -58,8 +58,8 @@ endif MULTITHREAD = $(MULTITHREAD_CPP) $(MULTITHREAD_LD) VOID = /dev/null -ZSTREAM_TESTTIME ?= -T2mn -FUZZERTEST ?= -T5mn +ZSTREAM_TESTTIME ?= -T90s +FUZZERTEST ?= -T200s ZSTDRTTEST = --test-large-data DECODECORPUS_TESTTIME ?= -T30 @@ -241,7 +241,7 @@ endif #----------------------------------------------------------------------------- -#make tests validated only for MSYS, Linux, OSX, BSD, Hurd and Solaris targets +# make tests validated only for below targets #----------------------------------------------------------------------------- ifneq (,$(filter $(HOST_OS),MSYS POSIX)) diff --git a/tests/playTests.sh b/tests/playTests.sh index b3e0b8ab8..3ed8df9ef 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -594,7 +594,6 @@ $ECHO "\n**** zstd --list/-l single frame tests ****" ./datagen > tmp1 ./datagen > tmp2 ./datagen > tmp3 -./datagen > tmp4 $ZSTD tmp* $ZSTD -l *.zst $ZSTD -lv *.zst @@ -603,9 +602,7 @@ $ZSTD --list -v *.zst $ECHO "\n**** zstd --list/-l multiple frame tests ****" cat tmp1.zst tmp2.zst > tmp12.zst -cat tmp3.zst tmp4.zst > tmp34.zst -cat tmp12.zst tmp34.zst > tmp1234.zst -cat tmp12.zst tmp4.zst > tmp124.zst +cat tmp12.zst tmp3.zst > tmp123.zst $ZSTD -l *.zst $ZSTD -lv *.zst $ZSTD --list *.zst @@ -615,7 +612,7 @@ $ECHO "\n**** zstd --list/-l error detection tests ****" ! $ZSTD -l tmp1 tmp1.zst ! $ZSTD --list tmp* ! $ZSTD -lv tmp1* -! $ZSTD --list -v tmp2 tmp23.zst +! $ZSTD --list -v tmp2 tmp12.zst $ECHO "\n**** zstd --list/-l test with null files ****" ./datagen -g0 > tmp5 @@ -644,44 +641,43 @@ if [ "$1" != "--test-large-data" ]; then fi roundTripTest -g270000000 1 -roundTripTest -g270000000 2 -roundTripTest -g270000000 3 +roundTripTest -g250000000 2 +roundTripTest -g230000000 3 roundTripTest -g140000000 -P60 4 -roundTripTest -g140000000 -P60 5 -roundTripTest -g140000000 -P60 6 +roundTripTest -g130000000 -P62 5 +roundTripTest -g120000000 -P65 6 roundTripTest -g70000000 -P70 7 -roundTripTest -g70000000 -P70 8 -roundTripTest -g70000000 -P70 9 +roundTripTest -g60000000 -P71 8 +roundTripTest -g50000000 -P73 9 roundTripTest -g35000000 -P75 10 -roundTripTest -g35000000 -P75 11 -roundTripTest -g35000000 -P75 12 +roundTripTest -g30000000 -P76 11 +roundTripTest -g25000000 -P78 12 roundTripTest -g18000013 -P80 13 roundTripTest -g18000014 -P80 14 -roundTripTest -g18000015 -P80 15 -roundTripTest -g18000016 -P80 16 -roundTripTest -g18000017 -P80 17 +roundTripTest -g18000015 -P81 15 +roundTripTest -g18000016 -P84 16 +roundTripTest -g18000017 -P88 17 roundTripTest -g18000018 -P94 18 -roundTripTest -g18000019 -P94 19 +roundTripTest -g18000019 -P96 19 -roundTripTest -g68000020 -P99 20 -roundTripTest -g6000000000 -P99 1 +roundTripTest -g5000000000 -P99 1 fileRoundTripTest -g4193M -P99 1 $ECHO "\n**** zstd long, long distance matching round-trip tests **** " roundTripTest -g0 "2 --long" roundTripTest -g270000000 "1 --long" -roundTripTest -g140000000 -P60 "5 --long" -roundTripTest -g70000000 -P70 "8 --long" +roundTripTest -g130000000 -P60 "5 --long" +roundTripTest -g35000000 -P70 "8 --long" roundTripTest -g18000001 -P80 "18 --long" fileRoundTripTest -g4100M -P99 "1 --long" # Test large window logs -roundTripTest -g4100M -P50 "1 --long=30" -roundTripTest -g4100M -P50 "1 --long --zstd=wlog=30,clog=30" +roundTripTest -g2100M -P50 "1 --long=30" +roundTripTest -g1000M -P50 "1 --long --zstd=wlog=30,clog=30" # Test parameter parsing roundTripTest -g1M -P50 "1 --long=30" " --memory=1024MB" roundTripTest -g1M -P50 "1 --long=30 --zstd=wlog=29" " --memory=512MB" @@ -692,11 +688,11 @@ roundTripTest -g1M -P50 "1 --long=30" " --zstd=wlog=29 --memory=1024MB" if [ -n "$hasMT" ] then $ECHO "\n**** zstdmt long round-trip tests **** " - roundTripTest -g99000000 -P99 "20 -T2" " " - roundTripTest -g6000000000 -P99 "1 -T2" " " - roundTripTest -g1500000000 -P97 "1 -T999" " " - fileRoundTripTest -g4195M -P98 " -T0" " " - roundTripTest -g1500000000 -P97 "1 --long=23 -T2" " " + roundTripTest -g80000000 -P99 "19 -T2" " " + roundTripTest -g5000000000 -P99 "1 -T2" " " + roundTripTest -g500000000 -P97 "1 -T999" " " + fileRoundTripTest -g4103M -P98 " -T0" " " + roundTripTest -g400000000 -P97 "1 --long=24 -T2" " " else $ECHO "\n**** no multithreading, skipping zstdmt tests **** " fi From bdd0f6f0468eae5779cceac7b36459418ddbabde Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 27 Sep 2017 15:20:08 -0700 Subject: [PATCH 02/19] improved make clean in tests/fuzz --- tests/fuzz/Makefile | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/fuzz/Makefile b/tests/fuzz/Makefile index 6d2a0cfa9..d9b00fd2a 100644 --- a/tests/fuzz/Makefile +++ b/tests/fuzz/Makefile @@ -118,9 +118,10 @@ regressiontest: corpora clean: @$(MAKE) -C $(ZSTDDIR) clean - @$(RM) -f *.a *.o - @$(RM) -f simple_round_trip stream_round_trip simple_decompress stream_decompress + @$(RM) *.a *.o + @$(RM) simple_round_trip stream_round_trip simple_decompress \ + stream_decompress block_decompress block_round_trip cleanall: - @$(RM) -rf Fuzzer - @$(RM) -rf corpora + @$(RM) -r Fuzzer + @$(RM) -r corpora From f9de54acfb27c0e02cb22abb02eb2d72cb51ea50 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 27 Sep 2017 15:38:27 -0700 Subject: [PATCH 03/19] reduced memory requirements for --long tests in new --long test section --- tests/playTests.sh | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/playTests.sh b/tests/playTests.sh index 3ed8df9ef..4689dcdda 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -674,15 +674,14 @@ roundTripTest -g270000000 "1 --long" roundTripTest -g130000000 -P60 "5 --long" roundTripTest -g35000000 -P70 "8 --long" roundTripTest -g18000001 -P80 "18 --long" -fileRoundTripTest -g4100M -P99 "1 --long" # Test large window logs -roundTripTest -g2100M -P50 "1 --long=30" -roundTripTest -g1000M -P50 "1 --long --zstd=wlog=30,clog=30" +roundTripTest -g700M -P50 "1 --long=29" +roundTripTest -g600M -P50 "1 --long --zstd=wlog=29,clog=28" # Test parameter parsing -roundTripTest -g1M -P50 "1 --long=30" " --memory=1024MB" -roundTripTest -g1M -P50 "1 --long=30 --zstd=wlog=29" " --memory=512MB" -roundTripTest -g1M -P50 "1 --long=30" " --long=29 --memory=1024MB" -roundTripTest -g1M -P50 "1 --long=30" " --zstd=wlog=29 --memory=1024MB" +roundTripTest -g1M -P50 "1 --long=29" " --memory=512MB" +roundTripTest -g1M -P50 "1 --long=29 --zstd=wlog=28" " --memory=256MB" +roundTripTest -g1M -P50 "1 --long=29" " --long=28 --memory=512MB" +roundTripTest -g1M -P50 "1 --long=29" " --zstd=wlog=28 --memory=512MB" if [ -n "$hasMT" ] From 02502191e5ea2b60a88c42ed4501e8a18432d7a7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 27 Sep 2017 15:48:06 -0700 Subject: [PATCH 04/19] separated --long tests between short and long tests A fast subset of these tests is now played in short test mode --- tests/playTests.sh | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/tests/playTests.sh b/tests/playTests.sh index 4689dcdda..a4cd4ffb5 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -635,11 +635,23 @@ $ZSTD -lv tmp1.zst rm tmp* +$ECHO "\n**** zstd long distance matching tests **** " +roundTripTest -g0 " --long" +roundTripTest -g9M "2 --long" +# Test parameter parsing +roundTripTest -g1M -P50 "1 --long=29" " --memory=512MB" +roundTripTest -g1M -P50 "1 --long=29 --zstd=wlog=28" " --memory=256MB" +roundTripTest -g1M -P50 "1 --long=29" " --long=28 --memory=512MB" +roundTripTest -g1M -P50 "1 --long=29" " --zstd=wlog=28 --memory=512MB" + + if [ "$1" != "--test-large-data" ]; then $ECHO "Skipping large data tests" exit 0 fi +$ECHO "\n**** large files tests **** " + roundTripTest -g270000000 1 roundTripTest -g250000000 2 roundTripTest -g230000000 3 @@ -668,8 +680,8 @@ roundTripTest -g5000000000 -P99 1 fileRoundTripTest -g4193M -P99 1 + $ECHO "\n**** zstd long, long distance matching round-trip tests **** " -roundTripTest -g0 "2 --long" roundTripTest -g270000000 "1 --long" roundTripTest -g130000000 -P60 "5 --long" roundTripTest -g35000000 -P70 "8 --long" @@ -677,11 +689,6 @@ roundTripTest -g18000001 -P80 "18 --long" # Test large window logs roundTripTest -g700M -P50 "1 --long=29" roundTripTest -g600M -P50 "1 --long --zstd=wlog=29,clog=28" -# Test parameter parsing -roundTripTest -g1M -P50 "1 --long=29" " --memory=512MB" -roundTripTest -g1M -P50 "1 --long=29 --zstd=wlog=28" " --memory=256MB" -roundTripTest -g1M -P50 "1 --long=29" " --long=28 --memory=512MB" -roundTripTest -g1M -P50 "1 --long=29" " --zstd=wlog=28 --memory=512MB" if [ -n "$hasMT" ] From bc32b40b9808f8b6c2d4526ae9738f5240ac01f4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 27 Sep 2017 17:27:38 -0700 Subject: [PATCH 05/19] reduced zstreamtest --mt memory load adjust compression level, hence memory usage, depending on nb threads in order to run correctly on memory-starved VM. --- Makefile | 20 ++++++++++---------- tests/zstreamtest.c | 38 ++++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/Makefile b/Makefile index 5b3f5fdf7..ac53c2dd8 100644 --- a/Makefile +++ b/Makefile @@ -296,7 +296,7 @@ endif #------------------------------------------------------------------------ -#make tests validated only for MSYS, Linux, OSX, kFreeBSD and Hurd targets +# target specific tests #------------------------------------------------------------------------ ifneq (,$(filter $(HOST_OS),MSYS POSIX)) cmakebuild: @@ -306,38 +306,38 @@ cmakebuild: cd $(BUILDIR)/cmake/build ; cmake -DCMAKE_INSTALL_PREFIX:PATH=~/install_test_dir $(CMAKE_PARAMS) .. ; $(MAKE) install ; $(MAKE) uninstall c90build: clean - gcc -v + $(CC) -v CFLAGS="-std=c90" $(MAKE) allmost # will fail, due to missing support for `long long` gnu90build: clean - gcc -v + $(CC) -v CFLAGS="-std=gnu90" $(MAKE) allmost c99build: clean - gcc -v + $(CC) -v CFLAGS="-std=c99" $(MAKE) allmost gnu99build: clean - gcc -v + $(CC) -v CFLAGS="-std=gnu99" $(MAKE) allmost c11build: clean - gcc -v + $(CC) -v CFLAGS="-std=c11" $(MAKE) allmost bmix64build: clean - gcc -v + $(CC) -v CFLAGS="-O3 -mbmi -Werror" $(MAKE) -C $(TESTDIR) test bmix32build: clean - gcc -v + $(CC) -v CFLAGS="-O3 -mbmi -mx32 -Werror" $(MAKE) -C $(TESTDIR) test bmi32build: clean - gcc -v + $(CC) -v CFLAGS="-O3 -mbmi -m32 -Werror" $(MAKE) -C $(TESTDIR) test staticAnalyze: clean - gcc -v + $(CC) -v CPPFLAGS=-g scan-build --status-bugs -v $(MAKE) all endif diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 3f190f050..28259dda8 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -741,20 +741,20 @@ static int fuzzerTests(U32 seed, U32 nbTests, unsigned startTest, double compres static const U32 maxSampleLog = 19; size_t const srcBufferSize = (size_t)1<= srcBufferSize) - maxTestSize = srcBufferSize-1; + maxTestSize = MIN(maxTestSize, srcBufferSize-16); { U64 const pledgedSrcSize = (FUZ_rand(&lseed) & 3) ? 0 : maxTestSize; CHECK_Z( ZSTD_resetCStream(zc, pledgedSrcSize) ); } @@ -999,15 +998,16 @@ static int fuzzerTests_MT(U32 seed, U32 nbTests, unsigned startTest, double comp U32 result = 0; U32 testNb = 0; U32 coreSeed = seed; - ZSTDMT_CCtx* zc = ZSTDMT_createCCtx(2); /* will be reset sometimes */ + U32 nbThreads = 2; + ZSTDMT_CCtx* zc = ZSTDMT_createCCtx(nbThreads); /* will be reset sometimes */ ZSTD_DStream* zd = ZSTD_createDStream(); /* will be reset sometimes */ ZSTD_DStream* const zd_noise = ZSTD_createDStream(); clock_t const startClock = clock(); const BYTE* dict=NULL; /* can keep same dict on 2 consecutive tests */ size_t dictSize = 0; U32 oldTestLog = 0; - U32 const cLevelMax = bigTests ? (U32)ZSTD_maxCLevel() : g_cLevelMax_smallTests; - U32 const nbThreadsMax = bigTests ? 5 : 2; + int const cLevelMax = bigTests ? (U32)ZSTD_maxCLevel()-1 : g_cLevelMax_smallTests; + U32 const nbThreadsMax = bigTests ? 4 : 2; /* allocations */ cNoiseBuffer[0] = (BYTE*)malloc (srcBufferSize); @@ -1043,8 +1043,9 @@ static int fuzzerTests_MT(U32 seed, U32 nbTests, unsigned startTest, double comp size_t maxTestSize; /* init */ - if (nbTests >= testNb) { DISPLAYUPDATE(2, "\r%6u/%6u ", testNb, nbTests); } - else { DISPLAYUPDATE(2, "\r%6u ", testNb); } + if (testNb < nbTests) { + DISPLAYUPDATE(2, "\r%6u/%6u ", testNb, nbTests); + } else { DISPLAYUPDATE(2, "\r%6u ", testNb); } FUZ_rand(&coreSeed); lseed = coreSeed ^ prime32; @@ -1052,7 +1053,7 @@ static int fuzzerTests_MT(U32 seed, U32 nbTests, unsigned startTest, double comp /* some issues can only happen when reusing states */ if ((FUZ_rand(&lseed) & 0xFF) == 131) { U32 const nbThreadsCandidate = (FUZ_rand(&lseed) % 6) + 1; - U32 const nbThreads = MIN(nbThreadsCandidate, nbThreadsMax); + nbThreads = MIN(nbThreadsCandidate, nbThreadsMax); DISPLAYLEVEL(5, "Creating new context with %u threads \n", nbThreads); ZSTDMT_freeCCtx(zc); zc = ZSTDMT_createCCtx(nbThreads); @@ -1092,11 +1093,12 @@ static int fuzzerTests_MT(U32 seed, U32 nbTests, unsigned startTest, double comp } else { U32 const testLog = FUZ_rand(&lseed) % maxSrcLog; U32 const dictLog = FUZ_rand(&lseed) % maxSrcLog; - U32 const cLevelCandidate = (FUZ_rand(&lseed) % - (ZSTD_maxCLevel() - - (MAX(testLog, dictLog) / 3))) + - 1; - U32 const cLevel = MIN(cLevelCandidate, cLevelMax); + int const cLevelCandidate = ( FUZ_rand(&lseed) + % (ZSTD_maxCLevel() - (MAX(testLog, dictLog) / 2)) ) + + 1; + int const cLevelThreadAdjusted = cLevelCandidate - (nbThreads * 2) + 2; /* reduce cLevel when multiple threads to reduce memory consumption */ + int const cLevelMin = MAX(cLevelThreadAdjusted, 1); /* no negative cLevel yet */ + int const cLevel = MIN(cLevelMin, cLevelMax); maxTestSize = FUZ_rLogLength(&lseed, testLog); oldTestLog = testLog; /* random dictionary selection */ From aa800c4793a4bc044df53614ca652c96d59e1078 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 27 Sep 2017 18:00:15 -0700 Subject: [PATCH 06/19] reduced memory usage of zstreamtest --newapi to run on memory-constrained VM --- tests/zstreamtest.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 28259dda8..62af8e167 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -1279,8 +1279,9 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double const BYTE* dict = NULL; /* can keep same dict on 2 consecutive tests */ size_t dictSize = 0; U32 oldTestLog = 0; - U32 const cLevelMax = bigTests ? (U32)ZSTD_maxCLevel() : g_cLevelMax_smallTests; - U32 const nbThreadsMax = bigTests ? 5 : 1; + U32 windowLogMalus = 0; /* can survive between 2 loops */ + U32 const cLevelMax = bigTests ? (U32)ZSTD_maxCLevel()-1 : g_cLevelMax_smallTests; + U32 const nbThreadsMax = bigTests ? 4 : 2; ZSTD_CCtx_params* cctxParams = ZSTD_createCCtxParams(); /* allocations */ @@ -1388,10 +1389,14 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double cParams.chainLog += (FUZ_rand(&lseed) & 3) - 1; cParams.searchLog += (FUZ_rand(&lseed) & 3) - 1; cParams.searchLength += (FUZ_rand(&lseed) & 3) - 1; - cParams.targetLength = (U32)(cParams.targetLength * (0.5 + ((double)(FUZ_rand(&lseed) & 127) / 128))); + cParams.targetLength = (U32)((cParams.targetLength + 1 ) * (0.5 + ((double)(FUZ_rand(&lseed) & 127) / 128))); cParams = ZSTD_adjustCParams(cParams, 0, 0); - if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_windowLog, cParams.windowLog, useOpaqueAPI) ); + if (FUZ_rand(&lseed) & 1) { + CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_windowLog, cParams.windowLog, useOpaqueAPI) ); + assert(cParams.windowLog >= ZSTD_WINDOWLOG_MIN); /* guaranteed by ZSTD_adjustCParams() */ + windowLogMalus = (cParams.windowLog - ZSTD_WINDOWLOG_MIN) / 5; + } if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_hashLog, cParams.hashLog, useOpaqueAPI) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_chainLog, cParams.chainLog, useOpaqueAPI) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_searchLog, cParams.searchLog, useOpaqueAPI) ); @@ -1405,7 +1410,6 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double if (FUZ_rand(&lseed) & 3) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_ldmBucketSizeLog, FUZ_randomClampedLength(&lseed, 0, ZSTD_LDM_BUCKETSIZELOG_MAX), useOpaqueAPI) ); if (FUZ_rand(&lseed) & 3) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_ldmHashEveryLog, FUZ_randomClampedLength(&lseed, 0, ZSTD_WINDOWLOG_MAX - ZSTD_HASHLOG_MIN), useOpaqueAPI) ); - /* unconditionally set, to be sync with decoder */ /* mess with frame parameters */ if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_checksumFlag, FUZ_rand(&lseed) & 1, useOpaqueAPI) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_dictIDFlag, FUZ_rand(&lseed) & 1, useOpaqueAPI) ); @@ -1415,7 +1419,8 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double /* multi-threading parameters */ { U32 const nbThreadsCandidate = (FUZ_rand(&lseed) & 4) + 1; - U32 const nbThreads = MIN(nbThreadsCandidate, nbThreadsMax); + U32 const nbThreadsAdjusted = (windowLogMalus < nbThreadsCandidate) ? nbThreadsCandidate - windowLogMalus : 1; + U32 const nbThreads = MIN(nbThreadsAdjusted, nbThreadsMax); CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_nbThreads, nbThreads, useOpaqueAPI) ); if (nbThreads > 1) { U32 const jobLog = FUZ_rand(&lseed) % (testLog+1); From 9b5b47ac930442b24c0fb123274b5cc7a216d478 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 01:25:40 -0700 Subject: [PATCH 07/19] ensure adjustCParams adjust hLog and cLog even without srcSize It would previously exit when srcSize is unknown. But in the case of custom parameters, hLog and cLog can still be too large in comparison with windowLog. Reduces maximum memory allocated during zstreamtest --newapi --- lib/compress/zstd_compress.c | 21 +++++++++++++-------- lib/zstd.h | 30 +++++++++++++++--------------- tests/zstreamtest.c | 17 ++++++++++++++--- 3 files changed, 42 insertions(+), 26 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index e84149bd3..0dbe196ff 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -634,26 +634,31 @@ static U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat) mostly downsizing to reduce memory consumption and initialization. Both `srcSize` and `dictSize` are optional (use 0 if unknown), but if both are 0, no optimization can be done. - Note : cPar is considered validated at this stage. Use ZSTD_checkParams() to ensure that. */ + Note : cPar is considered validated at this stage. Use ZSTD_checkCParams() to ensure that condition. */ ZSTD_compressionParameters ZSTD_adjustCParams_internal(ZSTD_compressionParameters cPar, unsigned long long srcSize, size_t dictSize) { assert(ZSTD_checkCParams(cPar)==0); - if (srcSize+dictSize == 0) return cPar; /* no size information available : no adjustment */ - /* resize params, to use less memory when necessary */ - { U32 const minSrcSize = (srcSize==0) ? 500 : 0; + /* resize windowLog if src is small, to use less memory when necessary */ + ZSTD_STATIC_ASSERT(ZSTD_CONTENTSIZE_UNKNOWN == -1ULL); + if ( (dictSize || (srcSize+1 > 1)) /* srcSize test depends on static assert condition */ + && (srcSize-1 < (1ULL< srcLog) cPar.windowLog = srcLog; } } if (cPar.hashLog > cPar.windowLog) cPar.hashLog = cPar.windowLog; { U32 const cycleLog = ZSTD_cycleLog(cPar.chainLog, cPar.strategy); - if (cycleLog > cPar.windowLog) cPar.chainLog -= (cycleLog - cPar.windowLog); + if (cycleLog > cPar.windowLog) + cPar.chainLog -= (cycleLog - cPar.windowLog); } - if (cPar.windowLog < ZSTD_WINDOWLOG_ABSOLUTEMIN) cPar.windowLog = ZSTD_WINDOWLOG_ABSOLUTEMIN; /* required for frame header */ + if (cPar.windowLog < ZSTD_WINDOWLOG_ABSOLUTEMIN) + cPar.windowLog = ZSTD_WINDOWLOG_ABSOLUTEMIN; /* required for frame header */ return cPar; } diff --git a/lib/zstd.h b/lib/zstd.h index b7c75d5f2..59aa0b020 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -376,22 +376,22 @@ ZSTDLIB_API size_t ZSTD_DStreamOutSize(void); /*!< recommended size for output #define ZSTD_MAGIC_SKIPPABLE_START 0x184D2A50U #define ZSTD_MAGIC_DICTIONARY 0xEC30A437 /* v0.7+ */ -#define ZSTD_WINDOWLOG_MAX_32 30 -#define ZSTD_WINDOWLOG_MAX_64 31 +#define ZSTD_WINDOWLOG_MAX_32 30 +#define ZSTD_WINDOWLOG_MAX_64 31 #define ZSTD_WINDOWLOG_MAX ((unsigned)(sizeof(size_t) == 4 ? ZSTD_WINDOWLOG_MAX_32 : ZSTD_WINDOWLOG_MAX_64)) -#define ZSTD_WINDOWLOG_MIN 10 -#define ZSTD_HASHLOG_MAX MIN(ZSTD_WINDOWLOG_MAX, 30) -#define ZSTD_HASHLOG_MIN 6 -#define ZSTD_CHAINLOG_MAX MIN(ZSTD_WINDOWLOG_MAX+1, 30) -#define ZSTD_CHAINLOG_MIN ZSTD_HASHLOG_MIN -#define ZSTD_HASHLOG3_MAX 17 -#define ZSTD_SEARCHLOG_MAX (ZSTD_WINDOWLOG_MAX-1) -#define ZSTD_SEARCHLOG_MIN 1 -#define ZSTD_SEARCHLENGTH_MAX 7 /* only for ZSTD_fast, other strategies are limited to 6 */ -#define ZSTD_SEARCHLENGTH_MIN 3 /* only for ZSTD_btopt, other strategies are limited to 4 */ -#define ZSTD_TARGETLENGTH_MIN 4 -#define ZSTD_TARGETLENGTH_MAX 999 -#define ZSTD_LDM_MINMATCH_MIN 4 +#define ZSTD_WINDOWLOG_MIN 10 +#define ZSTD_HASHLOG_MAX MIN(ZSTD_WINDOWLOG_MAX, 30) +#define ZSTD_HASHLOG_MIN 6 +#define ZSTD_CHAINLOG_MAX MIN(ZSTD_WINDOWLOG_MAX+1, 30) +#define ZSTD_CHAINLOG_MIN ZSTD_HASHLOG_MIN +#define ZSTD_HASHLOG3_MAX 17 +#define ZSTD_SEARCHLOG_MAX (ZSTD_WINDOWLOG_MAX-1) +#define ZSTD_SEARCHLOG_MIN 1 +#define ZSTD_SEARCHLENGTH_MAX 7 /* only for ZSTD_fast, other strategies are limited to 6 */ +#define ZSTD_SEARCHLENGTH_MIN 3 /* only for ZSTD_btopt, other strategies are limited to 4 */ +#define ZSTD_TARGETLENGTH_MIN 4 /* only useful for btopt */ +#define ZSTD_TARGETLENGTH_MAX 999 /* only useful for btopt */ +#define ZSTD_LDM_MINMATCH_MIN 4 #define ZSTD_LDM_MINMATCH_MAX 4096 #define ZSTD_LDM_BUCKETSIZELOG_MAX 8 diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 62af8e167..0b8995c5d 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -1322,6 +1322,7 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double else { DISPLAYUPDATE(2, "\r%6u ", testNb); } FUZ_rand(&coreSeed); lseed = coreSeed ^ prime32; + DISPLAYLEVEL(5, " *** Test %u *** \n", testNb); /* states full reset (deliberately not synchronized) */ /* some issues can only happen when reusing states */ @@ -1371,7 +1372,9 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double (MAX(testLog, dictLog) / 3))) + 1; U32 const cLevel = MIN(cLevelCandidate, cLevelMax); + DISPLAYLEVEL(5, "t%u: cLevel : %u \n", testNb, cLevel); maxTestSize = FUZ_rLogLength(&lseed, testLog); + DISPLAYLEVEL(5, "t%u: maxTestSize : %u \n", testNb, (U32)maxTestSize); oldTestLog = testLog; /* random dictionary selection */ dictSize = ((FUZ_rand(&lseed)&63)==1) ? FUZ_rLogLength(&lseed, dictLog) : 0; @@ -1396,9 +1399,16 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_windowLog, cParams.windowLog, useOpaqueAPI) ); assert(cParams.windowLog >= ZSTD_WINDOWLOG_MIN); /* guaranteed by ZSTD_adjustCParams() */ windowLogMalus = (cParams.windowLog - ZSTD_WINDOWLOG_MIN) / 5; + DISPLAYLEVEL(5, "t%u: windowLog : %u \n", testNb, cParams.windowLog); + } + if (FUZ_rand(&lseed) & 1) { + CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_hashLog, cParams.hashLog, useOpaqueAPI) ); + DISPLAYLEVEL(5, "t%u: hashLog : %u \n", testNb, cParams.hashLog); + } + if (FUZ_rand(&lseed) & 1) { + CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_chainLog, cParams.chainLog, useOpaqueAPI) ); + DISPLAYLEVEL(5, "t%u: chainLog : %u \n", testNb, cParams.chainLog); } - if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_hashLog, cParams.hashLog, useOpaqueAPI) ); - if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_chainLog, cParams.chainLog, useOpaqueAPI) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_searchLog, cParams.searchLog, useOpaqueAPI) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_minMatch, cParams.searchLength, useOpaqueAPI) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_targetLength, cParams.targetLength, useOpaqueAPI) ); @@ -1415,13 +1425,14 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_dictIDFlag, FUZ_rand(&lseed) & 1, useOpaqueAPI) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_contentSizeFlag, FUZ_rand(&lseed) & 1, useOpaqueAPI) ); if (FUZ_rand(&lseed) & 1) CHECK_Z( ZSTD_CCtx_setPledgedSrcSize(zc, pledgedSrcSize) ); - DISPLAYLEVEL(5, "pledgedSrcSize : %u \n", (U32)pledgedSrcSize); + DISPLAYLEVEL(5, "t%u: pledgedSrcSize : %u \n", testNb, (U32)pledgedSrcSize); /* multi-threading parameters */ { U32 const nbThreadsCandidate = (FUZ_rand(&lseed) & 4) + 1; U32 const nbThreadsAdjusted = (windowLogMalus < nbThreadsCandidate) ? nbThreadsCandidate - windowLogMalus : 1; U32 const nbThreads = MIN(nbThreadsAdjusted, nbThreadsMax); CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_nbThreads, nbThreads, useOpaqueAPI) ); + DISPLAYLEVEL(5, "t%u: nbThreads : %u \n", testNb, nbThreads); if (nbThreads > 1) { U32 const jobLog = FUZ_rand(&lseed) % (testLog+1); CHECK_Z( setCCtxParameter(zc, cctxParams, ZSTD_p_overlapSizeLog, FUZ_rand(&lseed) % 10, useOpaqueAPI) ); From 9fe50ed6239f86d569030744f6545feb89a6d933 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 01:42:06 -0700 Subject: [PATCH 08/19] fixed maximum windowLog for zstreamtest --newapi for compatibility with low memory VM --- tests/zstreamtest.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 0b8995c5d..3d0ab8896 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -50,7 +50,6 @@ static const U32 g_cLevelMax_smallTests = 10; #define COMPRESSIBLE_NOISE_LENGTH (10 MB) #define FUZ_COMPRESSIBILITY_DEFAULT 50 static const U32 prime32 = 2654435761U; -static const U32 windowLogMax = 27; /*-************************************ @@ -1052,8 +1051,7 @@ static int fuzzerTests_MT(U32 seed, U32 nbTests, unsigned startTest, double comp /* states full reset (deliberately not synchronized) */ /* some issues can only happen when reusing states */ if ((FUZ_rand(&lseed) & 0xFF) == 131) { - U32 const nbThreadsCandidate = (FUZ_rand(&lseed) % 6) + 1; - nbThreads = MIN(nbThreadsCandidate, nbThreadsMax); + nbThreads = (FUZ_rand(&lseed) % nbThreadsMax) + 1; DISPLAYLEVEL(5, "Creating new context with %u threads \n", nbThreads); ZSTDMT_freeCCtx(zc); zc = ZSTDMT_createCCtx(nbThreads); @@ -1369,7 +1367,7 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double U32 const dictLog = FUZ_rand(&lseed) % maxSrcLog; U32 const cLevelCandidate = (FUZ_rand(&lseed) % (ZSTD_maxCLevel() - - (MAX(testLog, dictLog) / 3))) + + (MAX(testLog, dictLog) / 2))) + 1; U32 const cLevel = MIN(cLevelCandidate, cLevelMax); DISPLAYLEVEL(5, "t%u: cLevel : %u \n", testNb, cLevel); @@ -1384,6 +1382,7 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double } { U64 const pledgedSrcSize = (FUZ_rand(&lseed) & 3) ? ZSTD_CONTENTSIZE_UNKNOWN : maxTestSize; ZSTD_compressionParameters cParams = ZSTD_getCParams(cLevel, pledgedSrcSize, dictSize); + static const U32 windowLogMax = 25; /* mess with compression parameters */ cParams.windowLog += (FUZ_rand(&lseed) & 3) - 1; From d6770f80af091f208c2bfc2f49281984b00be52b Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 02:14:48 -0700 Subject: [PATCH 09/19] minor : rewrite unit tests using CHECK_Z macro --- lib/compress/zstdmt_compress.c | 5 +- tests/zstreamtest.c | 111 ++++++++++++++------------------- 2 files changed, 49 insertions(+), 67 deletions(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index ecb799ab3..202d61b6a 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -985,7 +985,7 @@ size_t ZSTDMT_compressStream_generic(ZSTDMT_CCtx* mtctx, assert(output->pos <= output->size); assert(input->pos <= input->size); if ((mtctx->frameEnded) && (endOp==ZSTD_e_continue)) { - /* current frame being ended. Only flush/end are allowed. Or start new frame with init */ + /* current frame being ended. Only flush/end are allowed */ return ERROR(stage_wrong); } if (mtctx->params.nbThreads==1) { /* delegate to single-thread (synchronous) */ @@ -1014,7 +1014,8 @@ size_t ZSTDMT_compressStream_generic(ZSTDMT_CCtx* mtctx, if (input->size > input->pos) { /* support NULL input */ if (mtctx->inBuff.buffer.start == NULL) { mtctx->inBuff.buffer = ZSTDMT_getBuffer(mtctx->bufPool); - if (mtctx->inBuff.buffer.start == NULL) return ERROR(memory_allocation); + if (mtctx->inBuff.buffer.start == NULL) + return ERROR(memory_allocation); mtctx->inBuff.filled = 0; } { size_t const toLoad = MIN(input->size - input->pos, mtctx->inBuffSize - mtctx->inBuff.filled); diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 3d0ab8896..acaf8f962 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -96,6 +96,15 @@ unsigned int FUZ_rand(unsigned int* seedPtr) return rand32 >> 5; } +#define CHECK_Z(f) { \ + size_t const err = f; \ + if (ZSTD_isError(err)) { \ + DISPLAY("Error => %s : %s ", \ + #f, ZSTD_getErrorName(err)); \ + DISPLAY(" (seed %u, test nb %u) \n", seed, testNb); \ + goto _output_error; \ +} } + /*====================================================== * Basic Unit tests @@ -166,8 +175,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo RDG_genBuffer(CNBuffer, CNBufferSize, compressibility, 0., seed); /* Create dictionary */ - MEM_STATIC_ASSERT(COMPRESSIBLE_NOISE_LENGTH >= 4 MB); - dictionary = FUZ_createDictionary(CNBuffer, 4 MB, 4 KB, 40 KB); + dictionary = FUZ_createDictionary(CNBuffer, CNBufferSize, 4 KB, 40 KB); if (!dictionary.start) { DISPLAY("Error creating dictionary, aborting \n"); goto _output_error; @@ -181,16 +189,14 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo /* Basic compression test */ DISPLAYLEVEL(3, "test%3i : compress %u bytes : ", testNb++, COMPRESSIBLE_NOISE_LENGTH); - { size_t const r = ZSTD_initCStream_usingDict(zc, CNBuffer, dictSize, 1); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_initCStream_usingDict(zc, CNBuffer, dictSize, 1) ); outBuff.dst = (char*)(compressedBuffer)+cSize; outBuff.size = compressedBufferSize; outBuff.pos = 0; inBuff.src = CNBuffer; inBuff.size = CNBufferSize; inBuff.pos = 0; - { size_t const r = ZSTD_compressStream(zc, &outBuff, &inBuff); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ { size_t const r = ZSTD_endStream(zc, &outBuff); if (r != 0) goto _output_error; } /* error, or some data not flushed */ @@ -225,8 +231,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo /* skippable frame test */ DISPLAYLEVEL(3, "test%3i : decompress skippable frame : ", testNb++); - if (ZSTD_isError( ZSTD_initDStream_usingDict(zd, CNBuffer, dictSize) )) - goto _output_error; + CHECK_Z( ZSTD_initDStream_usingDict(zd, CNBuffer, dictSize) ); inBuff.src = compressedBuffer; inBuff.size = cSize; inBuff.pos = 0; @@ -244,8 +249,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo inBuff2 = inBuff; DISPLAYLEVEL(3, "test%3i : decompress %u bytes : ", testNb++, COMPRESSIBLE_NOISE_LENGTH); ZSTD_initDStream_usingDict(zd, CNBuffer, dictSize); - { size_t const r = ZSTD_setDStreamParameter(zd, DStream_p_maxWindowSize, 1000000000); /* large limit */ - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_setDStreamParameter(zd, DStream_p_maxWindowSize, 1000000000) ); /* large limit */ { size_t const remaining = ZSTD_decompressStream(zd, &outBuff, &inBuff); if (remaining != 0) goto _output_error; } /* should reach end of frame == 0; otherwise, some data left, or an error */ if (outBuff.pos != CNBufferSize) goto _output_error; /* should regenerate the same amount */ @@ -335,8 +339,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo inBuff.src = CNBuffer; inBuff.size = CNBufferSize; inBuff.pos = 0; - { size_t const r = ZSTD_compressStream(zc, &outBuff, &inBuff); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ { size_t const r = ZSTD_endStream(zc, &outBuff); if (r != 0) goto _output_error; } /* error, or some data not flushed */ @@ -353,8 +356,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo inBuff.src = CNBuffer; inBuff.size = CNBufferSize; inBuff.pos = 0; - { size_t const r = ZSTD_compressStream(zc, &outBuff, &inBuff); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ { size_t const r = ZSTD_endStream(zc, &outBuff); if (ZSTD_getErrorCode(r) != ZSTD_error_srcSize_wrong) goto _output_error; /* must fail : wrong srcSize */ @@ -376,8 +378,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo outBuff.size = ZSTD_compressBound(inSize); outBuff.pos = 0; DISPLAYLEVEL(5, "compress1 "); - { size_t const r = ZSTD_compressStream(zc, &outBuff, &inBuff); - if (ZSTD_isError(r)) goto _output_error; } + 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); @@ -394,8 +395,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo outBuff.size = ZSTD_compressBound(inSize); outBuff.pos = 0; DISPLAYLEVEL(5, "compress2 "); - { size_t const r = ZSTD_compressStream(zc, &outBuff, &inBuff); - if (ZSTD_isError(r)) goto _output_error; } + 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); @@ -415,8 +415,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo inBuff.src = CNBuffer; inBuff.size = CNBufferSize; inBuff.pos = 0; - { size_t const r = ZSTD_compressStream(zc, &outBuff, &inBuff); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ { size_t const r = ZSTD_endStream(zc, &outBuff); if (r != 0) goto _output_error; } /* error, or some data not flushed */ @@ -465,8 +464,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo /* Memory restriction */ DISPLAYLEVEL(3, "test%3i : maxWindowSize < frame requirement : ", testNb++); ZSTD_initDStream_usingDict(zd, CNBuffer, dictSize); - { size_t const r = ZSTD_setDStreamParameter(zd, DStream_p_maxWindowSize, 1000); /* too small limit */ - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_setDStreamParameter(zd, DStream_p_maxWindowSize, 1000) ); /* too small limit */ inBuff.src = compressedBuffer; inBuff.size = cSize; inBuff.pos = 0; @@ -490,8 +488,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo inBuff.src = CNBuffer; inBuff.size = CNBufferSize; inBuff.pos = 0; - { size_t const r = ZSTD_compressStream(zc, &outBuff, &inBuff); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ { size_t const r = ZSTD_endStream(zc, &outBuff); if (r != 0) goto _output_error; } /* error, or some data not flushed */ @@ -513,16 +510,14 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo } DISPLAYLEVEL(3, "test%3i : compress with ZSTD_CCtx_refPrefix : ", testNb++); - { size_t const refErr = ZSTD_CCtx_refPrefix(zc, dictionary.start, dictionary.filled); - if (ZSTD_isError(refErr)) goto _output_error; } + CHECK_Z( ZSTD_CCtx_refPrefix(zc, dictionary.start, dictionary.filled) ); outBuff.dst = compressedBuffer; outBuff.size = compressedBufferSize; outBuff.pos = 0; inBuff.src = CNBuffer; inBuff.size = CNBufferSize; inBuff.pos = 0; - { size_t const r = ZSTD_compress_generic(zc, &outBuff, &inBuff, ZSTD_e_end); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_compress_generic(zc, &outBuff, &inBuff, ZSTD_e_end) ); if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ cSize = outBuff.pos; DISPLAYLEVEL(3, "OK (%u bytes : %.2f%%)\n", (U32)cSize, (double)cSize/CNBufferSize*100); @@ -549,23 +544,20 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo inBuff.src = CNBuffer; inBuff.size = CNBufferSize; inBuff.pos = 0; - { size_t const r = ZSTD_compress_generic(zc, &outBuff, &inBuff, ZSTD_e_end); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTD_compress_generic(zc, &outBuff, &inBuff, ZSTD_e_end) ); if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ cSize = outBuff.pos; DISPLAYLEVEL(3, "OK (%u bytes : %.2f%%)\n", (U32)cSize, (double)cSize/CNBufferSize*100); DISPLAYLEVEL(3, "test%3i : decompress without dictionary (should work): ", testNb++); - { size_t const r = ZSTD_decompress(decodedBuffer, CNBufferSize, compressedBuffer, cSize); - if (ZSTD_isError(r)) goto _output_error; /* must fail : dictionary not used */ - DISPLAYLEVEL(3, "OK \n"); - } + CHECK_Z( ZSTD_decompress(decodedBuffer, CNBufferSize, compressedBuffer, cSize) ); + DISPLAYLEVEL(3, "OK \n"); /* Empty srcSize */ DISPLAYLEVEL(3, "test%3i : ZSTD_initCStream_advanced with pledgedSrcSize=0 and dict : ", testNb++); { ZSTD_parameters params = ZSTD_getParams(5, 0, 0); params.fParams.contentSizeFlag = 1; - ZSTD_initCStream_advanced(zc, dictionary.start, dictionary.filled, params, 0); + CHECK_Z( ZSTD_initCStream_advanced(zc, dictionary.start, dictionary.filled, params, 0) ); } /* cstream advanced shall write content size = 0 */ inBuff.src = CNBuffer; inBuff.size = 0; @@ -573,7 +565,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo outBuff.dst = compressedBuffer; outBuff.size = compressedBufferSize; outBuff.pos = 0; - if (ZSTD_isError(ZSTD_compressStream(zc, &outBuff, &inBuff))) goto _output_error; + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); if (ZSTD_endStream(zc, &outBuff) != 0) goto _output_error; cSize = outBuff.pos; if (ZSTD_findDecompressedSize(compressedBuffer, cSize) != 0) goto _output_error; @@ -582,7 +574,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo DISPLAYLEVEL(3, "test%3i : pledgedSrcSize == 0 behaves properly : ", testNb++); { ZSTD_parameters params = ZSTD_getParams(5, 0, 0); params.fParams.contentSizeFlag = 1; - ZSTD_initCStream_advanced(zc, NULL, 0, params, 0); + CHECK_Z( ZSTD_initCStream_advanced(zc, NULL, 0, params, 0) ); } /* cstream advanced shall write content size = 0 */ inBuff.src = CNBuffer; inBuff.size = 0; @@ -590,7 +582,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo outBuff.dst = compressedBuffer; outBuff.size = compressedBufferSize; outBuff.pos = 0; - if (ZSTD_isError(ZSTD_compressStream(zc, &outBuff, &inBuff))) goto _output_error; + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); if (ZSTD_endStream(zc, &outBuff) != 0) goto _output_error; cSize = outBuff.pos; if (ZSTD_findDecompressedSize(compressedBuffer, cSize) != 0) goto _output_error; @@ -602,7 +594,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo outBuff.dst = compressedBuffer; outBuff.size = compressedBufferSize; outBuff.pos = 0; - if (ZSTD_isError(ZSTD_compressStream(zc, &outBuff, &inBuff))) goto _output_error; + CHECK_Z( ZSTD_compressStream(zc, &outBuff, &inBuff) ); if (ZSTD_endStream(zc, &outBuff) != 0) goto _output_error; cSize = outBuff.pos; if (ZSTD_findDecompressedSize(compressedBuffer, cSize) != ZSTD_CONTENTSIZE_UNKNOWN) goto _output_error; @@ -610,17 +602,16 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo /* Basic multithreading compression test */ DISPLAYLEVEL(3, "test%3i : compress %u bytes with multiple threads : ", testNb++, COMPRESSIBLE_NOISE_LENGTH); - { ZSTD_parameters const params = ZSTD_getParams(1, 0, 0); - size_t const r = ZSTDMT_initCStream_advanced(mtctx, CNBuffer, dictSize, params, CNBufferSize); - if (ZSTD_isError(r)) goto _output_error; } + { ZSTD_parameters const params = ZSTD_getParams(1, 0, 0); + CHECK_Z( ZSTDMT_initCStream_advanced(mtctx, CNBuffer, dictSize, params, CNBufferSize) ); + } outBuff.dst = (char*)(compressedBuffer); outBuff.size = compressedBufferSize; outBuff.pos = 0; inBuff.src = CNBuffer; inBuff.size = CNBufferSize; inBuff.pos = 0; - { size_t const r = ZSTDMT_compressStream_generic(mtctx, &outBuff, &inBuff, ZSTD_e_end); - if (ZSTD_isError(r)) goto _output_error; } + CHECK_Z( ZSTDMT_compressStream_generic(mtctx, &outBuff, &inBuff, ZSTD_e_end) ); if (inBuff.pos != inBuff.size) goto _output_error; /* entire input should be consumed */ { size_t const r = ZSTDMT_endStream(mtctx, &outBuff); if (r != 0) goto _output_error; } /* error, or some data not flushed */ @@ -641,9 +632,10 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo "\x28\xB5\x2F\xFD\x04\x00\x4C\x00\x00\x10\x61\x61\x01\x00\x00\x2A" "\x80\x05\x44\x00\x00\x08\x62\x01\x00\x00\x2A\x20\x04\x5D\x00\x00" "\x00\x03\x40\x00\x00\x64\x60\x27\xB0\xE0\x0C\x67\x62\xCE\xE0"; - ZSTD_DStream* zds = ZSTD_createDStream(); + ZSTD_DStream* const zds = ZSTD_createDStream(); + if (zds==NULL) goto _output_error; - ZSTD_initDStream(zds); + CHECK_Z( ZSTD_initDStream(zds) ); inBuff.src = testCase; inBuff.size = 47; inBuff.pos = 0; @@ -652,9 +644,7 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo outBuff.pos = 0; while (inBuff.pos < inBuff.size) { - size_t const r = ZSTD_decompressStream(zds, &outBuff, &inBuff); - /* Bug will cause checksum to fail */ - if (ZSTD_isError(r)) goto _output_error; + CHECK_Z( ZSTD_decompressStream(zds, &outBuff, &inBuff) ); } ZSTD_freeDStream(zds); @@ -725,15 +715,6 @@ static U32 FUZ_randomClampedLength(U32* seed, U32 minVal, U32 maxVal) goto _output_error; \ } } -#define CHECK_Z(f) { \ - size_t const err = f; \ - if (ZSTD_isError(err)) { \ - DISPLAY("Error => %s : %s ", \ - #f, ZSTD_getErrorName(err)); \ - DISPLAY(" (seed %u, test nb %u) \n", seed, testNb); \ - goto _output_error; \ -} } - static int fuzzerTests(U32 seed, U32 nbTests, unsigned startTest, double compressibility, int bigTests) { U32 const maxSrcLog = bigTests ? 24 : 22; @@ -1482,7 +1463,7 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double outBuff.size = outBuff.pos + dstBuffSize; CHECK_Z( ZSTD_compress_generic(zc, &outBuff, &inBuff, flush) ); - DISPLAYLEVEL(5, "compress consumed %u bytes (total : %u) \n", + DISPLAYLEVEL(6, "compress consumed %u bytes (total : %u) \n", (U32)inBuff.pos, (U32)(totalTestSize + inBuff.pos)); XXH64_update(&xxhState, srcBuffer+srcStart, inBuff.pos); @@ -1497,11 +1478,11 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double size_t const randomDstSize = FUZ_randomLength(&lseed, maxSampleLog+1); size_t const adjustedDstSize = MIN(cBufferSize - cSize, randomDstSize); outBuff.size = outBuff.pos + adjustedDstSize; - DISPLAYLEVEL(5, "End-flush into dst buffer of size %u \n", (U32)adjustedDstSize); + DISPLAYLEVEL(6, "End-flush into dst buffer of size %u \n", (U32)adjustedDstSize); remainingToFlush = ZSTD_compress_generic(zc, &outBuff, &inBuff, ZSTD_e_end); - CHECK(ZSTD_isError(remainingToFlush), - "ZSTD_compress_generic w/ ZSTD_e_end error : %s", - ZSTD_getErrorName(remainingToFlush) ); + CHECK( ZSTD_isError(remainingToFlush), + "ZSTD_compress_generic w/ ZSTD_e_end error : %s", + ZSTD_getErrorName(remainingToFlush) ); } } crcOrig = XXH64_digest(&xxhState); cSize = outBuff.pos; @@ -1525,11 +1506,11 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double size_t const dstBuffSize = MIN(dstBufferSize - totalGenSize, randomDstSize); inBuff.size = inBuff.pos + readCSrcSize; outBuff.size = inBuff.pos + dstBuffSize; - DISPLAYLEVEL(5, "ZSTD_decompressStream input %u bytes (pos:%u/%u)\n", + DISPLAYLEVEL(6, "ZSTD_decompressStream input %u bytes (pos:%u/%u)\n", (U32)readCSrcSize, (U32)inBuff.pos, (U32)cSize); decompressionResult = ZSTD_decompressStream(zd, &outBuff, &inBuff); CHECK (ZSTD_isError(decompressionResult), "decompression error : %s", ZSTD_getErrorName(decompressionResult)); - DISPLAYLEVEL(5, "inBuff.pos = %u \n", (U32)readCSrcSize); + DISPLAYLEVEL(6, "inBuff.pos = %u \n", (U32)readCSrcSize); } CHECK (outBuff.pos != totalTestSize, "decompressed data : wrong size (%u != %u)", (U32)outBuff.pos, (U32)totalTestSize); CHECK (inBuff.pos != cSize, "compressed data should be fully read (%u != %u)", (U32)inBuff.pos, (U32)cSize); From 377abcc02c9db9cec4f348a04a1caaa45cfa13a7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 02:23:44 -0700 Subject: [PATCH 10/19] zstdmt : better behavior when freeing a context right after a memory allocation error wait for all jobs to be completed, so that freeing can happen safely --- lib/compress/zstdmt_compress.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index 202d61b6a..5001f4a00 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -1014,8 +1014,10 @@ size_t ZSTDMT_compressStream_generic(ZSTDMT_CCtx* mtctx, if (input->size > input->pos) { /* support NULL input */ if (mtctx->inBuff.buffer.start == NULL) { mtctx->inBuff.buffer = ZSTDMT_getBuffer(mtctx->bufPool); - if (mtctx->inBuff.buffer.start == NULL) + if (mtctx->inBuff.buffer.start == NULL) { + ZSTDMT_waitForAllJobsCompleted(mtctx); return ERROR(memory_allocation); + } mtctx->inBuff.filled = 0; } { size_t const toLoad = MIN(input->size - input->pos, mtctx->inBuffSize - mtctx->inBuff.filled); From 2cd15dd9a426a4dea70c4dac100410efbc81fbc4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 02:33:41 -0700 Subject: [PATCH 11/19] fixed minor Visual conversion warning --- lib/compress/zstd_compress.c | 2 +- lib/compress/zstdmt_compress.c | 35 ++++++++++++++++++---------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 0dbe196ff..eff6c57c6 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -640,7 +640,7 @@ ZSTD_compressionParameters ZSTD_adjustCParams_internal(ZSTD_compressionParameter assert(ZSTD_checkCParams(cPar)==0); /* resize windowLog if src is small, to use less memory when necessary */ - ZSTD_STATIC_ASSERT(ZSTD_CONTENTSIZE_UNKNOWN == -1ULL); + ZSTD_STATIC_ASSERT(ZSTD_CONTENTSIZE_UNKNOWN == (0ULL - 1)); if ( (dictSize || (srcSize+1 > 1)) /* srcSize test depends on static assert condition */ && (srcSize-1 < (1ULL<allJobsCompleted = 1; } +static void ZSTDMT_waitForAllJobsCompleted(ZSTDMT_CCtx* zcs) +{ + DEBUGLOG(4, "ZSTDMT_waitForAllJobsCompleted"); + while (zcs->doneJobID < zcs->nextJobID) { + unsigned const jobID = zcs->doneJobID & zcs->jobIDMask; + PTHREAD_MUTEX_LOCK(&zcs->jobCompleted_mutex); + while (zcs->jobs[jobID].jobCompleted==0) { + DEBUGLOG(5, "waiting for jobCompleted signal from chunk %u", zcs->doneJobID); /* we want to block when waiting for data to flush */ + pthread_cond_wait(&zcs->jobCompleted_cond, &zcs->jobCompleted_mutex); + } + pthread_mutex_unlock(&zcs->jobCompleted_mutex); + zcs->doneJobID++; + } +} + size_t ZSTDMT_freeCCtx(ZSTDMT_CCtx* mtctx) { if (mtctx==NULL) return 0; /* compatible with free on NULL */ POOL_free(mtctx->factory); - if (!mtctx->allJobsCompleted) ZSTDMT_releaseAllJobResources(mtctx); /* stop workers first */ + if (!mtctx->allJobsCompleted) { + ZSTDMT_waitForAllJobsCompleted(mtctx); + ZSTDMT_releaseAllJobResources(mtctx); /* stop workers first */ + } ZSTDMT_freeBufferPool(mtctx->bufPool); /* release job resources into pools first */ ZSTD_free(mtctx->jobs, mtctx->cMem); ZSTDMT_freeCCtxPool(mtctx->cctxPool); @@ -724,21 +742,6 @@ size_t ZSTDMT_compressCCtx(ZSTDMT_CCtx* mtctx, /* ======= Streaming API ======= */ /* ====================================== */ -static void ZSTDMT_waitForAllJobsCompleted(ZSTDMT_CCtx* zcs) -{ - DEBUGLOG(4, "ZSTDMT_waitForAllJobsCompleted"); - while (zcs->doneJobID < zcs->nextJobID) { - unsigned const jobID = zcs->doneJobID & zcs->jobIDMask; - PTHREAD_MUTEX_LOCK(&zcs->jobCompleted_mutex); - while (zcs->jobs[jobID].jobCompleted==0) { - DEBUGLOG(5, "waiting for jobCompleted signal from chunk %u", zcs->doneJobID); /* we want to block when waiting for data to flush */ - pthread_cond_wait(&zcs->jobCompleted_cond, &zcs->jobCompleted_mutex); - } - pthread_mutex_unlock(&zcs->jobCompleted_mutex); - zcs->doneJobID++; - } -} - size_t ZSTDMT_initCStream_internal( ZSTDMT_CCtx* zcs, const void* dict, size_t dictSize, ZSTD_dictMode_e dictMode, From 8074261d00045ca7f677d0750ac5c01d5a13f9f9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 11:46:19 -0700 Subject: [PATCH 12/19] zstdmt : move on when not enough memory for a new input buffer just continue operations without input forward progress, instead of an error that stops current compression session. --- lib/compress/zstd_compress.c | 2 +- lib/compress/zstdmt_compress.c | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index eff6c57c6..117a1f58b 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -642,7 +642,7 @@ ZSTD_compressionParameters ZSTD_adjustCParams_internal(ZSTD_compressionParameter /* resize windowLog if src is small, to use less memory when necessary */ ZSTD_STATIC_ASSERT(ZSTD_CONTENTSIZE_UNKNOWN == (0ULL - 1)); if ( (dictSize || (srcSize+1 > 1)) /* srcSize test depends on static assert condition */ - && (srcSize-1 < (1ULL<dictSize + mtctx->targetSectionSize; + unsigned forwardInputProgress = 0; assert(output->pos <= output->size); assert(input->pos <= input->size); if ((mtctx->frameEnded) && (endOp==ZSTD_e_continue)) { @@ -995,10 +996,10 @@ size_t ZSTDMT_compressStream_generic(ZSTDMT_CCtx* mtctx, return ZSTD_compressStream_generic(mtctx->cctxPool->cctx[0], output, input, endOp); } - /* single-pass shortcut (note : this is synchronous-mode) */ - if ( (mtctx->nextJobID==0) /* just started */ - && (mtctx->inBuff.filled==0) /* nothing buffered */ - && (endOp==ZSTD_e_end) /* end order */ + /* single-pass shortcut (note : synchronous-mode) */ + if ( (mtctx->nextJobID == 0) /* just started */ + && (mtctx->inBuff.filled == 0) /* nothing buffered */ + && (endOp == ZSTD_e_end) /* end order */ && (output->size - output->pos >= ZSTD_compressBound(input->size - input->pos)) ) { /* enough room */ size_t const cSize = ZSTDMT_compress_advanced_internal(mtctx, (char*)output->dst + output->pos, output->size - output->pos, @@ -1016,18 +1017,16 @@ size_t ZSTDMT_compressStream_generic(ZSTDMT_CCtx* mtctx, /* fill input buffer */ if (input->size > input->pos) { /* support NULL input */ if (mtctx->inBuff.buffer.start == NULL) { - mtctx->inBuff.buffer = ZSTDMT_getBuffer(mtctx->bufPool); - if (mtctx->inBuff.buffer.start == NULL) { - ZSTDMT_waitForAllJobsCompleted(mtctx); - return ERROR(memory_allocation); - } + mtctx->inBuff.buffer = ZSTDMT_getBuffer(mtctx->bufPool); /* note : may fail, in which case, no forward input progress */ mtctx->inBuff.filled = 0; } - { size_t const toLoad = MIN(input->size - input->pos, mtctx->inBuffSize - mtctx->inBuff.filled); + if (mtctx->inBuff.buffer.start) { + size_t const toLoad = MIN(input->size - input->pos, mtctx->inBuffSize - mtctx->inBuff.filled); DEBUGLOG(5, "inBuff:%08X; inBuffSize=%u; ToCopy=%u", (U32)(size_t)mtctx->inBuff.buffer.start, (U32)mtctx->inBuffSize, (U32)toLoad); memcpy((char*)mtctx->inBuff.buffer.start + mtctx->inBuff.filled, (const char*)input->src + input->pos, toLoad); input->pos += toLoad; mtctx->inBuff.filled += toLoad; + forwardInputProgress = toLoad>0; } } if ( (mtctx->inBuff.filled >= newJobThreshold) /* filled enough : let's compress */ @@ -1036,7 +1035,7 @@ size_t ZSTDMT_compressStream_generic(ZSTDMT_CCtx* mtctx, } /* check for potential compressed data ready to be flushed */ - CHECK_F( ZSTDMT_flushNextJob(mtctx, output, (mtctx->inBuff.filled == mtctx->inBuffSize) /* blockToFlush */) ); /* block if it wasn't possible to create new job due to saturation */ + CHECK_F( ZSTDMT_flushNextJob(mtctx, output, !forwardInputProgress /* blockToFlush */) ); /* block if there was no forward input progress */ if (input->pos < input->size) /* input not consumed : do not flush yet */ endOp = ZSTD_e_continue; From bbef058ae6f2e931c9af59b6da4f1f666775fb23 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 11:48:45 -0700 Subject: [PATCH 13/19] zstreamtest --newapi : reduced maximum allocated memory --- tests/zstreamtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index acaf8f962..f69cdd646 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -1363,7 +1363,7 @@ static int fuzzerTests_newAPI(U32 seed, U32 nbTests, unsigned startTest, double } { U64 const pledgedSrcSize = (FUZ_rand(&lseed) & 3) ? ZSTD_CONTENTSIZE_UNKNOWN : maxTestSize; ZSTD_compressionParameters cParams = ZSTD_getCParams(cLevel, pledgedSrcSize, dictSize); - static const U32 windowLogMax = 25; + static const U32 windowLogMax = 24; /* mess with compression parameters */ cParams.windowLog += (FUZ_rand(&lseed) & 3) - 1; From 86b4fe5b45458c65f26762fb1de2803056e3d37e Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 18:14:28 -0700 Subject: [PATCH 14/19] adjustCParams : restored previous behavior unknowns srcSize presumed small if there is a dictionary (dictSize>0) and presumed large otherwise. --- lib/common/pool.c | 10 +++++----- lib/compress/zstd_compress.c | 33 ++++++++++++++++++--------------- 2 files changed, 23 insertions(+), 20 deletions(-) diff --git a/lib/common/pool.c b/lib/common/pool.c index 9e6f802e8..1b0fe1035 100644 --- a/lib/common/pool.c +++ b/lib/common/pool.c @@ -99,16 +99,16 @@ static void* POOL_thread(void* opaque) { /* Unreachable */ } -POOL_ctx *POOL_create(size_t numThreads, size_t queueSize) { +POOL_ctx* POOL_create(size_t numThreads, size_t queueSize) { return POOL_create_advanced(numThreads, queueSize, ZSTD_defaultCMem); } -POOL_ctx *POOL_create_advanced(size_t numThreads, size_t queueSize, ZSTD_customMem customMem) { - POOL_ctx *ctx; +POOL_ctx* POOL_create_advanced(size_t numThreads, size_t queueSize, ZSTD_customMem customMem) { + POOL_ctx* ctx; /* Check the parameters */ if (!numThreads) { return NULL; } /* Allocate the context and zero initialize */ - ctx = (POOL_ctx *)ZSTD_calloc(sizeof(POOL_ctx), customMem); + ctx = (POOL_ctx*)ZSTD_calloc(sizeof(POOL_ctx), customMem); if (!ctx) { return NULL; } /* Initialize the job queue. * It needs one extra space since one space is wasted to differentiate empty @@ -146,7 +146,7 @@ POOL_ctx *POOL_create_advanced(size_t numThreads, size_t queueSize, ZSTD_customM /*! POOL_join() : Shutdown the queue, wake any sleeping threads, and join all of the threads. */ -static void POOL_join(POOL_ctx *ctx) { +static void POOL_join(POOL_ctx* ctx) { /* Shut down the queue */ ZSTD_pthread_mutex_lock(&ctx->queueMutex); ctx->shutdown = 1; diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 7e2dbffd2..4c2254abe 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -640,26 +640,29 @@ static U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat) /** ZSTD_adjustCParams_internal() : optimize `cPar` for a given input (`srcSize` and `dictSize`). - mostly downsizing to reduce memory consumption and initialization. - Both `srcSize` and `dictSize` are optional (use 0 if unknown), - but if both are 0, no optimization can be done. + mostly downsizing to reduce memory consumption and initialization latency. + Both `srcSize` and `dictSize` are optional (use 0 if unknown). Note : cPar is considered validated at this stage. Use ZSTD_checkCParams() to ensure that condition. */ ZSTD_compressionParameters ZSTD_adjustCParams_internal(ZSTD_compressionParameters cPar, unsigned long long srcSize, size_t dictSize) { + static const U64 minSrcSize = 513; /* (1<<9) + 1 */ + static const U64 maxWindowResize = 1ULL << (ZSTD_WINDOWLOG_MAX-1); assert(ZSTD_checkCParams(cPar)==0); - /* resize windowLog if src is small, to use less memory when necessary */ - ZSTD_STATIC_ASSERT(ZSTD_CONTENTSIZE_UNKNOWN == (0ULL - 1)); - if ( (dictSize || (srcSize+1 > 1)) /* srcSize test depends on static assert condition */ - && (srcSize-1 < (1ULL< srcLog) cPar.windowLog = srcLog; - } } + if (dictSize && (srcSize+1<2) /* srcSize unknown */ ) + srcSize = minSrcSize; /* presumed small when there is a dictionary */ + else + srcSize -= 1; /* unknown 0 => -1ULL : presumed large */ + + /* resize windowLog if input is small enough, to use less memory */ + if ( (srcSize < maxWindowResize) + && (dictSize < maxWindowResize) ) { + U32 const tSize = (U32)(srcSize + dictSize); + static U32 const hashSizeMin = 1 << ZSTD_HASHLOG_MIN; + U32 const srcLog = (tSize < hashSizeMin) ? ZSTD_HASHLOG_MIN : + ZSTD_highbit32(tSize-1) + 1; + if (cPar.windowLog > srcLog) cPar.windowLog = srcLog; + } if (cPar.hashLog > cPar.windowLog) cPar.hashLog = cPar.windowLog; { U32 const cycleLog = ZSTD_cycleLog(cPar.chainLog, cPar.strategy); if (cycleLog > cPar.windowLog) From 47c6a95d0786edad1910a62b620babe739fe218f Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 18:27:22 -0700 Subject: [PATCH 15/19] zstreamtest : run unit tests only during "normal" session not during --mt, --newapi and --opaque this avoids running them 4x during `make test` --- tests/zstreamtest.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 6ae4ef96c..16f514425 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -1614,9 +1614,9 @@ int main(int argc, const char** argv) /* Parsing commands. Aggregated commands are allowed */ if (argument[0]=='-') { - if (!strcmp(argument, "--mt")) { selected_api=mt_api; continue; } - if (!strcmp(argument, "--newapi")) { selected_api=advanced_api; continue; } - if (!strcmp(argument, "--opaqueapi")) { selected_api=advanced_api; useOpaqueAPI = 1; continue; } + if (!strcmp(argument, "--mt")) { selected_api=mt_api; testNb += !testNb; continue; } + if (!strcmp(argument, "--newapi")) { selected_api=advanced_api; testNb += !testNb; continue; } + if (!strcmp(argument, "--opaqueapi")) { selected_api=advanced_api; testNb += !testNb; useOpaqueAPI = 1; continue; } if (!strcmp(argument, "--no-big-tests")) { bigTests=0; continue; } argument++; From e0065cf660cdc1b66f8cb9bae65fac9f8ad63797 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 18:34:38 -0700 Subject: [PATCH 16/19] make test : removed zstreamtest unit tests for variants slightly reduced time to create dictionary at beginning of unit tests --- tests/Makefile | 6 +++--- tests/zstreamtest.c | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index fc62b9e26..95c33bc4b 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -316,9 +316,9 @@ test-zbuff32: zbufftest32 test-zstream: zstreamtest $(QEMU_SYS) ./zstreamtest $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) - $(QEMU_SYS) ./zstreamtest --mt $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) - $(QEMU_SYS) ./zstreamtest --newapi $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) - $(QEMU_SYS) ./zstreamtest --opaqueapi $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) + $(QEMU_SYS) ./zstreamtest --mt -t1 $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) + $(QEMU_SYS) ./zstreamtest --newapi -t1 $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) + $(QEMU_SYS) ./zstreamtest --opaqueapi -t1 $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) test-zstream32: zstreamtest32 $(QEMU_SYS) ./zstreamtest32 $(ZSTREAM_TESTTIME) $(FUZZER_FLAGS) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 16f514425..8b4c83694 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -175,7 +175,8 @@ static int basicUnitTests(U32 seed, double compressibility, ZSTD_customMem custo RDG_genBuffer(CNBuffer, CNBufferSize, compressibility, 0., seed); /* Create dictionary */ - dictionary = FUZ_createDictionary(CNBuffer, CNBufferSize, 4 KB, 40 KB); + DISPLAYLEVEL(3, "creating dictionary for unit tests \n"); + dictionary = FUZ_createDictionary(CNBuffer, CNBufferSize / 2, 8 KB, 40 KB); if (!dictionary.start) { DISPLAY("Error creating dictionary, aborting \n"); goto _output_error; From 754ae5cc0bc47e96bc30163c007295859dc04c29 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 20:44:22 -0700 Subject: [PATCH 17/19] removed ZSTDMT_waitForAllJobsCompleted() from ZSTDMT_freeCCtx() as per @terrelln comment --- lib/compress/zstdmt_compress.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index bc7d1e540..bfa1c42da 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -514,13 +514,10 @@ static void ZSTDMT_waitForAllJobsCompleted(ZSTDMT_CCtx* zcs) size_t ZSTDMT_freeCCtx(ZSTDMT_CCtx* mtctx) { if (mtctx==NULL) return 0; /* compatible with free on NULL */ - POOL_free(mtctx->factory); - if (!mtctx->allJobsCompleted) { - ZSTDMT_waitForAllJobsCompleted(mtctx); - ZSTDMT_releaseAllJobResources(mtctx); /* stop workers first */ - } - ZSTDMT_freeBufferPool(mtctx->bufPool); /* release job resources into pools first */ + POOL_free(mtctx->factory); /* stop and free worker threads */ + ZSTDMT_releaseAllJobResources(mtctx); /* release job resources into pools first */ ZSTD_free(mtctx->jobs, mtctx->cMem); + ZSTDMT_freeBufferPool(mtctx->bufPool); ZSTDMT_freeCCtxPool(mtctx->cctxPool); ZSTD_freeCDict(mtctx->cdictLocal); ZSTD_pthread_mutex_destroy(&mtctx->jobCompleted_mutex); From e963800e27ee85354bfba462685fc2cd0c2ac819 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 28 Sep 2017 23:01:31 -0700 Subject: [PATCH 18/19] zstdmt : fixed : buffer dst0 wasn't properly set to null after usage now it's possible to unconditionnally invoke ZSTD_releaseAllJobRessources() wether previous compression was completed correctly or not. --- lib/compress/zstdmt_compress.c | 13 +++++++++---- tests/fuzzer.c | 3 +-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index bfa1c42da..03871421c 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -115,9 +115,12 @@ static ZSTDMT_bufferPool* ZSTDMT_createBufferPool(unsigned nbThreads, ZSTD_custo static void ZSTDMT_freeBufferPool(ZSTDMT_bufferPool* bufPool) { unsigned u; + DEBUGLOG(3, "ZSTDMT_freeBufferPool (address:%08X)", (U32)bufPool); if (!bufPool) return; /* compatibility with free on NULL */ - for (u=0; utotalBuffers; u++) + for (u=0; utotalBuffers; u++) { + DEBUGLOG(4, "free buffer %2u (address:%08X)", u, (U32)bufPool->bTable[u].start); ZSTD_free(bufPool->bTable[u].start, bufPool->cMem); + } ZSTD_pthread_mutex_destroy(&bufPool->poolMutex); ZSTD_free(bufPool, bufPool->cMem); } @@ -485,12 +488,15 @@ static void ZSTDMT_releaseAllJobResources(ZSTDMT_CCtx* mtctx) unsigned jobID; DEBUGLOG(3, "ZSTDMT_releaseAllJobResources"); for (jobID=0; jobID <= mtctx->jobIDMask; jobID++) { + DEBUGLOG(4, "job%02u: release dst address %08X", jobID, (U32)mtctx->jobs[jobID].dstBuff.start); ZSTDMT_releaseBuffer(mtctx->bufPool, mtctx->jobs[jobID].dstBuff); mtctx->jobs[jobID].dstBuff = g_nullBuffer; + DEBUGLOG(4, "job%02u: release src address %08X", jobID, (U32)mtctx->jobs[jobID].src.start); ZSTDMT_releaseBuffer(mtctx->bufPool, mtctx->jobs[jobID].src); mtctx->jobs[jobID].src = g_nullBuffer; } memset(mtctx->jobs, 0, (mtctx->jobIDMask+1)*sizeof(ZSTDMT_jobDescription)); + DEBUGLOG(4, "input: release address %08X", (U32)mtctx->inBuff.buffer.start); ZSTDMT_releaseBuffer(mtctx->bufPool, mtctx->inBuff.buffer); mtctx->inBuff.buffer = g_nullBuffer; mtctx->allJobsCompleted = 1; @@ -684,9 +690,8 @@ static size_t ZSTDMT_compress_advanced_internal( if (chunkID >= compressWithinDst) { /* chunk compressed into its own buffer, which must be released */ DEBUGLOG(5, "releasing buffer %u>=%u", chunkID, compressWithinDst); ZSTDMT_releaseBuffer(mtctx->bufPool, mtctx->jobs[chunkID].dstBuff); - } - mtctx->jobs[chunkID].dstBuff = g_nullBuffer; - } + } } + mtctx->jobs[chunkID].dstBuff = g_nullBuffer; dstPos += cSize ; } } /* for (chunkID=0; chunkID Date: Fri, 29 Sep 2017 16:27:47 -0700 Subject: [PATCH 19/19] erase existence of a buffer when it's sent out of the pool In some complex scenario, the buffer would be freed because it's too large, another buffer would be allocated, but fail, trigger an error, and the general buffer pool would then be freed, where the definition of the already freed buffer would be found (beyond total index, but still), and freed again, resulting in double-free error. --- lib/compress/zstdmt_compress.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index 03871421c..2d4fe2573 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -155,6 +155,7 @@ static buffer_t ZSTDMT_getBuffer(ZSTDMT_bufferPool* bufPool) if (bufPool->nbBuffers) { /* try to use an existing buffer */ buffer_t const buf = bufPool->bTable[--(bufPool->nbBuffers)]; size_t const availBufferSize = buf.size; + bufPool->bTable[bufPool->nbBuffers] = g_nullBuffer; if ((availBufferSize >= bSize) & (availBufferSize <= 10*bSize)) { /* large enough, but not too much */ ZSTD_pthread_mutex_unlock(&bufPool->poolMutex);