Skip to content

Commit

Permalink
app-arch/zstd: multithreaded compression by default (#598)
Browse files Browse the repository at this point in the history
Integration of Clear Linux's 'multi-thread-default.patch'

By default, zstd uses one core for compression. This patch
makes zstd use all physical cores detected for compression,
increasing performance and reducing compression time.

Below's results are from using zstd's built-in benchmark,
showing a decrease of 78.13% in compression time with -T0.
The benefit is only apparent if compression is CPU-bound
and will differ based on machine and file contents.

Default (-T1)
	19#linux-5.8.tar     : 983869440 -> 121381009 (8.106),  4.05 MB/s ,1771.3 MB/s
	zstd -T1 -b19 -i0 --priority=rt linux-5.8.tar  244.99s user 0.46s system 99% cpu 4:05.47 total

Patched default (-T0)
	19#linux-5.8.tar     : 983869440 -> 121384544 (8.105),  19.2 MB/s ,1756.7 MB/s
	zstd -T0 -b19 -i0 --priority=rt linux-5.8.tar  297.19s user 0.63s system 554% cpu 53.692 total

Source: https://github.com/clearlinux-pkgs/zstd/blob/1.4.5-60/multi-thread-default.patch
References: #164
  • Loading branch information
jiblime authored Sep 11, 2020
1 parent dbeefd6 commit 579ed2d
Showing 1 changed file with 39 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
diff --git a/programs/zstdcli.c b/programs/zstdcli.c
index 70e2b70..ebd5b33 100644
--- a/programs/zstdcli.c
+++ b/programs/zstdcli.c
@@ -695,7 +695,7 @@ int main(int const argCount, const char* argv[])
if ((filenames==NULL) || (file_of_names==NULL)) { DISPLAY("zstd: allocation error \n"); exit(1); }
programName = lastNameFromPath(programName);
#ifdef ZSTD_MULTITHREAD
- nbWorkers = 1;
+ nbWorkers = 0;
#endif

/* preset behaviors */
diff --git a/programs/README.md b/programs/README.md
index 53706de..c1b032f 100644
--- a/programs/README.md
+++ b/programs/README.md
@@ -168,7 +168,7 @@ Advanced arguments :
--stream-size=# : optimize compression parameters for streaming input of given number of bytes
--size-hint=# optimize compression parameters for streaming input of approximately this size
--target-compressed-block-size=# : make compressed block near targeted size
- -T# : spawns # compression threads (default: 1, 0==# cores)
+ -T# : spawns # compression threads (default: 0==# cores)
-B# : select size of each job (default: 0==automatic)
--rsyncable : compress using a rsync-friendly method (-B sets block size)
--no-dictID : don't write dictID into header (dictionary compression)
diff --git a/programs/zstdcli.c b/programs/zstdcli.c
index 70e2b70..4a872fe 100644
--- a/programs/zstdcli.c
+++ b/programs/zstdcli.c
@@ -160,7 +160,7 @@ static void usage_advanced(const char* programName)
DISPLAYOUT( "--fast[=#]: switch to very fast compression levels (default: %u) \n", 1);
DISPLAYOUT( "--adapt : dynamically adapt compression level to I/O conditions \n");
# ifdef ZSTD_MULTITHREAD
- DISPLAYOUT( " -T# : spawns # compression threads (default: 1, 0==# cores) \n");
+ DISPLAYOUT( " -T# : spawns # compression threads (default: 0==# cores) \n");
DISPLAYOUT( " -B# : select size of each job (default: 0==automatic) \n");
DISPLAYOUT( "--single-thread : use a single thread for both I/O and compression (result slightly different than -T1) \n");
DISPLAYOUT( "--rsyncable : compress using a rsync-friendly method (-B sets block size) \n");

8 comments on commit 579ed2d

@Jannik2099
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not apply to zstd-1.4.4, which is the current stable version. can you please revert it?

@barolo
Copy link

@barolo barolo commented on 579ed2d Sep 28, 2020

Choose a reason for hiding this comment

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

@Jannik2099 I don't think that you should expect from an experimental overlay to cater to your stable needs

@Jannik2099
Copy link
Contributor

Choose a reason for hiding this comment

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

This has nothing to do with experimental vs stable, this patch was simply not tested before getting committed. The majority of users run stable on non-user applications, there's no good reason for this overlay shipping stuff that breaks default configs.

@barolo
Copy link

@barolo barolo commented on 579ed2d Sep 28, 2020

Choose a reason for hiding this comment

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

This overlay is not for majority of the users, even its recommends like gcc 10 are from testing

@Jannik2099
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, there is no reason to DELIBERATELY break stuff for people. You'll find a good chunk of package.cflags only applies to specific stable versions aswell

@barolo
Copy link

@barolo barolo commented on 579ed2d Sep 28, 2020

Choose a reason for hiding this comment

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

Disregarding my previous comments. I don't really get what the problem is, this should only apply to 1.4.5? Does it break 1.4.4?
Nvm, it's applied to all.
Revert is not needed just lock it to a version.

@Jannik2099
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the patch doesn't apply to 1.4.4 , which causes the build to fail
Patches can be restricted to >= just like normal ebuilds by renaming the directory, could we do this perhaps? (assuming the patch bashrc can do that aswell)

@barolo
Copy link

@barolo barolo commented on 579ed2d Sep 28, 2020

Choose a reason for hiding this comment

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

Yeah, it works the same, you can even do it on your own by renaming the dir [ I know that it will break repo updates since it introduces changes to a git dir which is super annoying ]

Please sign in to comment.