google / rpmpack

rpmpack (tar2rpm) - package rpms in pure golang or cli
Apache License 2.0
116 stars 32 forks source link

Replace compress/gzip by klauspost/pgzip #72

Closed Felixoid closed 2 years ago

Felixoid commented 2 years ago

This simple replacement gives a huge performance improvement. It's on my 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz, 8 cores, 16 threads:

ls -lh pkg.tgz
-rw-r--r-- 1 felixoid felixoid 3,8G Feb 12 21:25 pkg.tgz

See attached file for details pkg.tgz.txt

time ./tar2rpm.pgzip -name pkg2 -version 13 -file pkg2.rpm pkg.tgz
./tar2rpm.pgzip -name pkg2 -version 13 -file pkg2.rpm pkg.tgz  696,86s user 23,50s system 822% cpu 1:27,54 total

time ./tar2rpm.orig -name pkg -version 12 -file pkg.rpm pkg.tgz
./tar2rpm.orig -name pkg -version 12 -file pkg.rpm pkg.tgz  434,72s user 17,60s system 102% cpu 7:21,41 total

For the default compression level it's significant too:

time ./tar2rpm.pgzip -name pkg2 -version 13 -file pkg2.rpm -compressor gzip:-1 pkg.tgz
./tar2rpm.pgzip -name pkg2 -version 13 -file pkg2.rpm -compressor gzip:-1   75,09s user 9,98s system 308% cpu 27,606 total

time ./tar2rpm.orig -name pkg -version 12 -file pkg.rpm -compressor gzip:-1 pkg.tgz
./tar2rpm.orig -name pkg -version 12 -file pkg.rpm -compressor gzip:-1 pkg.tg  98,92s user 18,27s system 110% cpu 1:46,36 total
Felixoid commented 2 years ago

The CLA is signed

Felixoid commented 2 years ago

Dear @jarondl, can you please, review it?

jarondl commented 2 years ago

/gcbrun

jarondl commented 2 years ago

The code looks maintained, the license is fine (MIT). I did not know people use such big files here.

Yeah looks fine, let's see if the tests pass.

jarondl commented 2 years ago

You seem to be missing some changes in deps.bzl, which will break bazel users. The tests inside example_bazel fail.

Felixoid commented 2 years ago

Thank you. Sorry, I was a bit distracted the last week. I'll add it in a second

jarondl commented 2 years ago

/gcbrun

Felixoid commented 2 years ago

What is the current issue? I don't have permissions in the cloud to see

Felixoid commented 2 years ago
> bazel build rpmpack
INFO: Analyzed target //:rpmpack (70 packages loaded, 7509 targets configured).
INFO: Found 1 target...
Target //:rpmpack up-to-date:
  bazel-bin/rpmpack.a
INFO: Elapsed time: 1,872s, Critical Path: 1,17s
INFO: 19 processes: 2 internal, 17 linux-sandbox.
INFO: Build completed successfully, 19 total actions

Looks good now

jarondl commented 2 years ago

/gcbrun

jarondl commented 2 years ago

@Felixoid Thank you for the contribution, and for fixing build problems you cannot even see! (Yes it is messed up that you cannot see the build failure, I am gonna fix that).

Thank you for you patience.