New issue
Advanced search Search tips

Issue 901488 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Task

Blocked on:
issue 903820

Blocking:
issue 864634
issue 898840
issue 901331
issue 901698



Sign in to add a comment

Switch chrome_public_apk_for_test to be multidex

Project Member Reported by s...@chromium.org, Nov 2

Issue description

Patches being sent to the CQ that add very few methods are failing due to DEX size. We must be very close to the limit.

https://ci.chromium.org/p/chromium/builders/luci.chromium.try/android-kitkat-arm-rel/117397

FAILED: gen/chrome/android/chrome_public_apk_for_test/classes.dex.zip 
python ../../build/android/gyp/dex.py --depfile gen/chrome/android/chrome_public_apk_for_test__final_dex.d --dex-path gen/chrome/android/chrome_public_apk_for_test/classes.dex.zip gen/chrome/android/chrome_public_apk_for_test/classes.dex.zip.proguard.jar --release --d8-jar-path ../../third_party/r8/lib/d8.jar
Traceback (most recent call last):
  File "../../build/android/gyp/dex.py", line 347, in <module>
    sys.exit(main(sys.argv[1:]))
  File "../../build/android/gyp/dex.py", line 325, in main
    _RunD8(dex_cmd, paths, options.dex_path)
  File "../../build/android/gyp/dex.py", line 152, in _RunD8
    build_utils.CheckOutput(dex_cmd, print_stderr=False)
  File "/b/s/w/ir/cache/builder/src/build/android/gyp/util/build_utils.py", line 218, in CheckOutput
    raise CalledProcessError(cwd, args, stdout + stderr)
util.build_utils.CalledProcessError: Command failed: ( cd /b/s/w/ir/cache/builder/src/out/Release; java -jar ../../third_party/r8/lib/d8.jar --no-desugaring --release --output gen/chrome/android/chrome_public_apk_for_test/classes.dex.zip gen/chrome/android/chrome_public_apk_for_test/classes.dex.zip.proguard.jar )
Error: Cannot fit requested classes in a single dex file (# methods: 65549 > 65536). Try supplying a main-dex list
Compilation failed


 
Components: Build
Owner: agrieve@chromium.org
Status: Assigned (was: Untriaged)
I could be reading this wrong, but isn't it already?:
https://codesearch.chromium.org/chromium/src/chrome/android/BUILD.gn?rcl=8b4180419a3ecbf935bcedc6d79ab9d07647e7c8&l=1729
We seem to be having the same issue on a few of our patches:

 https://ci.chromium.org/b/8930690749683015808

Interestingly, that target builds locally with the same GN args as the tryjob.
Cc: estevenson@chromium.org smaier@chromium.org
Status: Started (was: Assigned)
Blocking: 901331
Cc: barkerd@google.com
Blocking: 898840
What I've figured out so far:
* The multidex support library does not support both the test apk and the under-test apk being multidex.
* Removing the classes.dex from the under-dex apk doesn't work

Options for buying time:
* Find some deps that can be moved from the under-test apk -> test apk
* Disable feed (only feature I know about that can be easily switched off)
* Enable r8 for targets (bug 872904 - already being worked on, but not quite ready yet)

The longer-term fix:
* Merge the two apks (bug 890452)
Update: 
I tried enabling r8 for the problem targets (https://chromium-review.googlesource.com/c/chromium/src/+/1318072) but that also fails:

Error: Library class android.test.AndroidTestRunner extends program class junit.runner.BaseTestRunner
Error: Library class android.test.suitebuilder.TestSuiteBuilder$FailedToCreateTests extends program class junit.framework.TestCase
Error: Library class android.test.AndroidTestCase extends program class junit.framework.TestCase
Error: Library class android.test.InstrumentationTestCase extends program class junit.framework.TestCase
Error: Library class android.test.InstrumentationTestSuite extends program class junit.framework.TestSuite
Compilation failed with an internal error.
java.lang.NullPointerException
        at com.android.tools.r8.shaking.Enqueuer.transitionDefaultMethodsForInstantiatedClass(Enqueuer.java:943)
        at com.android.tools.r8.shaking.Enqueuer.transitionDefaultMethodsForInstantiatedClass(Enqueuer.java:944)
        at com.android.tools.r8.shaking.Enqueuer.transitionMethodsForInstantiatedClass(Enqueuer.java:929)


I think for now we should disable feed to unblock CQ while we continue to work on a proper fix:
https://chromium-review.googlesource.com/c/chromium/src/+/1318669
Cc: twelling...@chromium.org janicewong@chromium.org
We discussed this as a team and unblocking the CQ is more important than having the Feed enabled right now. We just shut off our Canary/Dev Feed experiment to fix crashes, but we were hoping to be back in live experimentation this week .Disabling the Feed is almost certainly delaying our plans.

Would it be possible to get this resolved and the Feed re-enabled by next week's Dev push? If we're not back in live experimentation in Dev by next week we'll very likely miss our goal of Beta/1% Stable experimentation in M71.
Project Member

Comment 11 by bugdroid1@chromium.org, Nov 6

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

commit 419c073bc05dab4d76ce541671600fdd14d00d06
Author: Andrew Grieve <agrieve@chromium.org>
Date: Tue Nov 06 00:23:12 2018

Android: Temporarily disable Feed to fix CQ failures

Bug:  901488 
Change-Id: I7c98a48ab86533ee206ef7302e085e39be76cd7c
Reviewed-on: https://chromium-review.googlesource.com/c/1318669
Commit-Queue: Filip Gorski <fgorski@chromium.org>
Reviewed-by: Filip Gorski <fgorski@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605529}
[modify] https://crrev.com/419c073bc05dab4d76ce541671600fdd14d00d06/components/feed/features.gni

Blocking: 864634
Blocking: 901698
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 8

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

commit ecac5a9e2fd689a304c899f0ee6d3f2c230b8070
Author: Andrew Grieve <agrieve@chromium.org>
Date: Thu Nov 08 01:57:01 2018

Merge most chrome test apks

chrome_public_test_apk is now the union of:
* chrome_public_apk_for_test
* chrome_public_test_apk
* chrome_sync_shell_apk
* chrome_sync_shell_test_apk

Motivation is to simplify the targets, as well as to work around
multidex restrictions.

This change disabled two tests because fixing wasn't trivial:
* FirstRunIntegrationTest.java
* FirstRunTest.java
https://crbug.com/902774 will track un-@Disable'ing them.


NOTRY=true  # linux bot flakiness
TBR=agrieve  # Trivial rename in a couple files.

Bug:  833545 ,  841628 ,  901488 
Change-Id: I9ebe035e86f1b6e5a208e36040baf038efa6cc15
Reviewed-on: https://chromium-review.googlesource.com/c/1319335
Commit-Queue: agrieve <agrieve@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606283}
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/BUILD.gn
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/WATCHLISTS
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/base/test/android/javatests/src/org/chromium/base/test/BaseChromiumAndroidJUnitRunner.java
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/build/android/gradle/generate_gradle.py
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/build/android/main_dex_classes.flags
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/BUILD.gn
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/chrome_public_apk_tmpl.gni
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/java/AndroidManifest.xml
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/java/src/org/chromium/chrome/browser/sync/ui/SyncCustomizationFragment.java
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/java_sources.gni
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/AndroidManifest.xml
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/AndroidManifest_monochrome.xml
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/DEPS
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/OriginVerifierTest.java
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/browserservices/TrustedWebActivityTest.java
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/firstrun/FirstRunIntegrationTest.java
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageUtilsTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/AutofillTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/BookmarksTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/FakeServerHelper.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/FirstRunTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/GmsCoreSyncListenerTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/OpenTabsTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/SyncCustomizationFragmentTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/SyncTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/SyncTestRule.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/TypedUrlsTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/UkmTest.java
[rename] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/android/javatests/src/org/chromium/chrome/browser/sync/ui/PassphraseTypeDialogFragmentTest.java
[delete] https://crrev.com/939f1a0e0c1bf8f2453b8f705471463f1455cab0/chrome/android/sync_shell/DEPS
[delete] https://crrev.com/939f1a0e0c1bf8f2453b8f705471463f1455cab0/chrome/android/sync_shell/README
[delete] https://crrev.com/939f1a0e0c1bf8f2453b8f705471463f1455cab0/chrome/android/sync_shell/javatests/AndroidManifest.xml
[delete] https://crrev.com/939f1a0e0c1bf8f2453b8f705471463f1455cab0/chrome/android/sync_shell/javatests/DEPS
[delete] https://crrev.com/939f1a0e0c1bf8f2453b8f705471463f1455cab0/chrome/browser/android/chrome_sync_shell_entry_point.cc
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/browser/metrics/ukm_browsertest.cc
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/chrome/test/android/test_trusted_web_activity/src/org/chromium/chrome/browser/browserservices/TestTrustedWebActivityService.java
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/components/sync/BUILD.gn
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/content/shell/BUILD.gn
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/docs/android_test_instructions.md
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/testing/buildbot/chromium.android.fyi.json
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/testing/buildbot/chromium.android.json
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/testing/buildbot/gn_isolate_map.pyl
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/testing/buildbot/test_suite_exceptions.pyl
[modify] https://crrev.com/ecac5a9e2fd689a304c899f0ee6d3f2c230b8070/testing/buildbot/test_suites.pyl

Does this patch landing mean we're good to re-enable the Feed?

If so, we'll wait a day to make sure nothing blows up. I'm thinking we can re-enable after the CL in #14 has had time to work it's way through the bots and has been compiled into an official build.
Owner: estevenson@chromium.org
It does! Certainly a good plan to wait until tomorrow to re-enable feed to make sure this sticks. If it doesn't, assigning to estevenson@ for any follow-up.
Well, unfortunately chrome_public_apk had just a couple fewer methods than chrome_public_apk_for_test, and it is now going over the limit when we try to enable the Feed. Is it possible to make chrome_public_apk multidex? Since it isn't actually shipped to users, we may be less concerned about the startup penalty on old devices.

https://chromium-review.googlesource.com/c/chromium/src/+/1327141

FAILED: gen/chrome/android/chrome_public_apk/classes.dex.zip 
python ../../build/android/gyp/dex.py --depfile gen/chrome/android/chrome_public_apk__final_dex.d --dex-path gen/chrome/android/chrome_public_apk/classes.dex.zip gen/chrome/android/chrome_public_apk/classes.dex.zip.proguard.jar --release --min-api 16 --d8-jar-path ../../third_party/r8/lib/d8.jar
Traceback (most recent call last):
  File "../../build/android/gyp/dex.py", line 347, in <module>
    sys.exit(main(sys.argv[1:]))
  File "../../build/android/gyp/dex.py", line 325, in main
    _RunD8(dex_cmd, paths, options.dex_path)
  File "../../build/android/gyp/dex.py", line 152, in _RunD8
    build_utils.CheckOutput(dex_cmd, print_stderr=False)
  File "/b/s/w/ir/cache/builder/src/build/android/gyp/util/build_utils.py", line 218, in CheckOutput
    raise CalledProcessError(cwd, args, stdout + stderr)
util.build_utils.CalledProcessError: Command failed: ( cd /b/s/w/ir/cache/builder/src/out/Release; java -jar ../../third_party/r8/lib/d8.jar --no-desugaring --release --output gen/chrome/android/chrome_public_apk/classes.dex.zip gen/chrome/android/chrome_public_apk/classes.dex.zip.proguard.jar )
Error: Cannot fit requested classes in a single dex file (# methods: 65605 > 65536). Try supplying a main-dex list
Compilation failed

I'll put up a CL to enable multidex for chrome_public_apk and chrome_modern_public_apk (we already do it for monochrome_public_apk)
Submitting the CL to enable multidex and feed here: https://chromium-review.googlesource.com/c/chromium/src/+/1327403
Thanks for the quick fix Eric! When this lands, it will unblock our whole team. Much appreciated!
Hopefully it sticks :)
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 9

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

commit a0bf7a19945107dcda1cba3e9d118c9894e2aa2e
Author: Eric Stevenson <estevenson@chromium.org>
Date: Fri Nov 09 00:41:40 2018

Android: Enable feed and multidex.

chrome_modern_public_apk and chrome_public_apk hit the single dex
method limit when feed is enabled. This CL re-enables feed while also
turning on multidex for the required APKs.

Binary-Size: Feed was disabled to fix the multidex method limit issue.
Bug:  901488 
Change-Id: Ifba41178fad6c206ec1f458d420187ef1c2eb6f3
Reviewed-on: https://chromium-review.googlesource.com/c/1327403
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606670}
[modify] https://crrev.com/a0bf7a19945107dcda1cba3e9d118c9894e2aa2e/chrome/android/BUILD.gn
[modify] https://crrev.com/a0bf7a19945107dcda1cba3e9d118c9894e2aa2e/chrome/android/monochrome/scripts/monochrome_apk_checker.py
[modify] https://crrev.com/a0bf7a19945107dcda1cba3e9d118c9894e2aa2e/components/feed/features.gni

Blockedon: 903820
Status: Fixed (was: Started)

Sign in to add a comment