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

Better zipping #6587

Merged
merged 2 commits into from
Aug 29, 2020
Merged

Better zipping #6587

merged 2 commits into from
Aug 29, 2020

Conversation

zwim
Copy link
Contributor

@zwim zwim commented Aug 28, 2020

This gives about 1Meg smaller apk.
Compression time on a quad-core is about twice as zip -r9, when using mx=7 its about the same time (of course then the compression is not as good, but saves 800k)
Decompression memory usage does not seem to be bigger: Tested with grep grep VmPeak /proc/$PID/status and grep VmHWM /proc/$PID/status showed almost identical values.

I have used this compression the last weeks on Tolino and phone without strange effects.


This change is Reviewable

@zwim zwim requested a review from Frenzie as a code owner August 28, 2020 15:19
@Frenzie Frenzie added this to the 2020.09 milestone Aug 28, 2020
@pazos
Copy link
Member

pazos commented Aug 28, 2020

Looks cool. An alternative to save a few MB would be https://commons.apache.org/proper/commons-compress/examples.html. and switch back to xz compression.

Copy link
Member

@Frenzie Frenzie left a comment

Choose a reason for hiding this comment

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

Is 7z available on F-Droid?

@pazos
Copy link
Member

pazos commented Aug 28, 2020

Is 7z available on F-Droid?

@licaon-kter ^^. Can we assume that the build machine has 7z binary installed?

Also, for future proof, it is ok to have this artifact as a dependency: https://mvnrepository.com/artifact/org.apache.commons/commons-compress ?

@licaon-kter
Copy link

If Debian Stretch (for now) has it...

I'd go with xz though, as 7zip is soo old. But whatever you want.

@zwim
Copy link
Contributor Author

zwim commented Aug 28, 2020

I'd go with xz though, as 7zip is soo old. But whatever you want.

zip is even older!

I would have no problems with xz. In fact I would prefer it.
But there was a discussion #5264 (almost exact one year ago). I did not get the point, why KOReader moved to zip.

By the way. Using 7z for zip-file creation might be less invasive than switching to xz, my 5ct.

@licaon-kter
Copy link

I meant old as in stuck at version 16 and no newer source code.

@pazos
Copy link
Member

pazos commented Aug 28, 2020

But there was a discussion #5264 (almost exact one year ago). I did not get the point, why KOReader moved to zip

Because ZipInputStream is part of java/android libraries. That makes possible to catch errors uncompressing, which was not possible using 7z via ffi. It should be a matter of replacing the offending function with an updated one using apache common compress artifact. See https://github.com/koreader/android-luajit-launcher/blob/master/app/src/org/koreader/launcher/utils/AssetsUtils.kt#L101-L151

@pazos pazos merged commit 20a96fe into koreader:master Aug 29, 2020
@zwim zwim deleted the betterZipping branch August 29, 2020 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants