New issue
Advanced search Search tips

Issue 883162 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Use r8's dexsplitter for apk bundles

Project Member Reported by agrieve@chromium.org, Sep 12

Issue description

For bundles, we're currently splitting a proguarded .jar into module .jars via:
https://cs.chromium.org/chromium/src/build/android/gyp/generate_proguarded_module_jar.py

This works on .class files, and is so far working fine. However...

We're trying to move to r8 (bug 872904), and r8 produces better code (and runs a bit faster) when outputting .dex rather than .class files.

Rather than maintain our custom module splitting python script, we should use r8's dexsplitter:
https://r8.googlesource.com/r8/+/779702a0d4f48f7b92e26c3f984eb26c0fda670b/src/main/java/com/android/tools/r8/SwissArmyKnife.java#52

Example usage here:
https://r8.googlesource.com/r8/+/779702a0d4f48f7b92e26c3f984eb26c0fda670b/src/test/java/com/android/tools/r8/dexsplitter/DexSplitterTests.java#290

It'd be something like:
java -jar r8.jar dexslitter --input classes.dex --output /tmp/tempdir --proguard-map path_to.mapping --base-jar base1.jar --base-jar base2.jar --feature-jar feature1.jar:MODULE_NAME1 --feature-jar feature2.jar:MODULE_NAME1 ...
 
Cc: -smaier@chromium.org tiborg@chromium.org
Owner: smaier@chromium.org
It has occurred to me that doing this with dexsplitter means we don't have guarantees about a minimal main dex. After talking to Andrew, we think this is fine since modules aren't going out for pre-L devices, where the minimal main dex is really important.
I don't know what minimal main dex is. But at some point we also want to push out bundles for K as well.
It looks like with the current setup we have no minimal main dex anyways, so I won't bother with implementing this yet.
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3599daa6d03cd737cf540ae10403af8f07543bde

commit 3599daa6d03cd737cf540ae10403af8f07543bde
Author: Sam Maier <smaier@chromium.org>
Date: Mon Nov 26 18:02:59 2018

Android: dexsplitter being used for feature modules

Testing done using is_java_debug=false, on patchset #6:
- Ran Monochrome.apks with just the base module, went to a few webpages
- Ran ChromeModernPublic.apks with base module only, few webpages
- Ran Monochrome.apks with base and ar modules, opened AR and it worked
- Ran ChromeModernPublic.apks with base and ar modules, ran VR fine
- Unzipped the .aab files, compared the dex files to current master:
  * base/classes.dex files were byte-for-byte identical for both
    Monochrome and ChromeModernPublic
  * ar/classes.dex file in Monochrome was a bit smaller. In total, we
    went from 170->119 methods, 88->70 types, 27->21 fields, 260->213
    strings
  * vr/classes.dex file in ChromeModernPublic was a bit smaller. We
    went from 1088->1030 methods, 330->316 types, 277->270 fields,
    987->951 strings

Since the base/classes.dex files were identical, and the vr and ar
modules worked, I have good confidence that this is functional.

Bug:  883162 
Change-Id: Id1100193a0787c38680a63fe3b5f85a1c10ef926
Reviewed-on: https://chromium-review.googlesource.com/c/1327571
Commit-Queue: Sam Maier <smaier@chromium.org>
Reviewed-by: Tibor Goldschwendt <tiborg@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610884}
[modify] https://crrev.com/3599daa6d03cd737cf540ae10403af8f07543bde/PRESUBMIT.py
[add] https://crrev.com/3599daa6d03cd737cf540ae10403af8f07543bde/build/android/gyp/dexsplitter.py
[add] https://crrev.com/3599daa6d03cd737cf540ae10403af8f07543bde/build/android/gyp/dexsplitter.pydeps
[delete] https://crrev.com/9ce5a84b39faa643549adf3b9f8ee104767f0e3e/build/android/gyp/generate_proguarded_module_jar.py
[delete] https://crrev.com/9ce5a84b39faa643549adf3b9f8ee104767f0e3e/build/android/gyp/generate_proguarded_module_jar.pydeps
[modify] https://crrev.com/3599daa6d03cd737cf540ae10403af8f07543bde/build/config/android/internal_rules.gni
[modify] https://crrev.com/3599daa6d03cd737cf540ae10403af8f07543bde/build/config/android/rules.gni

Status: Fixed (was: Assigned)

Sign in to add a comment