New issue
Advanced search Search tips

Issue 820570 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 5
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocked on:
issue 725306

Blocking:
issue 806868



Sign in to add a comment

chrome_public_apk close to main dex limit

Project Member Reported by agrieve@chromium.org, Mar 9 2018

Issue description

We are really close to hitting the dex limit when compiling with is_debug=false. 

The bug will track efforts to reduce the size of the main dex (of the multi-dex apk)
 
Current counts:
ChromePublic.apk: 54003 methods
MonochromePublic.apk: 54138 methods

These aren't severe, but downstream we are much closer to the limit!
Labels: -Pri-3 Pri-1

Comment 3 by kbr@chromium.org, Mar 14 2018

Blockedon: 725306
Project Member

Comment 4 by bugdroid1@chromium.org, Mar 16 2018

Project Member

Comment 5 by bugdroid1@chromium.org, Mar 16 2018

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

commit 638786f9e7b2501b483a7f32bfd96f0936173214
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Mar 16 03:16:55 2018

Disallow use of ApplicationStatus on non-main processes

This will help to reduce the main dex size of debug apks.

TBR=agrieve  # ComponentsBrowserTestApplication same as ContentBrowserTestsApplication

Bug:  820570 
Cq-Include-Trybots: luci.chromium.try:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ie380d22faa10d2a9b4527574859770f39800af71
Reviewed-on: https://chromium-review.googlesource.com/962874
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Siddhartha S <ssid@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543608}
[modify] https://crrev.com/638786f9e7b2501b483a7f32bfd96f0936173214/base/android/java/src/org/chromium/base/ApplicationStatus.java
[modify] https://crrev.com/638786f9e7b2501b483a7f32bfd96f0936173214/components/test/android/browsertests_apk/src/org/chromium/components_browsertests_apk/ComponentsBrowserTestsApplication.java
[modify] https://crrev.com/638786f9e7b2501b483a7f32bfd96f0936173214/content/shell/android/browsertests_apk/src/org/chromium/content_browsertests_apk/ContentBrowserTestsApplication.java
[modify] https://crrev.com/638786f9e7b2501b483a7f32bfd96f0936173214/content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ContentShellApplication.java
[modify] https://crrev.com/638786f9e7b2501b483a7f32bfd96f0936173214/gpu/ipc/service/gpu_channel_manager.cc
[modify] https://crrev.com/638786f9e7b2501b483a7f32bfd96f0936173214/gpu/ipc/service/gpu_channel_manager.h
[modify] https://crrev.com/638786f9e7b2501b483a7f32bfd96f0936173214/gpu/ipc/service/gpu_channel_manager_unittest.cc

Woohoo! ChromePublic.apk classes.dex now has 46659 methods and downstream versions are also well under the limit.
Andrew, can we bring the feed back?
Yes! I'm still doing more for this, but should be safe to turn it back on (I'll let you?)
Project Member

Comment 9 by bugdroid1@chromium.org, Mar 16 2018

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

commit 524c5cdd3d57ab87dc90a7d8842dd13921761a69
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Mar 16 21:18:13 2018

Android: Remove ApplicationStatus from @MainDex

ApplicationStatus ends up pulling in a *lot* of code, because it ends up
pulling in all of its listeners.

This reduces the main dex size by 3400 methods.

To get ApplicationStatus to not be included:
 * Made @MainDex applicable to methods
   * Changed ContextUtils and ChromeApplication to annotate methods
     rather than entire class.
 * Stopped using mainDexClasses.rules, in order to avoid -keep'ing all
   of ChromeApplication.attachBaseContext(). Really, all that's needed
   to be kept is the part of attachBaseContext() before the
   ChromiumMultiDexInstaller.install() call.

Bug:  820570 
Change-Id: I39220b99f6d89b2429ba7406619a485179a3b2fc
Reviewed-on: https://chromium-review.googlesource.com/963307
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543831}
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/base/android/java/src/org/chromium/base/ApplicationStatus.java
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/base/android/java/src/org/chromium/base/annotations/MainDex.java
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/base/android/java/src/org/chromium/base/multidex/ChromiumMultiDexInstaller.java
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/build/android/gyp/main_dex_list.py
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/build/android/main_dex_classes.flags
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/build/android/multidex.flags
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/build/config/android/rules.gni
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/chrome/android/java/src/org/chromium/chrome/browser/BrowserRestartActivity.java
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/chrome/test/BUILD.gn
[delete] https://crrev.com/747b7b5664f539ce4f58f4aba37c462d1d5c834a/chrome/test/android/unit_tests_apk/AndroidManifest.xml
[modify] https://crrev.com/524c5cdd3d57ab87dc90a7d8842dd13921761a69/content/shell/android/BUILD.gn

Project Member

Comment 10 by bugdroid1@chromium.org, Mar 19 2018

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

commit 4fdd012a70f6cff4b0fc62b92db09ed84ca4b30f
Author: Andrew Grieve <agrieve@chromium.org>
Date: Mon Mar 19 16:38:22 2018

Android: Don't keep all Services in main dex.

Reduces main dex by 4500 methods

TBR=agrieve  # Trivial change to DecoderService

Bug:  820570 
Change-Id: Ifeeb0cc768dd762c58c6a11061a17e73e12e38a1
Reviewed-on: https://chromium-review.googlesource.com/966877
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544059}
[modify] https://crrev.com/4fdd012a70f6cff4b0fc62b92db09ed84ca4b30f/build/android/main_dex_classes.flags
[modify] https://crrev.com/4fdd012a70f6cff4b0fc62b92db09ed84ca4b30f/chrome/android/java/src/org/chromium/chrome/browser/photo_picker/DecoderService.java

Project Member

Comment 11 by bugdroid1@chromium.org, Mar 20 2018

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

commit 66b933a5eeb1a86db60bccb36869d50e8937d605
Author: Andrew Grieve <agrieve@chromium.org>
Date: Tue Mar 20 01:32:24 2018

Android Multidex: Stop keeping all annotations in main dex

Shrinks main dex method count by 21k

Bug:  820570 
Change-Id: Icff816f23428dca5c6d81e9170d2caf3a1bf40b9
Reviewed-on: https://chromium-review.googlesource.com/966931
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544253}
[modify] https://crrev.com/66b933a5eeb1a86db60bccb36869d50e8937d605/build/android/main_dex_classes.flags

Project Member

Comment 12 by bugdroid1@chromium.org, Mar 21 2018

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

commit 1a33b26602019e9995eae81151bc526208b94912
Author: Andrew Grieve <agrieve@chromium.org>
Date: Wed Mar 21 15:21:26 2018

Android: Remove @MainDex from SelectFileDialog

It's used only by the browser process, so doesn't need to be in the main
dex.

Bug:  820570 
Change-Id: I3f02e1f2d1255452f474db306e68583f8c7200e6
Reviewed-on: https://chromium-review.googlesource.com/971343
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544706}
[modify] https://crrev.com/1a33b26602019e9995eae81151bc526208b94912/ui/android/java/src/org/chromium/ui/base/SelectFileDialog.java

Project Member

Comment 13 by bugdroid1@chromium.org, Mar 23 2018

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

commit 792d73c084465e4d91da4d1bf0a6a2178f2202de
Author: Andrew Grieve <agrieve@chromium.org>
Date: Fri Mar 23 18:04:01 2018

Android: Remove @MainDex from LocationProviderAdapter

Found from auditing suspicious classes in main dex.
https://maps.google.com shows location just fine without it.

Bug:  820570 
Change-Id: Idb0d05c3598d966cbf3cfab667a5d4ddd876c541
Reviewed-on: https://chromium-review.googlesource.com/971022
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Commit-Queue: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545515}
[modify] https://crrev.com/792d73c084465e4d91da4d1bf0a6a2178f2202de/device/geolocation/android/java/src/org/chromium/device/geolocation/LocationProviderAdapter.java

Project Member

Comment 14 by bugdroid1@chromium.org, Mar 26 2018

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

commit faee7beb7acd1da1475d2e0f4a5c938ea3607e96
Author: Andrew Grieve <agrieve@chromium.org>
Date: Mon Mar 26 20:00:13 2018

Android: @MainDex fixes and assertion to prevent @MainDex regressions

* Fixes recently introduced -keep rule for @MainDex on methods:
  * <methods> works, whereas * sometimes doesn't
  * Teach jni_generator.py about @MainDex on methods
* Removes @MainDex from browser-process-only AnimationFrameTimeHistogram
* Removes @MainDex from browser-process-only MediaServerCrashListener
* Adds a missing @MainDex needed to play <video>
  (SurfaceTexturePlatformWrapper, MediaCodecBridgeBuilder)
* Outputs a new artifact from main_dex_list.py for seeing which classes
  are kept by ProGuard vs MainDexListBuilder
* Adds assertions for a few classes that should not appear in the main dex.

With this change, main dex method counts:
chrome_public_apk=7623
chrome_apk=11333
monochrome_apk=22086

NOTRY=true  # ios-simulator is unhappy.

Bug:  820570 
Change-Id: I4ea925ed985a0daa5a4db54a9cf72744fa92aeca
Reviewed-on: https://chromium-review.googlesource.com/972641
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: Theresa <twellington@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545866}
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/base/android/java/src/org/chromium/base/AnimationFrameTimeHistogram.java
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/base/android/java/src/org/chromium/base/ContextUtils.java
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/base/android/java/src/org/chromium/base/library_loader/LegacyLinker.java
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/base/android/jni_generator/jni_generator.py
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/base/android/jni_generator/jni_generator_tests.py
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/build/android/gyp/main_dex_list.py
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/build/android/gyp/util/proguard_util.py
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/build/android/main_dex_classes.flags
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/build/config/android/internal_rules.gni
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/build/config/android/rules.gni
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/chrome/android/java/src/org/chromium/chrome/browser/metrics/UmaUtils.java
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/media/base/android/java/src/org/chromium/media/MediaCodecBridgeBuilder.java
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/media/base/android/java/src/org/chromium/media/MediaServerCrashListener.java
[modify] https://crrev.com/faee7beb7acd1da1475d2e0f4a5c938ea3607e96/ui/android/java/src/org/chromium/ui/gl/SurfaceTexturePlatformWrapper.java

Status: Fixed (was: Assigned)
With the assertion added in #14, we should never hit the limit again :)
Project Member

Comment 16 by bugdroid1@chromium.org, Jun 7 2018

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

commit eb3da8724a3dade4753c8612b4e7abbeaf1cd65b
Author: Eric Stevenson <estevenson@chromium.org>
Date: Thu Jun 07 17:05:29 2018

Android: Remove BuildHooksAndroidImpl from the main dex.

Removing the reference to BuildHooksAndroidImpl from BuildHooksAndroid
shrinks the main dex by ~2000 methods for downstream targets.

Bug:  820570 
Change-Id: If4051f7a85e387129c65a7a5b927183b5b36bf65
Reviewed-on: https://chromium-review.googlesource.com/1089625
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565305}
[modify] https://crrev.com/eb3da8724a3dade4753c8612b4e7abbeaf1cd65b/build/android/buildhooks/BUILD.gn
[modify] https://crrev.com/eb3da8724a3dade4753c8612b4e7abbeaf1cd65b/build/android/buildhooks/java/org/chromium/build/BuildHooksAndroid.java
[add] https://crrev.com/eb3da8724a3dade4753c8612b4e7abbeaf1cd65b/build/android/buildhooks/proguard/build_hooks_android_impl.flags
[modify] https://crrev.com/eb3da8724a3dade4753c8612b4e7abbeaf1cd65b/chrome/android/chrome_public_apk_tmpl.gni

Project Member

Comment 17 by bugdroid1@chromium.org, Jun 8 2018

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

commit 2de6bd20f1ca25f04cbdf190a364506267bb8d9c
Author: Eric Stevenson <estevenson@chromium.org>
Date: Fri Jun 08 14:38:02 2018

Revert "Android: Remove BuildHooksAndroidImpl from the main dex."

This reverts commit eb3da8724a3dade4753c8612b4e7abbeaf1cd65b.

Reason for revert: Breaks downstream builder. See https://crbug.com/850916.

Original change's description:
> Android: Remove BuildHooksAndroidImpl from the main dex.
> 
> Removing the reference to BuildHooksAndroidImpl from BuildHooksAndroid
> shrinks the main dex by ~2000 methods for downstream targets.
> 
> Bug:  820570 
> Change-Id: If4051f7a85e387129c65a7a5b927183b5b36bf65
> Reviewed-on: https://chromium-review.googlesource.com/1089625
> Commit-Queue: Eric Stevenson <estevenson@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#565305}

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

Change-Id: I2d5c807f6b4dfa0639a75ec665af4e494b765a48
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  820570 , 850916
Reviewed-on: https://chromium-review.googlesource.com/1092971
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565629}
[modify] https://crrev.com/2de6bd20f1ca25f04cbdf190a364506267bb8d9c/build/android/buildhooks/BUILD.gn
[modify] https://crrev.com/2de6bd20f1ca25f04cbdf190a364506267bb8d9c/build/android/buildhooks/java/org/chromium/build/BuildHooksAndroid.java
[delete] https://crrev.com/6cdd975c34feaba7bee1bc3572a79575c368f41c/build/android/buildhooks/proguard/build_hooks_android_impl.flags
[modify] https://crrev.com/2de6bd20f1ca25f04cbdf190a364506267bb8d9c/chrome/android/chrome_public_apk_tmpl.gni

Project Member

Comment 18 by bugdroid1@chromium.org, Jun 14 2018

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

commit 9df6d778b2da39ebba7898079dd6562671611047
Author: Eric Stevenson <estevenson@chromium.org>
Date: Thu Jun 14 19:45:42 2018

Reland "Android: Remove BuildHooksAndroidImpl from the main dex."

This is a reland of eb3da8724a3dade4753c8612b4e7abbeaf1cd65b

Original change's description:
> Android: Remove BuildHooksAndroidImpl from the main dex.
> 
> Removing the reference to BuildHooksAndroidImpl from BuildHooksAndroid
> shrinks the main dex by ~2000 methods for downstream targets.
> 
> Bug:  820570 
> Change-Id: If4051f7a85e387129c65a7a5b927183b5b36bf65
> Reviewed-on: https://chromium-review.googlesource.com/1089625
> Commit-Queue: Eric Stevenson <estevenson@chromium.org>
> Reviewed-by: agrieve <agrieve@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#565305}

Bug:  820570 
Change-Id: I8fbaf650ac1ed444ccc6d2cbd3d6d45ab045451f
Reviewed-on: https://chromium-review.googlesource.com/1093111
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567370}
[modify] https://crrev.com/9df6d778b2da39ebba7898079dd6562671611047/build/android/buildhooks/BUILD.gn
[modify] https://crrev.com/9df6d778b2da39ebba7898079dd6562671611047/build/android/buildhooks/java/org/chromium/build/BuildHooksAndroid.java
[add] https://crrev.com/9df6d778b2da39ebba7898079dd6562671611047/build/android/buildhooks/proguard/build_hooks_android_impl.flags
[modify] https://crrev.com/9df6d778b2da39ebba7898079dd6562671611047/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/9df6d778b2da39ebba7898079dd6562671611047/chrome/android/java/src/org/chromium/chrome/browser/ChromeApplication.java

Project Member

Comment 19 by bugdroid1@chromium.org, Jun 15 2018

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/31cb7d39274778a3a3b7b2819b15a410213925b5

commit 31cb7d39274778a3a3b7b2819b15a410213925b5
Author: Eric Stevenson <estevenson@google.com>
Date: Fri Jun 15 16:04:21 2018

Blocking: 806868
Status: Unconfirmed (was: Fixed)
The limit has been reached, preventing to submit new changes:

https://crrev.com/c/1296501
https://crrev.com/c/1316730

The error occurs on chrome_public_apk_for_test, but public_chrome is also very close to the limit with 65251 methods on my local build.

Status: Verified (was: Unconfirmed)
(marking as fixed back since I'm not sure if I my analysis is right, or if this should be a new bug)
Status: Fixed (was: Verified)

Sign in to add a comment