New issue
Advanced search Search tips

Issue 891475 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 30
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Feature

Blocked on:
issue 892644
issue 894112



Sign in to add a comment

Run dexer (D8) in release mode

Project Member Reported by mheikal@chromium.org, Oct 2

Issue description

Currently the dexer D8 is run in debug mode (default). Running it in release mode may improve binary size by ~77k (for monochrome).

This should be pretty simple, ie. if we are not building a java debug build, pass --release to d8.

However, running d8 in release causes some line numbers to be stripped from the dex files (code that cannot throw exceptions does not get a line number). This might cause issues with apkanalyzer which uses the line numbers to deobfuscate methods in the dex in order to compare two obfuscated dexes method sizes.

Any fix for this bug should check the output of the binary size trybot to ensure that our binary size tools still work correctly after this change.
 
In addition to --release, we should also pass --nodesugaring (already done by this point in the pipeline) and --min-api (might help it optimize more).
Blockedon: 892644
Cl to do what is mentioned in the bug is complete (crrev.com/c/1258059). --min-api is a noop for now because it breaks dexdump that is used catapult/devil. Blocking on dexdump update. Once dexdump is updated will submit another cl to actually pass --min-api to d8.
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 10

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

commit 6c66109f38dad75ecb55643e371679a37c6b99b5
Author: Mohamed Heikal <mheikal@chromium.org>
Date: Wed Oct 10 03:11:42 2018

[Android] Run D8 in release mode for release builds

This cl changes the D8 cmd to run it in release mode if we are building
a non-java-debug build. This change may improve binary size by ~80KB

We also pass --no-desurgaring (already done in pipeline) and --min-api
(might help enable more optimisations). However --min-api is just a noop
until catapult/devil dexdump is updated ( crbug.com/892644 ).

Bug:  891475 
Change-Id: Ie5ec84da2a09ed1054d9531ba324cdf098fc689c
Reviewed-on: https://chromium-review.googlesource.com/c/1258059
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598196}
[modify] https://crrev.com/6c66109f38dad75ecb55643e371679a37c6b99b5/build/android/gyp/dex.py
[modify] https://crrev.com/6c66109f38dad75ecb55643e371679a37c6b99b5/build/config/android/internal_rules.gni
[modify] https://crrev.com/6c66109f38dad75ecb55643e371679a37c6b99b5/build/config/android/rules.gni
[modify] https://crrev.com/6c66109f38dad75ecb55643e371679a37c6b99b5/chrome/android/BUILD.gn
[modify] https://crrev.com/6c66109f38dad75ecb55643e371679a37c6b99b5/chrome/android/chrome_public_apk_tmpl.gni

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 10

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

commit bc3c78be762f4d207940840ced20d51b9048b65d
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Wed Oct 10 03:54:29 2018

Revert "[Android] Run D8 in release mode for release builds"

This reverts commit 6c66109f38dad75ecb55643e371679a37c6b99b5.

Reason for revert:
Let me speculatively revert this for compile failures on the android-rel bot:
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/android-rel/2974

Original change's description:
> [Android] Run D8 in release mode for release builds
> 
> This cl changes the D8 cmd to run it in release mode if we are building
> a non-java-debug build. This change may improve binary size by ~80KB
> 
> We also pass --no-desurgaring (already done in pipeline) and --min-api
> (might help enable more optimisations). However --min-api is just a noop
> until catapult/devil dexdump is updated ( crbug.com/892644 ).
> 
> Bug:  891475 
> Change-Id: Ie5ec84da2a09ed1054d9531ba324cdf098fc689c
> Reviewed-on: https://chromium-review.googlesource.com/c/1258059
> Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598196}

TBR=agrieve@chromium.org,mheikal@chromium.org

Change-Id: I3646418b28a2b8411529414433c5d74dd382ea56
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  891475 
Reviewed-on: https://chromium-review.googlesource.com/c/1272737
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598204}
[modify] https://crrev.com/bc3c78be762f4d207940840ced20d51b9048b65d/build/android/gyp/dex.py
[modify] https://crrev.com/bc3c78be762f4d207940840ced20d51b9048b65d/build/config/android/internal_rules.gni
[modify] https://crrev.com/bc3c78be762f4d207940840ced20d51b9048b65d/build/config/android/rules.gni
[modify] https://crrev.com/bc3c78be762f4d207940840ced20d51b9048b65d/chrome/android/BUILD.gn
[modify] https://crrev.com/bc3c78be762f4d207940840ced20d51b9048b65d/chrome/android/chrome_public_apk_tmpl.gni

Blockedon: 894112
Reverted because somehow leak canary is breaking in d8 --release. Since release canary is not being actively used/developed on, I created a bug and cl to remove it (see  issue 894112 ). Blocking on it to try and reland the change.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 11

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

commit 4468f175eb1447cf20ec8ae2e4a5b0f5f0a3e3bb
Author: Mohamed Heikal <mheikal@chromium.org>
Date: Thu Oct 11 02:24:24 2018

Reland "[Android] Run D8 in release mode for release builds"

This is a reland of 6c66109f38dad75ecb55643e371679a37c6b99b5

Fixed build error due to LeakCanary by removing the unused library
https://crrev.com/c/1274050

TBR=agrieve@chromium.org

Original change's description:
> [Android] Run D8 in release mode for release builds
>
> This cl changes the D8 cmd to run it in release mode if we are building
> a non-java-debug build. This change may improve binary size by ~80KB
>
> We also pass --no-desurgaring (already done in pipeline) and --min-api
> (might help enable more optimisations). However --min-api is just a noop
> until catapult/devil dexdump is updated ( crbug.com/892644 ).
>
> Bug:  891475 
> Change-Id: Ie5ec84da2a09ed1054d9531ba324cdf098fc689c
> Reviewed-on: https://chromium-review.googlesource.com/c/1258059
> Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#598196}

Bug:  891475 
Change-Id: I9aa40d1b7ecfd9bd42ab6452d8b25261e153f268
Reviewed-on: https://chromium-review.googlesource.com/c/1274600
Reviewed-by: Mohamed Heikal <mheikal@chromium.org>
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598637}
[modify] https://crrev.com/4468f175eb1447cf20ec8ae2e4a5b0f5f0a3e3bb/build/android/gyp/dex.py
[modify] https://crrev.com/4468f175eb1447cf20ec8ae2e4a5b0f5f0a3e3bb/build/config/android/internal_rules.gni
[modify] https://crrev.com/4468f175eb1447cf20ec8ae2e4a5b0f5f0a3e3bb/build/config/android/rules.gni
[modify] https://crrev.com/4468f175eb1447cf20ec8ae2e4a5b0f5f0a3e3bb/chrome/android/BUILD.gn
[modify] https://crrev.com/4468f175eb1447cf20ec8ae2e4a5b0f5f0a3e3bb/chrome/android/chrome_public_apk_tmpl.gni

Status: Fixed (was: Assigned)
https://chromium-review.googlesource.com/c/chromium/src/+/1357379

This has been fixed.

Sign in to add a comment