1468487 - Method count is getting close to the 65k limit for some local build configurations

Status: RESOLVED FIXED

(Firefox Build System :: Android Studio and Gradle Integration, enhancement)

Description

[Jan Henning [:JanH]]

... and upgrading the support library versions makes things worse.

Comment 1

[Petru-Mugurel Lingurar [:petru]]

As discussed here bug 1414054 comment 2 building with mach which just runs `officialWithoutGeckoBinariesNoMinApiPhotonDebug` is still possible.

Maybe we could enable multidex for the local builds to start?
And only when truly needed enable multidex on the official builds? (As multidex means an increase in apk size and a slightly slower first startup)

Comment 2

[Nick Alexander :nalexander [he/him]]

(In reply to Petru-Mugurel Lingurar[:petru] from comment #1)
> As discussed here bug 1414054 comment 2 building with mach which just runs
> `officialWithoutGeckoBinariesNoMinApiPhotonDebug` is still possible.
> 
> Maybe we could enable multidex for the local builds to start?
> And only when truly needed enable multidex on the official builds? (As
> multidex means an increase in apk size and a slightly slower first startup)

Yes, we can do this.  I tried to find my discussions of multiDex with Max Liu and couldn't; let me try again.  OK, they were in https://bugzilla.mozilla.org/show_bug.cgi?id=1411654 and related bugs that you're on, Petru.

This should Just Work after enabling multiDex in the Gradle file _and_ replacing `GeckoApplication` with something that extends `MultiDexApplication` or whatever in the code.  It's that part that's tricky.  It's possible that we don't need to do that at all any more; I think it was needed perhaps for API v15 and below, which we no longer target?

So yes, it would be helpful if somebody could try to do this for non-official builds (local flavor dimension).

Comment 3

[Petru-Mugurel Lingurar [:petru]]

According to https://developer.android.com/studio/build/multidex
"Versions of the platform prior to Android 5.0 (API level 21) use the Dalvik runtime for executing app code. By default, Dalvik limits apps to a single classes.dex bytecode file per APK. In order to get around this limitation, you can add the multidex support library to your project"

So I think we can just include the dependencies in gradle, including `multiDexEnabled true` but only enable Multidex from inside the Application if the flavor is local and minSdk < 21.

Comment 4

[Petru-Mugurel Lingurar [:petru]]

Attachment: Bug 1468487 - Enable multidex for local builds;

Review commit: https://reviewboard.mozilla.org/r/250936/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/250936/

Comment 5

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8985316 [details]
Bug 1468487 - Enable multidex for local builds;

https://reviewboard.mozilla.org/r/250936/#review257376

If try is green and this works for you locally, I'm okay with it.  (Try will verify that we don't accidentally lose some DEX code when we package/re-package.)

::: mobile/android/app/build.gradle:223
(Diff revision 1)
>      implementation "com.android.support:cardview-v7:$support_library_version"
>      implementation "com.android.support:recyclerview-v7:$support_library_version"
>      implementation "com.android.support:design:$support_library_version"
>      implementation "com.android.support:customtabs:$support_library_version"
>      implementation "com.android.support:palette-v7:$support_library_version"
> +    implementation "com.android.support:multidex:1.0.3"

Worth a comment:
```We always include support for multidexing even when we don't enable it at runtime.  We could conditionally include support, but we'd need to generate the `Application` class or fork the file on disk.```

::: mobile/android/base/java/org/mozilla/gecko/GeckoApplication.java:376
(Diff revision 1)
>  
> +    @Override
> +    protected void attachBaseContext(Context base) {
> +        super.attachBaseContext(base);
> +
> +        // We want Multidex enabled only for local builds atm

nit: full sentence, no abbreviation:

```
// Enable multiDex only for local builds.
```

Comment 6

[Nick Alexander :nalexander [he/him]]

Robocop and the android-* suites (lint, etc) would be good to test on try.

Comment 7

[Petru-Mugurel Lingurar [:petru]]

Comment on attachment 8985316 [details]
Bug 1468487 - Enable multidex for local builds;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/250936/diff/1-2/

Comment 8

[Petru-Mugurel Lingurar [:petru]]

Updated the patch with your suggestions, thanks!

For me the patch does it's job.
After importing the patch from bug 1385464 and doing a local build I see
> Total methods in app-local-withGeckoBinaries-minApi21-photon-debug.apk: 66991 (102.22% used)
> Total fields in app-local-withGeckoBinaries-minApi21-photon-debug.apk:  43603 (66.53% used)
> Total classes in app-local-withGeckoBinaries-minApi21-photon-debug.apk:  8706 (13.28% used)

I also pushed to try with all tests included, all seems fine - https://treeherder.mozilla.org/#/jobs?repo=try&revision=3acd7bd415941b3f744ff72f3c877c2f953db8df

Comment 9

[Petru-Mugurel Lingurar [:petru]]

After Andrei's recent patch for updating Google Play Services which added play-services-ads and ~10.000 the new method stats are:
> Total methods in app-official-withoutGeckoBinaries-noMinApi-photon-debug.apk: 76300 (116.43% used)
> Total fields in app-official-withoutGeckoBinaries-noMinApi-photon-debug.apk:  47925 (73.13% used)
> Total classes in app-official-withoutGeckoBinaries-noMinApi-photon-debug.apk:  8820 (13.46% used)

This is a very big increase in method count and in apk size and needs to be investigated on how can it's impact be reduced but probably we'll need to multidex on the official builds also.

Comment 10

[Petru-Mugurel Lingurar [:petru]]

Latest stats from mozilla-central from :app:countLocalWithGeckoBinariesMinApi21PhotonDebugDexMethods 

> Total methods in app-local-withGeckoBinaries-minApi21-photon-debug.apk: 65624 (100.14% used)
> Total fields in app-local-withGeckoBinaries-minApi21-photon-debug.apk:  38869 (59.31% used)
> Total classes in app-local-withGeckoBinaries-minApi21-photon-debug.apk:  8418 (12.85% used)

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4eb6f5567674
Enable multidex for local builds; r=nalexander

Comment 12

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/4eb6f5567674