From d5986f235f6fbe4f72aabb943cf6d4b9fe51772f Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 10 Mar 2025 00:12:34 -0700 Subject: [PATCH 1/5] fix #4332: setting ZSTD_NBTHREADS=0 via environment variables --- programs/zstdcli.c | 55 ++++++++----------- tests/cli-tests/compression/multi-threaded.sh | 4 -- .../multi-threaded.sh.stderr.exact | 5 -- 3 files changed, 24 insertions(+), 40 deletions(-) diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 66e9d064b..30a406a7d 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -44,7 +44,7 @@ #endif #ifndef ZSTDCLI_NBTHREADS_DEFAULT -#define ZSTDCLI_NBTHREADS_DEFAULT MAX(1, MIN(4, UTIL_countLogicalCores() / 4)) +#define ZSTDCLI_NBTHREADS_DEFAULT (unsigned)(MAX(1, MIN(4, UTIL_countLogicalCores() / 4))) #endif @@ -94,6 +94,7 @@ static U32 g_ldmBucketSizeLog = LDM_PARAM_DEFAULT; #define DEFAULT_ACCEL 1 +#define NBWORKERS_AUTOCPU 0 typedef enum { cover, fastCover, legacy } dictType; @@ -745,7 +746,7 @@ static void printActualCParams(const char* filename, const char* dictFileName, i /* Environment variables for parameter setting */ #define ENV_CLEVEL "ZSTD_CLEVEL" -#define ENV_NBTHREADS "ZSTD_NBTHREADS" /* takes lower precedence than directly specifying -T# in the CLI */ +#define ENV_NBWORKERS "ZSTD_NBTHREADS" /* takes lower precedence than directly specifying -T# in the CLI */ /* pick up environment variable */ static int init_cLevel(void) { @@ -775,26 +776,28 @@ static int init_cLevel(void) { return ZSTDCLI_CLEVEL_DEFAULT; } +static unsigned init_nbWorkers(void) { #ifdef ZSTD_MULTITHREAD -static int default_nbThreads(void) { - const char* const env = getenv(ENV_NBTHREADS); + const char* const env = getenv(ENV_NBWORKERS); if (env != NULL) { const char* ptr = env; if ((*ptr>='0') && (*ptr<='9')) { unsigned nbThreads; if (readU32FromCharChecked(&ptr, &nbThreads)) { - DISPLAYLEVEL(2, "Ignore environment variable setting %s=%s: numeric value too large \n", ENV_NBTHREADS, env); + DISPLAYLEVEL(2, "Ignore environment variable setting %s=%s: numeric value too large \n", ENV_NBWORKERS, env); return ZSTDCLI_NBTHREADS_DEFAULT; } else if (*ptr == 0) { - return (int)nbThreads; + return nbThreads; } } - DISPLAYLEVEL(2, "Ignore environment variable setting %s=%s: not a valid unsigned value \n", ENV_NBTHREADS, env); + DISPLAYLEVEL(2, "Ignore environment variable setting %s=%s: not a valid unsigned value \n", ENV_NBWORKERS, env); } return ZSTDCLI_NBTHREADS_DEFAULT; -} +#else + return 1; #endif +} #define NEXT_FIELD(ptr) { \ if (*argument == '=') { \ @@ -874,13 +877,15 @@ int main(int argCount, const char* argv[]) singleThread = 0, defaultLogicalCores = 0, showDefaultCParams = 0, - ultra=0, - contentSize=1, - removeSrcFile=0; - ZSTD_ParamSwitch_e mmapDict=ZSTD_ps_auto; + contentSize = 1, + removeSrcFile = 0, + cLevel = init_cLevel(), + ultra = 0, + cLevelLast = MINCLEVEL - 1; /* for benchmark range */ + unsigned nbWorkers = init_nbWorkers(); + ZSTD_ParamSwitch_e mmapDict = ZSTD_ps_auto; ZSTD_ParamSwitch_e useRowMatchFinder = ZSTD_ps_auto; FIO_compressionType_t cType = FIO_zstdCompression; - int nbWorkers = -1; /* -1 means unset */ double compressibility = -1.0; /* lorem ipsum generator */ unsigned bench_nbSeconds = 3; /* would be better if this value was synchronized from bench */ size_t chunkSize = 0; @@ -890,8 +895,6 @@ int main(int argCount, const char* argv[]) FIO_progressSetting_e progress = FIO_ps_auto; zstd_operation_mode operation = zom_compress; ZSTD_compressionParameters compressionParams; - int cLevel = init_cLevel(); - int cLevelLast = MINCLEVEL - 1; /* lower than minimum */ unsigned recursive = 0; unsigned memLimit = 0; FileNamesTable* filenames = UTIL_allocateFileNamesTable((size_t)argCount); /* argCount >= 1 */ @@ -930,7 +933,7 @@ int main(int argCount, const char* argv[]) programName = lastNameFromPath(programName); /* preset behaviors */ - if (exeNameMatch(programName, ZSTD_ZSTDMT)) nbWorkers=0, singleThread=0; + if (exeNameMatch(programName, ZSTD_ZSTDMT)) nbWorkers=NBWORKERS_AUTOCPU, singleThread=0; if (exeNameMatch(programName, ZSTD_UNZSTD)) operation=zom_decompress; if (exeNameMatch(programName, ZSTD_CAT)) { operation=zom_decompress; FIO_overwriteMode(prefs); forceStdout=1; followLinks=1; FIO_setPassThroughFlag(prefs, 1); outFileName=stdoutmark; g_displayLevel=1; } /* supports multiple formats */ if (exeNameMatch(programName, ZSTD_ZCAT)) { operation=zom_decompress; FIO_overwriteMode(prefs); forceStdout=1; followLinks=1; FIO_setPassThroughFlag(prefs, 1); outFileName=stdoutmark; g_displayLevel=1; } /* behave like zcat, also supports multiple formats */ @@ -938,7 +941,7 @@ int main(int argCount, const char* argv[]) suffix = GZ_EXTENSION; cType = FIO_gzipCompression; removeSrcFile=1; dictCLevel = cLevel = 6; /* gzip default is -6 */ } - if (exeNameMatch(programName, ZSTD_GUNZIP)) { operation=zom_decompress; removeSrcFile=1; } /* behave like gunzip, also supports multiple formats */ + if (exeNameMatch(programName, ZSTD_GUNZIP)) { operation=zom_decompress; removeSrcFile=1; } /* behave like gunzip, also supports multiple formats */ if (exeNameMatch(programName, ZSTD_GZCAT)) { operation=zom_decompress; FIO_overwriteMode(prefs); forceStdout=1; followLinks=1; FIO_setPassThroughFlag(prefs, 1); outFileName=stdoutmark; g_displayLevel=1; } /* behave like gzcat, also supports multiple formats */ if (exeNameMatch(programName, ZSTD_LZMA)) { suffix = LZMA_EXTENSION; cType = FIO_lzmaCompression; removeSrcFile=1; } /* behave like lzma */ if (exeNameMatch(programName, ZSTD_UNLZMA)) { operation=zom_decompress; cType = FIO_lzmaCompression; removeSrcFile=1; } /* behave like unlzma, also supports multiple formats */ @@ -1081,7 +1084,7 @@ int main(int argCount, const char* argv[]) continue; } #endif - if (longCommandWArg(&argument, "--threads")) { NEXT_INT32(nbWorkers); continue; } + if (longCommandWArg(&argument, "--threads")) { NEXT_UINT32(nbWorkers); continue; } if (longCommandWArg(&argument, "--memlimit")) { NEXT_UINT32(memLimit); continue; } if (longCommandWArg(&argument, "--memory")) { NEXT_UINT32(memLimit); continue; } if (longCommandWArg(&argument, "--memlimit-decompress")) { NEXT_UINT32(memLimit); continue; } @@ -1287,7 +1290,7 @@ int main(int argCount, const char* argv[]) /* nb of threads (hidden option) */ case 'T': argument++; - nbWorkers = (int)readU32FromChar(&argument); + nbWorkers = readU32FromChar(&argument); break; /* Dictionary Selection level */ @@ -1332,10 +1335,7 @@ int main(int argCount, const char* argv[]) DISPLAYLEVEL(3, WELCOME_MESSAGE); #ifdef ZSTD_MULTITHREAD - if ((operation==zom_decompress) && (nbWorkers > 1)) { - DISPLAYLEVEL(2, "Warning : decompression does not support multi-threading\n"); - } - if ((nbWorkers==0) && (!singleThread)) { + if ((nbWorkers==NBWORKERS_AUTOCPU) && (!singleThread)) { /* automatically set # workers based on # of reported cpus */ if (defaultLogicalCores) { nbWorkers = UTIL_countLogicalCores(); @@ -1345,14 +1345,7 @@ int main(int argCount, const char* argv[]) DISPLAYLEVEL(3, "Note: %d physical core(s) detected \n", nbWorkers); } } - /* Resolve to default if nbWorkers is still unset */ - if (nbWorkers == -1) { - if (operation == zom_decompress) { - nbWorkers = 1; - } else { - nbWorkers = default_nbThreads(); - } - } + assert(nbWorkers >= 0); if (operation != zom_bench) DISPLAYLEVEL(4, "Compressing with %u worker threads \n", nbWorkers); #else diff --git a/tests/cli-tests/compression/multi-threaded.sh b/tests/cli-tests/compression/multi-threaded.sh index ac094129e..25d862b45 100755 --- a/tests/cli-tests/compression/multi-threaded.sh +++ b/tests/cli-tests/compression/multi-threaded.sh @@ -10,7 +10,3 @@ zstd -T0 -f file -q ; zstd -t file.zst zstd -T0 --auto-threads=logical -f file -q ; zstd -t file.zst zstd -T0 --auto-threads=physical -f file -q ; zstd -t file.zst zstd -T0 --jobsize=1M -f file -q ; zstd -t file.zst - -# multi-thread decompression warning test -zstd -T0 -f file -q ; zstd -t file.zst; zstd -T0 -d file.zst -o file3 -zstd -T0 -f file -q ; zstd -t file.zst; zstd -T2 -d file.zst -o file4 diff --git a/tests/cli-tests/compression/multi-threaded.sh.stderr.exact b/tests/cli-tests/compression/multi-threaded.sh.stderr.exact index 0dcf52ac4..cb3a24aad 100644 --- a/tests/cli-tests/compression/multi-threaded.sh.stderr.exact +++ b/tests/cli-tests/compression/multi-threaded.sh.stderr.exact @@ -5,8 +5,3 @@ file.zst : 65537 bytes file.zst : 65537 bytes file.zst : 65537 bytes file.zst : 65537 bytes -file.zst : 65537 bytes -file.zst : 65537 bytes -file.zst : 65537 bytes -Warning : decompression does not support multi-threading -file.zst : 65537 bytes From 56e2ebf5c38a22e0264b709a802b1cfa2ce21167 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 10 Mar 2025 09:54:06 -0700 Subject: [PATCH 2/5] removed useless assert() --- programs/zstdcli.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 30a406a7d..637567b95 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -1338,14 +1338,13 @@ int main(int argCount, const char* argv[]) if ((nbWorkers==NBWORKERS_AUTOCPU) && (!singleThread)) { /* automatically set # workers based on # of reported cpus */ if (defaultLogicalCores) { - nbWorkers = UTIL_countLogicalCores(); + nbWorkers = (unsigned)UTIL_countLogicalCores(); DISPLAYLEVEL(3, "Note: %d logical core(s) detected \n", nbWorkers); } else { - nbWorkers = UTIL_countPhysicalCores(); + nbWorkers = (unsigned)UTIL_countPhysicalCores(); DISPLAYLEVEL(3, "Note: %d physical core(s) detected \n", nbWorkers); } } - assert(nbWorkers >= 0); if (operation != zom_bench) DISPLAYLEVEL(4, "Compressing with %u worker threads \n", nbWorkers); #else From c18374bb16ecea2a34d003be769bf14b7e97b931 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 10 Mar 2025 13:40:47 -0700 Subject: [PATCH 3/5] add test checks that ZSTD_NBTHREADS triggers the expected verbose message Also: checked that the new test script fails on current `dev` branch, and is fixed by this branch --- tests/playTests.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/playTests.sh b/tests/playTests.sh index 65aa5c0b1..366f9c741 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -1570,7 +1570,11 @@ then ZSTD_NBTHREADS=3a7 zstd -f mt_tmp # malformed env var, warn and revert to default setting ZSTD_NBTHREADS=50000000000 zstd -f mt_tmp # numeric value too large, warn and revert to default setting= ZSTD_NBTHREADS=2 zstd -f mt_tmp # correct usage - ZSTD_NBTHREADS=1 zstd -f mt_tmp # correct usage: single thread + ZSTD_NBTHREADS=1 zstd -f mt_tmp # correct usage: single worker + ZSTD_NBTHREADS=4 zstd -f mt_tmp -vv 2>&1 | grep "4 worker threads" # check message + zstd -tq mt_tmp.zst + ZSTD_NBTHREADS=0 zstd -f mt_tmp -vv 2>&1 | grep "core(s) detected" # check core count autodetection is triggered + zstd -tq mt_tmp.zst # temporary envvar changes in the above tests would actually persist in macos /bin/sh unset ZSTD_NBTHREADS rm -f mt_tmp* From 2ff87aefac1be81601a46a98135b15f57ff3ca56 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 10 Mar 2025 13:55:45 -0700 Subject: [PATCH 4/5] fix FreeBSD use an alias instead of a function also: added more traces and updated version nb to v1.5.8 --- lib/zstd.h | 2 +- programs/zstdcli.c | 10 ++++++++-- tests/playTests.sh | 24 ++++++++---------------- 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/lib/zstd.h b/lib/zstd.h index 4ce2f7742..9fe542edc 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -111,7 +111,7 @@ extern "C" { /*------ Version ------*/ #define ZSTD_VERSION_MAJOR 1 #define ZSTD_VERSION_MINOR 5 -#define ZSTD_VERSION_RELEASE 7 +#define ZSTD_VERSION_RELEASE 8 #define ZSTD_VERSION_NUMBER (ZSTD_VERSION_MAJOR *100*100 + ZSTD_VERSION_MINOR *100 + ZSTD_VERSION_RELEASE) /*! ZSTD_versionNumber() : diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 637567b95..04e9218cf 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -686,6 +686,12 @@ static void printVersion(void) DISPLAYOUT("lz4 version %s\n", FIO_lz4Version()); DISPLAYOUT("lzma version %s\n", FIO_lzmaVersion()); + #ifdef ZSTD_MULTITHREAD + DISPLAYOUT("supports Multithreading \n"); + #else + DISPLAYOUT("single-thread operations only \n"); + #endif + /* posix support */ #ifdef _POSIX_C_SOURCE DISPLAYOUT("_POSIX_C_SOURCE defined: %ldL\n", (long) _POSIX_C_SOURCE); @@ -1336,7 +1342,7 @@ int main(int argCount, const char* argv[]) #ifdef ZSTD_MULTITHREAD if ((nbWorkers==NBWORKERS_AUTOCPU) && (!singleThread)) { - /* automatically set # workers based on # of reported cpus */ + /* automatically set # workers based on # of reported cpu cores */ if (defaultLogicalCores) { nbWorkers = (unsigned)UTIL_countLogicalCores(); DISPLAYLEVEL(3, "Note: %d logical core(s) detected \n", nbWorkers); @@ -1345,7 +1351,7 @@ int main(int argCount, const char* argv[]) DISPLAYLEVEL(3, "Note: %d physical core(s) detected \n", nbWorkers); } } - if (operation != zom_bench) + if (operation == zom_compress) DISPLAYLEVEL(4, "Compressing with %u worker threads \n", nbWorkers); #else (void)singleThread; (void)nbWorkers; (void)defaultLogicalCores; diff --git a/tests/playTests.sh b/tests/playTests.sh index 366f9c741..3d6bcdb0b 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -1,7 +1,7 @@ #!/bin/sh set -e # exit immediately on error -# set -x # print commands before execution (debug) +set -x # print commands before execution (debug) unset ZSTD_CLEVEL unset ZSTD_NBTHREADS @@ -16,13 +16,7 @@ datagen() { "$DATAGEN_BIN" "$@" } -zstd() { - if [ -z "$EXE_PREFIX" ]; then - "$ZSTD_BIN" "$@" - else - "$EXE_PREFIX" "$ZSTD_BIN" "$@" - fi -} +alias zstd="$EXE_PREFIX $ZSTD_BIN" sudoZstd() { if [ -z "$EXE_PREFIX" ]; then @@ -1563,18 +1557,16 @@ then println "\n===> zstdmt environment variable tests " echo "multifoo" >> mt_tmp ZSTD_NBTHREADS=-3 zstd -f mt_tmp # negative value, warn and revert to default setting - ZSTD_NBTHREADS='' zstd -f mt_tmp # empty env var, warn and revert to default setting - ZSTD_NBTHREADS=- zstd -f mt_tmp # malformed env var, warn and revert to default setting - ZSTD_NBTHREADS=a zstd -f mt_tmp # malformed env var, warn and revert to default setting - ZSTD_NBTHREADS=+a zstd -f mt_tmp # malformed env var, warn and revert to default setting + ZSTD_NBTHREADS='' zstd -f mt_tmp # empty env var, warn and revert to default setting + ZSTD_NBTHREADS=- zstd -f mt_tmp # malformed env var, warn and revert to default setting + ZSTD_NBTHREADS=a zstd -f mt_tmp # malformed env var, warn and revert to default setting + ZSTD_NBTHREADS=+a zstd -f mt_tmp # malformed env var, warn and revert to default setting ZSTD_NBTHREADS=3a7 zstd -f mt_tmp # malformed env var, warn and revert to default setting ZSTD_NBTHREADS=50000000000 zstd -f mt_tmp # numeric value too large, warn and revert to default setting= ZSTD_NBTHREADS=2 zstd -f mt_tmp # correct usage ZSTD_NBTHREADS=1 zstd -f mt_tmp # correct usage: single worker - ZSTD_NBTHREADS=4 zstd -f mt_tmp -vv 2>&1 | grep "4 worker threads" # check message - zstd -tq mt_tmp.zst - ZSTD_NBTHREADS=0 zstd -f mt_tmp -vv 2>&1 | grep "core(s) detected" # check core count autodetection is triggered - zstd -tq mt_tmp.zst + ZSTD_NBTHREADS=4 zstd -f mt_tmp -vv 2>&1 | $GREP "4 worker threads" # check message + ZSTD_NBTHREADS=0 zstd -f mt_tmp -vv 2>&1 | $GREP "core(s) detected" # check core count autodetection is triggered # temporary envvar changes in the above tests would actually persist in macos /bin/sh unset ZSTD_NBTHREADS rm -f mt_tmp* From 3c4096c83e997adb181a7d2b8f5c02649bb98af5 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 10 Mar 2025 19:11:44 -0700 Subject: [PATCH 5/5] fixed ShellCheck warning --- tests/playTests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/playTests.sh b/tests/playTests.sh index 3d6bcdb0b..72a47d634 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -16,7 +16,7 @@ datagen() { "$DATAGEN_BIN" "$@" } -alias zstd="$EXE_PREFIX $ZSTD_BIN" +alias zstd='$EXE_PREFIX $ZSTD_BIN' sudoZstd() { if [ -z "$EXE_PREFIX" ]; then