Bug 1341990 is overwhelming our string table (Default maximum: 65536)[1] during Gradle build. Following are some statistics regarding our current string table usage: mach build / Gradle build before 1341990[1] 58419 / 64097 after 1341990[2] 58419 / 70840 This is observed by running `jar xvf app.apk classes.dex && hexdump -s 56 -n 4 -e '1/4 "%d\n"' classes.dex` as suggested from https://stackoverflow.com/a/40743786/3591480 (Verification on this approach could be needed.) Seems like it is already a must on Gradle build and we are closing to 65536 on mach build too. [1] Can't find a canonical source for jumbo mode but maybe these would be useful: https://developers.soundcloud.com/blog/congratulations-you-have-a-lot-of-code-remedying-androids-method-limit-part-1 https://source.android.com/devices/tech/dalvik/dalvik-bytecode

I've filed Bug 1370158 to keep track why only Gradle build is influenced by Bug 1341990. My guess is something is optimized in mach build.

I pushed a commit with some docs about how the moz.build (Makefile-based) and Gradle build systems interact. It's background to this ticket. See also Bug 1286677, which is the "we should think about this" version of this ticket. There are several things at play here: 1. string count vs. method count 2. jumbo mode (in moz.build and Gradle) 3. why Bug 1341990 ballooned the Gradle build but not the moz.build 4. multiDex mode (in moz.build and Gradle) 5. debug vs. release build types 1. and 2.: jumbo mode will address the string count problems. It's easy to enable jumbo mode in both moz.build and Gradle. I'd like to understand 3. before charging ahead with jumbo mode, since there shouldn't be much difference here. 3. The after builds are at [1], the before builds are at [2]. I find: ⋊> ~/Downloads jar xvf target-before-make.apk classes.dex ; and hexdump -s 56 -n 4 -e '1/4 "%d\n"' classes.dex 52590 ⋊> ~/Downloads jar xvf target-after-make.apk classes.dex ; and hexdump -s 56 -n 4 -e '1/4 "%d\n"' classes.dex 58488 ⋊> ~/Downloads jar xvf target-before-gradle.apk classes.dex ; and hexdump -s 56 -n 4 -e '1/4 "%d\n"' classes.dex 59743 ⋊> ~/Downloads jar xvf target-after-gradle.apk classes.dex ; and hexdump -s 56 -n 4 -e '1/4 "%d\n"' classes.dex 65583 So I see ~6000 strings in both Make and Gradle. That's good, 'cuz I have no idea why they would have different rates of change (the Proguard configurations are very similar). 4. We have multiDex mode enabled for the `local` Gradle configuration, but not `localOld` or `official`. There's a performance cost (in theory), and at one time I was worried about API levels supporting `multiDex`, but maybe those issues are solved now. If it's safe, I'd love to DEX HLS and Sync and Google Play Services and a bunch of things separately, so we don't need to load that code at startup time. It's not reasonable to try to multiDex in moz.build. If we *need* multiDex, we have to use Gradle to produce our release builds of Fennec. 5. I'd like to keep `debug` builds working, 'cuz otherwise it's too painful to develop. We could enable `multiDex` for all `local` builds/all `debug` builds. My conclusion is that we should immediately turn on jumbo mode (this ticket), both in moz.build and Gradle. And we need to figure out what the `multiDex` compatibility story really is, which I don't have time for right now. :tyu, can you 1. test with --force-jumbo in Makefile.in and Gradle and 2. dig into multiDex? [1] https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=ffaa07672466b06cd748b07a34cf95377afdde41 [2] https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=96e18bec9fc8a5ce623c16167c12756bbe190d73

(In reply to Nick Alexander :nalexander from comment #3) > So I see ~6000 strings in both Make and Gradle. That's good, 'cuz I have no > idea why they would have different rates of change (the Proguard > configurations are very similar). > 1. Great to see both builds are balloning, I'll take a look at what's going on on my local device. (Maybe it's because I'm building x86? I'll look into this later.) > 4. We have multiDex mode enabled for the `local` Gradle configuration, but > not `localOld` or `official`. There's a performance cost (in theory), and > at one time I was worried about API levels supporting `multiDex`, but maybe > those issues are solved now. If it's safe, I'd love to DEX HLS and Sync and > Google Play Services and a bunch of things separately, so we don't need to > load that code at startup time. We'll need multidex support library for that. and we did already have it in our code base. https://developer.android.com/studio/build/multidex.html#limitations There are some limitations, mentioned here, and if you're wondering what linearalloc limit is I've read this to figure out: https://rschilling.wordpress.com/category/android/linearalloc/ Simply put: There are still risk with multi-dex-support-library that app may fail at run time. The limit is extended from 5MB to 8MB since ICS(4.0) and since we're targeting 4.1 the risk is not that high but safety is not guaranteed. My personal opinion is we'll need them as we pour new features so we could give it a try and guard our users with some testings. > > It's not reasonable to try to multiDex in moz.build. If we *need* multiDex, > we have to use Gradle to produce our release builds of Fennec. > Sounds like yet another good reason to go for Gradle :) > 5. I'd like to keep `debug` builds working, 'cuz otherwise it's too painful > to develop. We could enable `multiDex` for all `local` builds/all `debug` > builds. > > My conclusion is that we should immediately turn on jumbo mode (this > ticket), both in moz.build and Gradle. And we need to figure out what the > `multiDex` compatibility story really is, which I don't have time for right > now. > > :tyu, can you 1. test with --force-jumbo in Makefile.in and Gradle and 2. > dig into multiDex? Since its soft code freeze now, it may not be a good timing to change the behavior of our release product. I'm thinking of landing just flip jumbomode to debugs builds to quick fix this problem and maybe add this to release later. I'll also have some time to make sure this is a good idea, too. (Though seems like with Gradle plugin 2.1.0 jumbo mode is default on, this sounds promising.) For the same reason I'll try --force-jumbo later. > > [1] > https://treeherder.mozilla.org/#/jobs?repo=mozilla- > central&revision=ffaa07672466b06cd748b07a34cf95377afdde41 > [2] > https://treeherder.mozilla.org/#/jobs?repo=mozilla- > central&revision=96e18bec9fc8a5ce623c16167c12756bbe190d73

> We'll need multidex support library for that. and we did already have it in > our code base. We already have it for Gradle, but for the `local` configuration and not the `official` configuration (which is the one that will be used eventually in automation). We definitely don't have multiDex support in moz.build. > Simply put: There are still risk with multi-dex-support-library that app may > fail at run time. Yes, but I don't think we're at particularly high risk for the linear allocation limit. > The limit is extended from 5MB to 8MB since ICS(4.0) and since we're > targeting 4.1 Our classes.dex is small, and could be profitably split into largely independent pieces. HLS, for example, is completely isolated. > > the risk is not that high but safety is not guaranteed. My personal opinion > is we'll need them > > as we pour new features so we could give it a try and guard our users with > some testings. I concur. > > It's not reasonable to try to multiDex in moz.build. If we *need* multiDex, > > we have to use Gradle to produce our release builds of Fennec. > > > > Sounds like yet another good reason to go for Gradle :) Aye, and it might be a blocker. > > 5. I'd like to keep `debug` builds working, 'cuz otherwise it's too painful > > to develop. We could enable `multiDex` for all `local` builds/all `debug` > > builds. > > > > My conclusion is that we should immediately turn on jumbo mode (this > > ticket), both in moz.build and Gradle. And we need to figure out what the > > `multiDex` compatibility story really is, which I don't have time for right > > now. > > > > :tyu, can you 1. test with --force-jumbo in Makefile.in and Gradle and 2. > > dig into multiDex? > > Since its soft code freeze now, it may not be a good timing to change the > behavior of our release product. I'm really not concerned about jumbo mode, even for release. More below. > I'm thinking of landing just flip jumbomode to debugs builds to quick fix > this problem and maybe add this to release later. Yes. > I'll also have some time to make sure this is a good idea, too. > > (Though seems like with Gradle plugin 2.1.0 jumbo mode is default on, this > sounds promising.) > > For the same reason I'll try --force-jumbo later. We should definitely enable jumbo mode in all Gradle debug configurations: local developers need it! We should probably enable jumbo mode in all Gradle configurations: it's the future. I'd prefer to enable jumbo mode in moz.build so that the Gradle configuration and the moz.build configuration stay as close together as possible. This will increase our APK size (a little) but should be safe in the meantime. :tyu, what do you think?

> We should definitely enable jumbo mode in all Gradle debug configurations: > local developers need it! > > We should probably enable jumbo mode in all Gradle configurations: it's the > future. > > I'd prefer to enable jumbo mode in moz.build so that the Gradle > configuration and the moz.build configuration stay as close together as > possible. This will increase our APK size (a little) but should be safe in > the meantime. > > :tyu, what do you think? I think it's a good idea to keep Gradle and mach build as consistent as possible. I agree with the idea there is barely any risk with jumbo mode, if any. However, I still feel like it is worth it to avoid surprising our Sheriffs by landing this right before merge. This could be a good practice concerning we're in the post-dawn world without Aurora. So I'm proposing yet another idea: maybe we could land them together but after the merge day (June 12th). What's your opinion on this plan? I'd also like to invite :janH to give me some feedback regarding this plan. Do you think it is acceptable to drag this problem a little bit longer to not interfere the merge?

Some other random findings: I've took a look into `jar xvf app.apk classes.dex && hexdump -s 56 -n 4 -e '1/4 "%d\n"' classes.dex` and indeed we are obtaining this string_ids_size from the header. (ref: https://source.android.com/devices/tech/dalvik/dex-format#header-item ) So it's a definite item rather than an approximate so I have no idea why ⋊> ~/Downloads jar xvf target-after-gradle.apk classes.dex ; and hexdump -s 56 -n 4 -e '1/4 "%d\n"' classes.dex 65583 mentioned earlier is not problematic, it's over 65536 already!

The try is here :) https://treeherder.mozilla.org/#/jobs?repo=try&revision=9614fa4b974d0e1984fa805e849957d4c5f8d47a I've verified jumbo is there by utilizing `./dexdump -d ~/Downloads/target-before-mach.apk | grep jumbo | head` One thing that was unexpected is: ** The Gradle build on CI is already utilizing Jumbo mode!! ** :janH You're right again, there are some unexpected differences in the Treeherder Gradle build. So this basically answers: > ⋊> ~/Downloads jar xvf target-after-gradle.apk classes.dex ; and hexdump -s > 56 -n 4 -e '1/4 "%d\n"' classes.dex > 65583 > > mentioned earlier is not problematic, it's over 65536 already! I suspect it could be caused by build machine running a newer version then we suggested in mach bootstrap and is defaulting jumbo? I'll dig into this later. Some more stats regarding the file sizes: tyu-40227:Downloads tyu$ stat -f%z target-before-mach.apk 36107561 tyu-40227:Downloads tyu$ stat -f%z target-after-mach.apk 36110736 tyu-40227:Downloads tyu$ stat -f%z target-before-gradle.apk 36733410 tyu-40227:Downloads tyu$ stat -f%z target-after-gradle.apk 36736009 That would be 0.008% for mach and 0.007% for Gradle. Benchmarks are made upon https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=302eb1670544f4e8d0347de03526c4fd160480e1 [before] and after is the aforementioned try.

(In reply to Teng-pao Yu [:tyu] from comment #6) > I'd also like to invite :janH to give me some feedback regarding this plan. > Do you think it is acceptable to drag this problem a little bit longer to > not interfere the merge? No objections - I can work around this until then.

(In reply to Teng-pao Yu [:tyu] from comment #6) > > We should definitely enable jumbo mode in all Gradle debug configurations: > > local developers need it! > > > > We should probably enable jumbo mode in all Gradle configurations: it's the > > future. > > > > I'd prefer to enable jumbo mode in moz.build so that the Gradle > > configuration and the moz.build configuration stay as close together as > > possible. This will increase our APK size (a little) but should be safe in > > the meantime. > > > > :tyu, what do you think? > > I think it's a good idea to keep Gradle and mach build as consistent as > possible. > > I agree with the idea there is barely any risk with jumbo mode, if any. > However, I still feel like it is worth it to avoid surprising our Sheriffs > by landing this right before merge. This could be a good practice concerning > we're in the post-dawn world without Aurora. > > So I'm proposing yet another idea: maybe we could land them together but > after the merge day (June 12th). What's your opinion on this plan? That's sensible. Let's roll with this. > I'd also like to invite :janH to give me some feedback regarding this plan. > Do you think it is acceptable to drag this problem a little bit longer to > not interfere the merge? We can safely turn on jumboMode for all debug Gradle builds in the meantime. That should address (part of) :janh's issue.

Comment on attachment 8875219 [details] Bug 1370156 part 1 - Enable jumbo mode: in mach build; https://reviewboard.mozilla.org/r/146636/#review150998

Comment on attachment 8875220 [details] Bug 1370156 part 2 - Enable jumbo mode: in all Gradle builds; https://reviewboard.mozilla.org/r/146628/#review151000 This is fine for globally. I think we can land (at any time) a patch around https://dxr.mozilla.org/mozilla-central/source/mobile/android/app/build.gradle#73 that adds: ``` buildTypes { debug { dexOptions { jumboMode = true } } } ```

I've setup jumboMode for Gradle debug builds in 1367768, they will be landed soon and I've update the patches in this bug to first revert the debug-only jumbo mode config along with what was already reviewed, so depends on 1367768.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ba5082603190 part 1 - Enable jumbo mode: in mach build; r=nalexander https://hg.mozilla.org/integration/autoland/rev/3062e322a49f part 2 - Enable jumbo mode: in all Gradle builds; r=nalexander

https://hg.mozilla.org/mozilla-central/rev/ba5082603190 https://hg.mozilla.org/mozilla-central/rev/3062e322a49f