Anki-Android icon indicating copy to clipboard operation
Anki-Android copied to clipboard

APK size optimization umbrella (remaining task: size comparison on PRs)

Open mikehardy opened this issue 1 year ago • 5 comments

Examine APK size on release builds, should be as small as possible

Currently has some bloat in there

(Optional) Anything else you want to share?

  • there is an APK size comparison tool but it doesn't run all the time (yet) #16613
  • there is proguard and release builds are set to optimize, but the proguard rules are currently not aggressive at all
  • we are not testing release builds in CI so changing proguard settings is dangerous until fixed (tracking in #16625 )

So, idea is:

  • test release builds in CI so changes to proguard get some validation
    • #16635
  • alter emulator test workflow to post release APK artifact for main merges (so we have current baseline size)
    • future enhancement
  • alter emulator test workflow to compare current posted main APK size with current release build size (ingesting compare size workflow logic...) and posting results (currently a comment, but as a status would be better in future
    • future enhancement
  • start changing proguard rules and verifying app still runs
    • #16635
  • also do APK inspection (Android Studio has a good tool) which can help find ridiculous things
    • anyone can pick this up after #16635 merges

Additionally, may make sense to examine Anki-Android-Backend, there are some thoughts on a previous effort #8473

  • anyone can pick this up if they like

mikehardy avatar Jun 20 '24 14:06 mikehardy

The issue is about APK optimizations, but there is also #9259 to keep in mind.

BrayanDSO avatar Jun 20 '24 14:06 BrayanDSO

@BrayanDSO

The issue is about APK optimizations, but there is also https://github.com/ankidroid/Anki-Android/issues/9259 to keep in mind.

...this is true, but unfortunately I do not think that will offer much optimization. Why? If I understand correctly that will allow users to get only the arch or language resources delivered that match their device (dynamically rendered as variants by Play Store as it renders the AAB to specific APKs in the cloud)

Now, that will allow us to stop sending the 4 different arches up to Play Store as they'll handle that, however we'll still need to build them for other stores since AAB is Play-Store-Proprietary as they have a release signing key then, but that's not a space savings, it's just a difference in where the arch split happens before device delivery

For resource variations, we need to send all languages to the device since we allow in-app language selection and it may not match the device language, so we cannot allow Play Store to strip resources. The only alternative to this is to implement the proprietary Play APIs for in-app language selection and dynamic app resource download, but as we are not Play Store specific and use other stores, this is a savings but not one I think we can pursue

TL;DR: I don't think the switch to AAB actually offers any savings. That's one of the reasons I haven't spent much time pursuing it - to me all it does is change where the arch split happens, with the cost that now Play Store has our release signing key and I still have to manage a signing key locally (a new AAB signing key...)

mikehardy avatar Jun 20 '24 15:06 mikehardy

Here is my thinking on comparing release APK size growth on an ongoing basis to prevent silly problems from creeping in again and doubling our app size without meaning to:

I want it to be event-based not schedule-based, and the event should be the PR. I don't want to compute when unnecessary though, so the general idea is to use the computation that is already occurring now that we test the release APK on every PR push and main push

What we want is a comparison of "main" release APK vs "PR" release APK, on every PR push

I've researched it and I think the feasible / not too hard to do "trick" will be to use github artifacts and always have a "main release APK size information" artifact uploaded.

  • in the emulator test workflow run use the javascript github workflow action to access the github API and for PR runs:

    • listing workflow runs for emulator workflow filtered on branch main, event push, status success - can probably just do per_page 1 as we only want last one? https://docs.github.com/en/rest/actions/workflow-runs?apiVersion=2022-11-28#list-workflow-runs-for-a-workflow
    • getting the "APK size text file" artifact (that I am inventing here...) from that workflow via a list artifacts / get artifact https://docs.github.com/en/rest/actions/artifacts?apiVersion=2022-11-28#list-workflow-run-artifacts / https://docs.github.com/en/rest/actions/artifacts?apiVersion=2022-11-28#get-an-artifact
    • comparing that size to the current APK size, then posting a comment using same logic currently in compare apk size workflow (or returning "unknown" for main if it doesn't exist yet)
  • for merges to main: in emulator workflow run (indicating a merge), echo the current release mode APK size information into a text file and uploading it as an artifact named with the correct "APK size text file" artifact name so future non-main PRs can find it for comparison

the examples in existing javascript action script chunks in our workflows should show how to define the async functions to interact with github API, and then how to "await" calls on them to get the API results, shouldn't be...too hard once that's working, can just delete the compare APK size workflow, as who would use a manual one when every PR just gets it for free with size storage on main merges and size comparison on PR runs

mikehardy avatar Jul 10 '24 16:07 mikehardy

A slight wrinkle in the implementation design idea I had above:

If we are just storing the release APK size for "main", updating it on any merge to "main" and always comparing against what is stored, then we will never know the actual change in APK size related to a specific PR in isolation because main is a moving target.

What we want is the comparison of the PR release APK size with the merge-base to main (point on main where the PR started adding commits) so that we know the APK size difference related only to the PR changes

  • See #17252

It is possible that we could just store a file that has a bunch of SHA/size tuples and update it as merges are made, then consult the file to pull the size for the SHA we are interested in, building it and calculating it on-demand if needed.

Even with as long-lived as ankidroid is, there are only 21,131 commits total to date 40bytes per SHA, sizes are 8bytes long, and a delimiter byte and newline for 50bytes per commit

That would make a 1031 kilobyte file of SHA/size tuples if we had them all going back for all time

Seems a little silly but at the same time, like it would work just fine.

Existing comparison apk workflow has the logic to determine the merge base and run a release build for it if needed (may need a git fetch --unshallow origin first since by default we use depth: 1 on a fetch, as we should)

mikehardy avatar Oct 13 '24 22:10 mikehardy

Hello 👋, this issue has been opened for more than 3 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

github-actions[bot] avatar Mar 04 '25 17:03 github-actions[bot]