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

New compressionMethod types: TARGZ_BEST_SPEED, TAR #162

Conversation

knut-olav-hoven-ske
Copy link
Contributor

@knut-olav-hoven-ske knut-olav-hoven-ske commented Feb 23, 2023

Adds more compression methods (some methods are without compression):

  • TARGZ_BEST_SPEED uses Deflate compressionLevel=1
  • TARGZ_NO_COMPRESSION uses Deflate compressionLevel=0
  • TAR uses no compression

This solves #161

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@knut-olav-hoven-ske knut-olav-hoven-ske requested a review from a team as a code owner February 23, 2023 14:52
GzipCompressorInputStream::new,
".tgz")
),
TARGZ_NO_COMPRESSION(new TarArbitraryFileCacheStrategy(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any actual benefit compared to "tar only"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think not... :)
This started out as one of my first options, before adding the TAR-only option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you want me to remove the TARGZ_NO_COMPRESSION option? I guess there is no practical use for it, when one can use the TAR option.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I think TAR is enough :)

@@ -454,6 +456,21 @@ public enum CompressionMethod {
GzipCompressorInputStream::new,
".tgz")
),
TARGZ_BEST_SPEED(new TarArbitraryFileCacheStrategy(
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you some examples which shows the difference in speed and archive size if using these parameters compared to the default settings?

Copy link
Contributor Author

@knut-olav-hoven-ske knut-olav-hoven-ske Feb 23, 2023

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking if it would make sense to set this for the TARGZ directly, as I did not really thought about this when while implementing the cache compression ;)
Do you know what happens, if you try to uncompress an archive with different settings as the ones used while compressing? If it works fine, I would say we change the settings of TARGZ. Or do you think it would be nice to keep both?

btw: Thanks for your contribution!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The gzip library is able to uncompress gz-files written with different compression levels without further input parameters. The (necessary) metadata about the compression level is written to the gz-file.
In the Java library commons-compress:1.21 (that this plugin use), the metadata about compression level is written by the class GzipCompressorOutputStream at lines 100-106.

For text files, such as source code (the node_modules directory in web-apps for instance), it makes sense to use the "default" compression level (which is 6), because it is a "sweet spot" between compression speed and size. My recommendation is not to change that.

For directories with lots of mixed file types, both lots of text and binary files, users might have "need for speed", and can use TARGZ_BEST_SPEED :)

For directories with lots of binary files, the TAR option might be best.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok nice, then lets go like this. Thanks for clarifying ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

would you mind to add this additional info to the readme? I think it would be helpful for new users to decide with setting they should choose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a new section Choosing the compression method to the README file.
I also included descriptions of the existing options, and I had to guess a bit... so please proof read it and correct me if I'm wrong :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks a lot! I have nothing to complain ;)

@repolevedavaj repolevedavaj merged commit 1dbb445 into jenkinsci:main Feb 27, 2023
@repolevedavaj repolevedavaj changed the title New compressionMethod types: TARGZ_BEST_SPEED, TARGZ_NO_COMPRESSION, TAR New compressionMethod types: TARGZ_BEST_SPEED, TAR Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants