Repository navigation
Change CLI to employ multithreading by default - #4211
Conversation
The difference between these 2 measurements is > 10%, which is quite substantial if they are expected to be equal. Also: |
|
Also as a follow-up, |
I re-did this experiment, compressing silesia with the default number threads (which on my machine evaluates to 4) and then explicitly passing Overall it seems that there is more noise in the benchmarks with multithreading, but the behavior looks overall as expected. |
| } | ||
| } | ||
| DISPLAYLEVEL(2, "Compressing with %u worker threads \n", nbWorkers); | ||
| DISPLAYLEVEL(3, "Compressing with %u worker threads \n", nbWorkers); |
There was a problem hiding this comment.
Yes, level 3 seems better
611e4f6 to
2a8502e
Compare
… level >= 4, and add lower bound of 1 for the default number of threads
457fe3f to
3fe52bf
Compare
I asserted that DataDog/zstd exposes no way to set ZSTD_c_nbWorkers. That is false: zstd_stream.go:291 has SetNbWorkers. The bench simply never called it, so the zstd-cgo row was a one-worker baseline by omission, not by constraint. zstd-cgo-mt is the same binding with it set, which makes libzstd's multithreading reachable in-process instead of only through the CLI. On a 274MiB tar: 389 MB/s at 1.02x cores for zstd-cgo against 2380 MB/s at 7.47x cores for zstd-cgo-mt. Note the worker count cannot be 0. At the library level ZSTD_c_nbWorkers=0 means *no* worker threads; the core detection behind the CLI's -T0 lives in programs/zstdcli.c, not in libzstd. Passing 0 silently yields a single-threaded encoder, measured at 1.03x cores and indistinguishable from the plain binding. It is set to GOMAXPROCS here, matching what pgzip and klauspost's zstd default to. The MT variant is registered only when the linked libzstd actually has threading, so a library built without it records a skip rather than failing the run. Also: the CLI became multi-threaded by default in v1.5.7 (meta/zstd#4211), capped at max(1, min(4, nbCores/4)), not "since 1.5.x". That bound matters -- on <= 1.5.6 a bare `zstd -{level}` really was single-threaded, so a CI running an older CLI would not show the problem this branch is correcting for.
…mode - -T# default is ZSTDCLI_NBTHREADS_DEFAULT, based on logical cores (UTIL_countLogicalCores() / 4, since #4211), not physical cores. - ZSTDMT_NBWORKERS_MAX is 64 on 32-bit and 256 on 64-bit, not 200. - Benchmark mode uses 1 worker thread by default since 725a152. Signed-off-by: YoungjuKim <[email protected]>
Summary
This PR modifies
ZSTDCLI_NBTHREADS_DEFAULTso that multithreading is used by default when the library is compiled with multithreading support.We use a conservative value of
max(1, min(4, nbCores/4))worker threads for this default value.Testing
I benchmarked on an Intel(R) Xeon(R) Gold 6138 CPU @ 2.00GHz machine with 80 logical cores. I observed that the default behavior is now similar to the behavior when explicitly doing multi-threaded compression using the
-Tflag.