1909511 - Re-enable the R8 fullMode optimization

Status: REOPENED

(Firefox for Android :: Performance, enhancement)

Description

[Ryan VanderMeulen [:RyanVM]]

Back when R8 fullMode was enabled by default in AGP, we had to disable it due to CI problems. I recently ran a fresh Try push and found that the Fenix and Focus Nightly robo startup tests still crash on startup with this optimization enabled, with similar-looking stacks.

Fenix

System.err: java.lang.RuntimeException: android.content.pm.PackageManager$NameNotFoundException: ComponentInfo{org.mozilla.fenix/kotlin.ExceptionsKt$tab0}
System.err: 	at org.mozilla.gecko.process.ServiceAllocator$InstanceInfo.<init>(SourceFile:84)
System.err: 	at org.mozilla.gecko.process.GeckoProcessManager$ChildConnection.<init>(SourceFile:1)
System.err: 	at org.mozilla.gecko.process.GeckoProcessManager$ConnectionManager.getNewContentConnection(SourceFile:7)
System.err: 	at org.mozilla.gecko.process.GeckoProcessManager$ConnectionManager.getConnectionForPreload(SourceFile:7)
System.err: 	at org.mozilla.gecko.process.GeckoProcessManager$$ExternalSyntheticLambda3.run(SourceFile:102)
System.err: 	at org.mozilla.gecko.util.XPCOMEventTarget$JNIRunnable.run(SourceFile:3)
System.err: Caused by: android.content.pm.PackageManager$NameNotFoundException: ComponentInfo{org.mozilla.fenix/kotlin.ExceptionsKt$tab0}
System.err: 	at android.app.ApplicationPackageManager.getServiceInfo(ApplicationPackageManager.java:501)
System.err: 	at org.mozilla.gecko.process.ServiceAllocator$InstanceInfo.<init>(SourceFile:62)
System.err: 	... 5 more

Focus

System.err: java.lang.RuntimeException: android.content.pm.PackageManager$NameNotFoundException: ComponentInfo{org.mozilla.focus.nightly/kotlin.text.CharsKt$tab0}
System.err: 	at org.mozilla.gecko.process.ServiceAllocator$InstanceInfo.<init>(SourceFile:85)
System.err: 	at org.mozilla.gecko.process.GeckoProcessManager$ChildConnection.<init>(SourceFile:1)
System.err: 	at org.mozilla.gecko.process.GeckoProcessManager$ConnectionManager.getNewContentConnection(SourceFile:7)
System.err: 	at org.mozilla.gecko.process.GeckoProcessManager$ConnectionManager.getConnectionForPreload(SourceFile:7)
System.err: 	at org.mozilla.gecko.process.GeckoProcessManager$$ExternalSyntheticLambda3.run(SourceFile:102)
System.err: 	at org.mozilla.gecko.util.XPCOMEventTarget$JNIRunnable.run(SourceFile:3)
System.err: Caused by: android.content.pm.PackageManager$NameNotFoundException: ComponentInfo{org.mozilla.focus.nightly/kotlin.text.CharsKt$tab0}
System.err: 	at android.app.ApplicationPackageManager.getServiceInfo(ApplicationPackageManager.java:501)
System.err: 	at org.mozilla.gecko.process.ServiceAllocator$InstanceInfo.<init>(SourceFile:62)
System.err: 	... 5 more

Reading the upstream docs on R8 fullMode, it sounds like we may just need to adjust our ProGuard rules a bit?
https://r8.googlesource.com/r8/+/refs/heads/master/compatibility-faq.md#r8-full-mode

Comment 1

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1909511 - Re-enable R8 fullMode for Fenix and Focus. r=#android-reviewers

Comment 2

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1909511 - Add an explicit AndroidX Concurrency dependency for Focus UI tests. r=#android-reviewers

Without this, Focus Nightly androidTest builds hit the below error:
ERROR: R8: Missing class androidx.concurrent.futures.SuspendToFutureAdapter (referenced from: com.google.common.util.concurrent.ListenableFuture androidx.test.core.view.ViewCapture.captureToBitmapAsync(android.view.View, android.graphics.Rect) and 1 other context)

Don't ask me why Fenix doesn't also hit this. Something, something, ProGuard maybe?

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1909511 - Disable optimizations for Fenix profile generation builds. r=#android-reviewers

This is similar to what we already do for UI test builds.
R8 apparently likes to over-optimize things to the point where it ends up breaking the profiling run:
org.mozilla.fenix.benchmark.baselineprofile.NormalBrowsingBaselineProfileGenerator#generateBaselineProfile[composableToolbar=false] | java.lang.IllegalStateException: Unable to confirm activity launch completion [] Please report a bug with the output of adb shell dumpsys gfxinfo org.mozilla.fenix framestats

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Attachment: Bug 1909511 - Update ProGuard rules. r=#android-reviewers

Fixes for various crashes encountered in CI with release builds caused by R8 over-optimizing things.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

One last Try push for sanity, but I'm very confident it'll be green.
https://treeherder.mozilla.org/jobs?repo=try&revision=d3b2769600af00b0b1f0942476f3be83769eaa59

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Looks like it's about a 4% reduction in nightly APK and AAB size. Not sure what that'll translate to for Play Store downloads, but seems like a pretty nice win anyway!

Comment 7

[Phabricator Automation]

Comment on attachment 9511884 [details]
Bug 1909511 - Add an explicit AndroidX Concurrency dependency for Focus UI tests. r=#android-reviewers

Revision D264010 was moved to bug 1987713. Setting attachment 9511884 [details] to obsolete.

Comment 8

[Phabricator Automation]

Comment on attachment 9511885 [details]
Bug 1909511 - Disable optimizations for Fenix profile generation builds. r=#android-reviewers

Revision D264011 was moved to bug 1987713. Setting attachment 9511885 [details] to obsolete.

Comment 9

[Phabricator Automation]

Comment on attachment 9511886 [details]
Bug 1909511 - Update ProGuard rules. r=#android-reviewers

Revision D264012 was moved to bug 1987713. Setting attachment 9511886 [details] to obsolete.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/277a161da99f
https://hg.mozilla.org/integration/autoland/rev/1383ca2b8c0e
Re-enable R8 fullMode for Fenix and Focus. r=android-reviewers,sfamisa

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/1383ca2b8c0e

Comment 12

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #10)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/277a161da99f
https://hg.mozilla.org/integration/autoland/rev/1383ca2b8c0e
Re-enable R8 fullMode for Fenix and Focus. r=android-reviewers,sfamisa

Perfherder has detected a mozperftest performance change from push 1383ca2b8c0e1d1c604c27037220911244b82e29.

If you have any questions, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
17%	background-resource pss-crashhelper-start	android-hw-a55-14-0-aarch64-shippable		24,679.50 -> 20,444.58
17%	foreground-resource pss-crashhelper-10%	android-hw-a55-14-0-aarch64-shippable		24,823.92 -> 20,565.25
17%	foreground-resource pss-crashhelper-start	android-hw-a55-14-0-aarch64-shippable		24,718.75 -> 20,518.83
17%	foreground-resource pss-crashhelper-50%	android-hw-a55-14-0-aarch64-shippable		24,874.50 -> 20,668.58
17%	background-resource pss-crashhelper-10%	android-hw-a55-14-0-aarch64-shippable		24,689.25 -> 20,513.58
...	...	...	...	...
8%	foreground-resource pss-gpu-50%	android-hw-a55-14-0-aarch64-shippable		52,079.25 -> 47,726.33

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 47196

The following documentation link provides more information about this command.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a8a7aa881515
https://hg.mozilla.org/integration/autoland/rev/94ab490a658b
Revert "Bug 1909511 - Re-enable R8 fullMode for Fenix and Focus. r=android-reviewers,sfamisa" as requested by RyanVm for causing bug 1995743.

Comment 14

[Serban Stanca [:SerbanS]]

Reverted this as requested by RyanVm for causing bug 1995743.

• Revert link

Comment 15

[Serban Stanca [:SerbanS]]

Backout merged in central: https://hg.mozilla.org/mozilla-central/rev/94ab490a658be5491b2d279f95087cfe2a615e25.

Comment 16

[Alex Finder]

(In reply to Serban Stanca [:SerbanS] from comment #14)

Reverted this as requested by RyanVm for causing bug 1995743.

• Revert link

Perfherder has detected a mozperftest performance change from push 2bc48cc453df653afe1a8666e091a8eedfe949b7.

This comment is for informational purposes only; no further action from the author is required.

If you have any questions or need any help with the investigation, please reach out to a performance sheriff. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
23%	background-resource pss-crashhelper-50%	android-hw-a55-14-0-aarch64-shippable		20,467.33 -> 25,252.17
23%	background-resource pss-crashhelper-10%	android-hw-a55-14-0-aarch64-shippable		20,416.50 -> 25,175.58
23%	background-resource pss-crashhelper-end	android-hw-a55-14-0-aarch64-shippable		20,488.17 -> 25,260.83
23%	background-resource pss-crashhelper-start	android-hw-a55-14-0-aarch64-shippable		20,350.58 -> 25,042.92
21%	foreground-resource pss-crashhelper-start	android-hw-a55-14-0-aarch64-shippable		20,665.58 -> 25,051.00
21%	foreground-resource pss-crashhelper-10%	android-hw-a55-14-0-aarch64-shippable		20,770.17 -> 25,137.17
21%	foreground-resource pss-crashhelper-50%	android-hw-a55-14-0-aarch64-shippable		20,829.50 -> 25,199.42
21%	foreground-resource pss-crashhelper-end	android-hw-a55-14-0-aarch64-shippable		20,932.83 -> 25,270.75
20%	newssite-applink-startup org.mozilla.fenix:crashhelper-cpu-time	android-hw-a55-14-0-aarch64-shippable		106.83 -> 128.67
10%	foreground-resource pss-gpu-50%	android-hw-a55-14-0-aarch64-shippable		47,175.08 -> 51,871.58
10%	foreground-resource pss-gpu-start	android-hw-a55-14-0-aarch64-shippable		47,062.08 -> 51,731.75
10%	foreground-resource pss-gpu-10%	android-hw-a55-14-0-aarch64-shippable		47,169.25 -> 51,809.17
9%	background-resource pss-gpu-10%	android-hw-a55-14-0-aarch64-shippable		47,240.12 -> 51,467.58
9%	background-resource pss-gpu-start	android-hw-a55-14-0-aarch64-shippable		47,172.25 -> 51,338.17

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a performance sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 47330

The following documentation link provides more information about this command.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

FWIW, AGP 9 has a new warning that fullMode will be the default in AGP 10 and the option to disable it will be removed.