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

Add an option for the zip compression level #3410

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vbfox
Copy link

@vbfox vbfox commented Oct 27, 2023

Add a new option l or compression-level to configure the compression level for the generated Zip files.

Add a new option "l" or "compression-level" to configure the compression
level for the generated Zip files.
@iBotPeaches
Copy link
Owner

Am I correct this is just the compression of the outer zip (ie apk)? Or am I misunderstanding?

We have plenty of logic at the moment that reviews all the unknown files and tags them as compressed or not. I see this could be expanded to record the exact compression level of the non-stored

@vbfox
Copy link
Author

vbfox commented Oct 31, 2023

Yes it's about the default compression level of files in the apk.

I changed in both places when the apk is build via buildApk > zipPackage > zipFolder and when unknown files are added. (I removed the 2 pass zip in #3415 so they'll conflict a bit)

As it set the default level of the zip (apk/jar) file it will affect all files marked with Method Deflate (So files in mDoNotCompress that have Method Stored won't be affected).

As for keeping the compression level along with the Deflate/Store information it can be done but I didn't really have the need.


To give more context what I'm trying to optimize is a system where we decode an apk then edit smali files then rebuild an apk. So i'm trying to find a balance between speed of this process and the final apk size that we'll keep in storage, and i'm more interested in forcing a compression level than keeping the original one.

The end user system is a testing platform where user's CI upload apk files and we inject our testing infrastructure tooling before running tests on the application ( https://www.waldo.com/ ).

@iBotPeaches
Copy link
Owner

As for keeping the compression level along with the Deflate/Store information it can be done but I didn't really have the need.

What I'm lost about though. Apktool attempts or strives to match the original. At one point I wanted to record the compression level of the original application and restore that. I couldn't do that without apache compress library, so deferred it.

Now this PR allows you to blanket specify compression level. My fear is the safeness, but as you mentioned in background. You want to do this solely to compress apk?

@vbfox
Copy link
Author

vbfox commented Nov 6, 2023

Now this PR allows you to blanket specify compression level. My fear is the safeness, but as you mentioned in background. You want to do this solely to compress apk?

My target is getting the build process to finish faster, potentially at the cost of size (Essentially using --compression-level 1)

As for blanket specifying compression level it's essentially what already happens when not specifying a value like before or explicitely using the -1 value (For OpenJDK 11 it's equivalent to level = 6 https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/19fb8f93c59dfd791f62d41f332db9e306bc1422/src/java.base/share/native/libzip/zlib/deflate.c#L606 ).

Also the default level isn't a blanket specification that override the per-entry level it's only a default value if an entry doesn't have a more specific level configured.

There is an interesting question to solve if this PR goes in and per-file compression level is added later to know if the argument should override the per-file record or not (By default it would not). But I think it should not and the exact stored information should take priority.

@iBotPeaches
Copy link
Owner

Also the default level isn't a blanket specification that override the per-entry level it's only a default value if an entry doesn't have a more specific level configured.

In Apktool's case though I believe we only have a difference between stored/deflated and make no current recording of the compression level - solely if was stored or not. So best I understand this now - nothing changes for users who ignore the flag as the default is the same as it was before. The difference though would come into play for deflated files.

Copy link
Owner

@iBotPeaches iBotPeaches left a comment

Choose a reason for hiding this comment

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

Re-visiting old PRs. Just conflicts here.

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.

2 participants