New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 857572 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

MonochromePublic.apk near dex limit (at 64,230) in non-debug builds

Project Member Reported by estevenson@chromium.org, Jun 28 2018

Issue description

https://chromeperf.appspot.com/report?sid=7e9c8966c6811b20fd28fd25bfc2dfdd000215754625d06113205c6888c33bd2&start_rev=564049&end_rev=571189

Currently at 64,230 methods. 65,536 is the max so we've got 1306 left. The short term fix will probably be to just disable the feed library (which adds 3918 methods to MonochromePublic), but we'll need to figure something out here fairly soon.

We could enable multidex for MonochromePublic.apk since we don't ship it anyway but this would really slow things down on K devices.
 
Another option would be to switch to R8 (https://android.googlesource.com/platform/external/r8/). Is it worth giving R8 a try? 
 Issue 868156  has been merged into this issue.
MonochromePublic.apk now has 65,101 methods and is blocking another CL from landing ( issue 868156 ). 

I'm going to try to update our upstream proguard version. Looks like agrieve@ tried this previously but it broke something: https://chromium-review.googlesource.com/c/chromium/src/+/986616. 
Cc: zqzh...@chromium.org
A local build of MonochromePublic.apk shows that we can save 2941 methods by upgrading to 6.0.3. 

I believe the reason we stopped updating upstream proguard was because we don't ship anything that uses it.

I also built cronet_test_instrumentation_apk and it passed. 
That sounds like a great win. Did you already upload a patch to run on the bots? Andrew will hopefully be around in 12 hours to help out with this.
Thanks for working on this.
Note that it was a runtime error the previous time I tried to upgrade it (wasn't clear if you just built the test, or actually ran it). The exact test that broke is here:

https://bugs.chromium.org/p/chromium/issues/detail?id=827285#c2

Another option would be to see r8 would work. It's supposedly proguard flag compatible.

Likely worth disabling feed in the meantime if the simple proguard rev doesn't work in order to unblock code from landing.
Ahh yeah the trybot failed this time as well: https://logs.chromium.org/v/?s=chromium%2Fbb%2Ftryserver.chromium.android%2Fandroid_cronet_tester%2F4016%2F%2B%2Frecipes%2Fsteps%2Fcronet_test_instrumentation_apk%2F0%2Fstdout

agrieve@ mentioned offline that we could check in the new proguard version and enable it only for monochrome_public_apk.

I'll put up a CL for this. 
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 31

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

commit df22b81425e5487282a2569f9fd274300a43e3e5
Author: Eric Stevenson <estevenson@chromium.org>
Date: Tue Jul 31 00:26:42 2018

Use proguard version 6.0.3 for monochrome_public_apk.

MonochromePublic.apk release builds are very close to the main dex limit
and updating proguard reduces the number of methods by about 3000.

6.0.3 cannot be used for all APKs yet due to a bug which causes
cronet_test_instrumentation_apk to fail.

The version mismatch should be temporary as we plan to switch to R8 in the
future ( https://crbug.com/868770 ).

Bug:  857572 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Change-Id: I3676a42908c5046dc3f3831e282150702038140d
Reviewed-on: https://chromium-review.googlesource.com/1154366
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579250}
[modify] https://crrev.com/df22b81425e5487282a2569f9fd274300a43e3e5/build/config/android/rules.gni
[modify] https://crrev.com/df22b81425e5487282a2569f9fd274300a43e3e5/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/df22b81425e5487282a2569f9fd274300a43e3e5/third_party/proguard/README.chromium
[add] https://crrev.com/df22b81425e5487282a2569f9fd274300a43e3e5/third_party/proguard/lib/proguard.6.0.3.jar

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 31

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

commit fb3a9711abe92f1e08611430bd619fd9ff20628e
Author: Nate Fischer <ntfschr@chromium.org>
Date: Tue Jul 31 00:44:37 2018

Revert "Use proguard version 6.0.3 for monochrome_public_apk."

This reverts commit df22b81425e5487282a2569f9fd274300a43e3e5.

Reason for revert: breaks generate_build_files ( http://crbug.com/869231 )

Original change's description:
> Use proguard version 6.0.3 for monochrome_public_apk.
> 
> MonochromePublic.apk release builds are very close to the main dex limit
> and updating proguard reduces the number of methods by about 3000.
> 
> 6.0.3 cannot be used for all APKs yet due to a bug which causes
> cronet_test_instrumentation_apk to fail.
> 
> The version mismatch should be temporary as we plan to switch to R8 in the
> future ( https://crbug.com/868770 ).
> 
> Bug:  857572 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
> Change-Id: I3676a42908c5046dc3f3831e282150702038140d
> Reviewed-on: https://chromium-review.googlesource.com/1154366
> Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
> Commit-Queue: Eric Stevenson <estevenson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#579250}

TBR=yfriedman@chromium.org,estevenson@chromium.org

Change-Id: I1d7e50cf720e9a128a62f06bb34a4c9f6d25c60d
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  857572 
Bug:  869231 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
Reviewed-on: https://chromium-review.googlesource.com/1155502
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Nate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579262}
[modify] https://crrev.com/fb3a9711abe92f1e08611430bd619fd9ff20628e/build/config/android/rules.gni
[modify] https://crrev.com/fb3a9711abe92f1e08611430bd619fd9ff20628e/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/fb3a9711abe92f1e08611430bd619fd9ff20628e/third_party/proguard/README.chromium
[delete] https://crrev.com/96e79bafa83371ce5e334dec900712439990b8a2/third_party/proguard/lib/proguard.6.0.3.jar

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 1

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

commit 4cdc1153e6f6a1182aec5fe42c072db5241b4c3f
Author: Eric Stevenson <estevenson@chromium.org>
Date: Wed Aug 01 08:20:38 2018

Android: Update proguard to 6.0.3.

This is a reland of df22b81425e5487282a2569f9fd274300a43e3e5.

The CL now forwards proguard_jar_path properly.

Original change's description:
> Use proguard version 6.0.3 for monochrome_public_apk.
>
> MonochromePublic.apk release builds are very close to the main dex limit
> and updating proguard reduces the number of methods by about 3000.
>
> 6.0.3 cannot be used for all APKs yet due to a bug which causes
> cronet_test_instrumentation_apk to fail.
>
> The version mismatch should be temporary as we plan to switch to R8 in the
> future ( https://crbug.com/868770 ).
>
> Bug:  857572 
> Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester
> Change-Id: I3676a42908c5046dc3f3831e282150702038140d
> Reviewed-on: https://chromium-review.googlesource.com/1154366
> Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
> Commit-Queue: Eric Stevenson <estevenson@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#579250}

Bug:  857572 
Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ia52b36e04623c5b72e668bb6c07be018ac9210b5
Reviewed-on: https://chromium-review.googlesource.com/1156004
Reviewed-by: David Turner <digit@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579728}
[modify] https://crrev.com/4cdc1153e6f6a1182aec5fe42c072db5241b4c3f/build/config/android/rules.gni
[modify] https://crrev.com/4cdc1153e6f6a1182aec5fe42c072db5241b4c3f/chrome/android/BUILD.gn
[modify] https://crrev.com/4cdc1153e6f6a1182aec5fe42c072db5241b4c3f/third_party/proguard/README.chromium
[add] https://crrev.com/4cdc1153e6f6a1182aec5fe42c072db5241b4c3f/third_party/proguard/lib/proguard.6.0.3.jar

Status: Fixed (was: Assigned)
With #11, the method count is now 61948. Now have 3000 methods of breathing room, which still isn't awesome, but good enough to close this out for now.
Filed  issue 868770  to explore replacing proguard with R8

Sign in to add a comment