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

migrate from lzma to zip file. #5264

Merged
merged 3 commits into from Aug 30, 2019

Conversation

@pazos
Copy link
Contributor

commented Aug 25, 2019

Requires koreader/android-luajit-launcher#174 and koreader/android-luajit-launcher#175

The main benefit of having the libraries outside the zip file is that they will be installed when installing the application instead of copying them on the first run (it could save a few seconds)

Edit: I'm leaving the libs thingy for the future.

@Frenzie Frenzie added this to the 2019.09 milestone Aug 25, 2019

@pazos pazos force-pushed the pazos:android_lzma_to_zip branch from d0a69ea to 3a8beda Aug 26, 2019

@pazos pazos changed the title [wip]: migrate from lzma to zip file [wip]: migrate from lzma to zip file and move libraries outside the zip. Aug 26, 2019

@Frenzie

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

Also needs:

diff --git a/kodev b/kodev
index e44e064b..4f8b7d15 100755
--- a/kodev
+++ b/kodev
@@ -9,15 +9,7 @@ fi
 
 # Default Android build to arm.
 ANDROID_ARCH="${ANDROID_ARCH:-arm}"
-if [ -z "${ANDROID_FULL_ARCH}" ]; then
-    if [ "${ANDROID_ARCH}" = "arm" ]; then
-        ANDROID_FULL_ARCH_APK="${ANDROID_FULL_ARCH_APK:-arm-linux-androideabi}"
-    elif [ "${ANDROID_ARCH}" = "x86" ]; then
-        ANDROID_FULL_ARCH_APK="${ANDROID_FULL_ARCH_APK:-i686-linux-android}"
-    else
-        ANDROID_FULL_ARCH_APK="${ANDROID_ARCH}"
-    fi
-fi
+
 # Default to Android 4.0+; required for NDK 15 but with a custom NDK the strict minimum is 9.
 NDKABI=${NDKABI:-14}
 export NDKABI
@@ -625,7 +617,7 @@ TARGET:
                 # uninstall existing package to make sure *everything* is gone from memory
                 # no assert_ret_zero; uninstall is allowed to fail if there's nothing to uninstall
                 adb uninstall "org.koreader.launcher"
-                adb install "koreader-android-${ANDROID_FULL_ARCH_APK}${KODEBUG_SUFFIX}-${VERSION}.apk"
+                adb install "koreader-android-${ANDROID_ARCH}${KODEBUG_SUFFIX}-${VERSION}.apk"
                 assert_ret_zero $?
                 # there's no adb run so we do this…
                 adb shell monkey -p org.koreader.launcher -c android.intent.category.LAUNCHER 1
@Frenzie

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

@pazos I seem to be missing some libraries in my APK:

root@generic_x86:/data/user/0/org.koreader.launcher/lib # ls                   
libblitbuffer.so
libcrengine.so
libkoreader-cre.so
libkoreader-djvu.so
libkoreader-lfs.so
liblodepng.so
libluacompat52.so
libluajit.so
liblualongnumber.so
libmupdf.so
libsqlite3.so
libturbojpeg.so
libwrap-mupdf.so

But they also exist here? Bit confused tbh.

root@generic_x86:/data/app/org.koreader.launcher-1/lib/x86 # ls
libblitbuffer.so
libcrengine.so
libkoreader-cre.so
libkoreader-djvu.so
libkoreader-lfs.so
liblodepng.so
libluacompat52.so
libluajit.so
liblualongnumber.so
libmupdf.so
libsqlite3.so
libturbojpeg.so
libwrap-mupdf.so
@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

seem to be missing some libraries in my APK

Ops, I didn't notice. Good catch!

But they also exist here? Bit confused tbh

/data/user/x/something should be a symbolic link to /data/app/something in multiuser environments.

@Frenzie

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

Oh, you're right. I hadn't noticed.

root@generic_x86:/data/user/0/org.koreader.launcher # ls -l
drwxrwx--x u0_a159  u0_a159           2019-08-26 21:17 cache
drwxrwx--x u0_a159  u0_a159           2019-08-26 21:17 code_cache
drwxrwx--x u0_a159  u0_a159           2019-08-26 21:17 files
lrwxrwxrwx root     root              2019-08-26 21:17 lib -> /data/app/org.koreader.launcher-1/lib/x86
@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2019

So the change from lzma to zip increases the assets 3'1MB on release builds. (Lastest x86 nightly has a 7z package of 32,9MB while the same assets compressed as a zip take 36MB)

There are a few micro optimizations that can save us a few KB:

  • enable proguard (reduces classes.dex from 1.4MB to 300KB and the whole APK about 300kb).
  • remove a few unused bits (man pages, README and COPYING...) aprox another 300kb of size reduction.

I think we need to take a decission about Noto:

NotoSansCJKsc-Regular has a size of 17,3MB. All the rest of Noto fonts have a size of 4MB together. If we want to keep CJK support in place we might to remove the rest of Noto fonts and use Roboto instead.

Probably all phones with 4.0+ on CJK countries have NotoSans/NotoSerif CJK available as a system font. This is difficult to check for me :/

@NiLuJe

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

@pazos: At worst we've established that mostly everything should have Droid Fallback, so, eh, that's good enough for me. That already used to be our CJK fallback back in the day.

@Frenzie

This comment has been minimized.

Copy link
Member

commented Aug 28, 2019

Yeah, older Androids have Droid Fallback until Noto showed up. The only exception would be if a manufacturer removed the default fonts, which would be pretty crazy but then again it's Android.

One can of course always just grab the fonts from https://github.com/koreader/koreader-fonts/tree/master/noto and stick 'em in /sdcard/koreader/fonts.

@pazos pazos changed the title [wip]: migrate from lzma to zip file and move libraries outside the zip. migrate from lzma to zip file. Aug 29, 2019

@pazos pazos force-pushed the pazos:android_lzma_to_zip branch from 3a8beda to 87e771f Aug 29, 2019

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

@Frenzie: as the libs thingy is going to take some time I think we can move to zip and do the fonts thing before.

@Frenzie

This comment has been minimized.

Copy link
Member

commented Aug 29, 2019

Sure, just merge the zip migration when ready and then I can look into the font thing tomorrow.

pazos added 3 commits Aug 29, 2019
android: fix some warnings on launcher script,
no need to chmod binaries as they are uncompressed on each update,
A becomes android

@pazos pazos force-pushed the pazos:android_lzma_to_zip branch from 87e771f to 7381e1d Aug 30, 2019

@pazos pazos merged commit 621a84c into koreader:master Aug 30, 2019

1 check passed

ci/circleci: build Your tests passed on CircleCI!
Details
@poire-z

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2019

Just a note, not sure it's related, but letting you know.
I updated on android from a version from last weekend (manual download of the nightly apk).
Then, on subsequent launches, I got the black screen with the rolling thingy, that I usually get only once, the first launch after the update. Today, I get it on each KOreader launch.

I then just uninstalled koreader and installed that same nightly: got the rolling thingy only once - no more on each launch.

(Dunno if we need/can do something, users will get that if they don't think about reinstalling.)

@pazos

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

@poire-z: it happened to me a few times before the change but I couldn't manage to reproduce after the change.

In any case we shouldn't show the dialog unless we are extracting something.

Frenzie added a commit to Frenzie/koreader that referenced this pull request Sep 3, 2019
[Android] Don't ship Noto
Follow-up to <koreader#5252>. This greatly reduces the Android package size.

See discussion in <koreader#5264 (comment)>.
@Frenzie Frenzie referenced this pull request Sep 3, 2019
Frenzie added a commit that referenced this pull request Sep 4, 2019
[Android] Don't ship Noto (#5310)
Follow-up to <#5252>. This greatly reduces the Android package size.

See discussion in <#5264 (comment)>.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.