Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Backport multi-threaded zstd to 4.14.x to support multi-threaded zstd compression on centos 8 #2130

Conversation

angry-cellophane
Copy link

@angry-cellophane angry-cellophane commented Jul 25, 2022

Summary:
zstd supports multi-threaded compression and is already available in rpm 4.17.
Unfortunately, this feature is not available on centos 8, that has 4.14.x only.
Adding this feature requires upgrading dependency on zstd lib and backporting the intergration.

Changes:

Testing:

  1. Created a test rpmbuild project https://gist.github.com/angry-cellophane/cfa31454f2376388079c74af7d459aa0
  2. Built rpm with my changes and zstd enabled - ./configure && make
  3. Ran rpmbuild with different payloads, unpacked the rpm after each run with rpm2cpio and checked the rpm's size.

w19T0.zstdio

❱❱❱ time ./rpmbuild --define '_source_payload w19T0.zstdio'  -bs -v ~/rpmbuild/SPECS/test.spec
Wrote: /home/****/rpmbuild/SRPMS/test-0.0.1-1.src.rpm
./rpmbuild --define '_source_payload w19T0.zstdio' -bs -v   277.81s user 1.49s system 803% cpu 34.775 total

❱❱❱ du -sh SRPMS/*
68M	SRPMS/test-0.0.1-1.src.rpm

❱❱❱ rpm2cpio SRPMS/test-0.0.1-1.src.rpm | cpio -idmv
binary_412MB
test.spec
text_file
843347 blocks

w19.zstdio

[❱❱❱ time ./rpmbuild --define '_source_payload w19.zstdio'  -bs -v ~/rpmbuild/SPECS/test.spec
Wrote: /home/****/rpmbuild/SRPMS/test-0.0.1-1.src.rpm
./rpmbuild --define '_source_payload w19.zstdio' -bs -v   240.95s user 0.55s system 99% cpu 4:01.54 total

❱❱❱ du -sh SRPMS/*
68M	SRPMS/test-0.0.1-1.src.rpm
[14:14:32] [cost 0.063s] du -sh SRPMS/*

❱❱❱ rpm2cpio SRPMS/test-0.0.1-1.src.rpm | cpio -idmv
binary_412MB
test.spec
text_file
843347 blocks

w19T1.zstdio

❱❱❱ time ./rpmbuild --define '_source_payload w19T1.zstdio'  -bs -v ~/rpmbuild/SPECS/test.spec
Wrote: /home/****/rpmbuild/SRPMS/test-0.0.1-1.src.rpm
./rpmbuild --define '_source_payload w19T1.zstdio' -bs -v   246.21s user 0.51s system 100% cpu 4:06.45 total

❱❱❱ du -sh SRPMS/*
68M	SRPMS/test-0.0.1-1.src.rpm

❱❱❱ rpm2cpio SRPMS/test-0.0.1-1.src.rpm | cpio -idmv
binary_412MB
test.spec
text_file
843347 blocks

w19T4.zstdio

❱❱❱ time ./rpmbuild --define '_source_payload w19T4.zstdio'  -bs -v ~/rpmbuild/SPECS/test.spec
Wrote: /home/****/rpmbuild/SRPMS/test-0.0.1-1.src.rpm
./rpmbuild --define '_source_payload w19T4.zstdio' -bs -v   255.46s user 0.65s system 353% cpu 1:12.46 total

❱❱❱ du -sh SRPMS/*
68M	SRPMS/test-0.0.1-1.src.rpm

❱❱❱ rpm2cpio SRPMS/test-0.0.1-1.src.rpm | cpio -idmv
binary_412MB
test.spec
text_file
843347 blocks

w2T0.xzdio (for comparison and to double check)

❱❱❱ time ./rpmbuild --define '_source_payload w2T0.xzdio'  -bs -v ~/rpmbuild/SPECS/test.spec
Wrote: /home/****/rpmbuild/SRPMS/test-0.0.1-1.src.rpm
./rpmbuild --define '_source_payload w2T0.xzdio' -bs -v   50.81s user 0.43s system 99% cpu 51.245 tota

❱❱❱ du -sh SRPMS/*
73M	SRPMS/test-0.0.1-1.src.rpm

❱❱❱ rpm2cpio SRPMS/test-0.0.1-1.src.rpm | cpio -idmv
binary_412MB
test.spec
text_file
843347 blocks

threads = strtol(s, (char **)&s, 10);
/* T0 means automatic detection */
if (threads == 0)
threads = sysconf(_SC_NPROCESSORS_ONLN);
Copy link
Author

Choose a reason for hiding this comment

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

same as in xz. Not sure if this is the right thing to do. getcpunumber macro is not available in 4.14.x

@ffesti
Copy link
Contributor

ffesti commented Jul 26, 2022

While your effort is laudable I fear you are wasting your time and I am hesitant to help with that.

As far as RPM upstream is concerned we are very unlikely to do another 4.14.x release. And even if we did a release I doubt this kind of change is eligible for that. So there is not really a place for this here in the GH project.

The discussion on what's happening in Centos 8 - and RHEL 8 - does not really belong here but I have trouble imagining this feature being back ported there. If you think this patch should go into Centos/RHEL 8 proper you are better off getting approval there first. But you'd need very good arguments.

So the only kinda viable thing here is you doing this patch and having it in your own private RPM package you use on your local build system. Are you sure this is worth the effort?

@angry-cellophane
Copy link
Author

thank you @ffesti, I'll go with the private RPM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants