Skip to content

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Feb 25, 2025

System.IO.Compression uses the default compression level which is not the Smallest compression level. This change updates the compression level to the best compression level since the size of the Apk is very important.

Unfortunately, the SmallestSize compression enum level is not available on netstandard2.0. So we have to use a cast to get it to work.

`System.IO.Compression`` uses the default compression level which is not
the best compression level. This change updates the compression
level to the best compression level since the size of the Apk is
very important.

Unfortunately, the best compression enum level is not available on `netstandard2.0`.
So we have to use a cast to get it to work.
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

We may want this one to actually test System.IO.Compression in main:

@jonathanpeppers
Copy link
Member

Should we make an MSBuild property to toggle the enum?

Then we could consider debug mode using a "fast" compression and Release doing a "slow" but small compression?

@jpobst
Copy link
Contributor

jpobst commented Feb 26, 2025

We should probably have performance data on this to understand the tradeoffs. For a Debug build compression time is probably more important than compression size.

Also, the Release aab is going to be recompressed by Google so a few percent probably doesn't matter there either, but if it's significant then it might be useful.

Archive size differences between LibZipSharp and SIC are available on #9623.

@dellis1972
Copy link
Contributor Author

We should probably have performance data on this to understand the tradeoffs. For a Debug build compression time is probably more important than compression size.

Also, the Release aab is going to be recompressed by Google so a few percent probably doesn't matter there either, but if it's significant then it might be useful.

Archive size differences between LibZipSharp and SIC are available on #9623.

Were there specific apps you got the differences in #9623 on? I should probably do the same ones

@dellis1972
Copy link
Contributor Author

We do need to consider upload size for aab as well. A smaller package means less bandwidth, people do still pay for data.

@jonpryor
Copy link
Contributor

Do we want or need this? As-is?

Is this codepath even used?

For example, we know that right now System.IO.Compression is broken for us; see also dotnet/runtime#113306

Given that, why were the PR checks green? This suggests that we're not using System.IO.Compression right now. (See also: 7454deb, f3ef4fe.)

I'm not sure what we should do with this PR right now.

@jpobst
Copy link
Contributor

jpobst commented Mar 11, 2025

Using an existing quick and dirty test I already had that simulates adding all our assemblies to a packaged_resources zip:

Compression Level Time Size
NoCompression 149 ms 65408901 bytes
Fastest 390 ms 23281585 bytes
Optimal 1332 ms 17289885 bytes
SmallestSize 2979 ms 16876305 bytes

(Tested on .NET 9)

This appears to take more than double the compression time for 2-3% smaller zip files.

@jonpryor
Copy link
Contributor

@jpobst wrote:

This appears to take more than double the compression time for 2-3% smaller zip files.

That might be worth it, depending on adb install/etc. time (reducing .apk size should speed up installation). Though 2% size reduction doesn't feel like it'll quite make up for doubling the time.

"Fastest" might be best.

@dellis1972
Copy link
Contributor Author

I'm gonna close this. Looks like we need a way to configure this from the users perspective. So that for debug we use fastest but for release we use max or optimal.

@dellis1972 dellis1972 closed this Mar 12, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants