The services created by GeckoView to run process such as media and content were not being created with the same nice level as the parent. Using the BIND_IMPORTANT flag during service creation ensures the service has the correct nice level.

Randall, should we uplift this BIND_IMPORTANT fix to GV 67 Beta for Fenix? Or is this just an FxR issue?

Also, should we use BIND_IMPORTANT for all bindService() calls? Your fix updated bindService() calls in RemoteManager.java and GeckoProcessManager.java, but not in SurfaceAllocator.java (from WebGL bug 1322650):

https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/SurfaceAllocator.java#36

It isn't clear that this change has any performance impact. snorp asked me to land it but I don't know that it is worth uplifting.

snorp, should we uplift this BIND_IMPORTANT fix to GV 67 Beta for Fenix? Or is this just an FxR issue?

Also, should we use BIND_IMPORTANT for all bindService() calls? This fix updated bindService() calls in RemoteManager.java and GeckoProcessManager.java, but not the bindService() call you added to SurfaceAllocator.java in WebGL bug 1322650:

https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/SurfaceAllocator.java#36

https://hg.mozilla.org/mozilla-central/rev/5a29abda4511

(In reply to Chris Peterson [:cpeterson] from comment #5)

snorp, should we uplift this BIND_IMPORTANT fix to GV 67 Beta for Fenix? Or is this just an FxR issue?

We should uplift.

Also, should we use BIND_IMPORTANT for all bindService() calls? This fix updated bindService() calls in RemoteManager.java and GeckoProcessManager.java, but not the bindService() call you added to SurfaceAllocator.java in WebGL bug 1322650:

https://searchfox.org/mozilla-central/rev/2c912888e3b7ae4baf161d98d7a01434f31830aa/mobile/android/geckoview/src/main/java/org/mozilla/gecko/gfx/SurfaceAllocator.java#36

Yeah, that's fine, the SurfaceAllocator is running the parent/app process which already has the correct priority set by the OS.

Noticed lots of big Raptor improvements on Android! \0/

== Change summary for alert #20105 (as of Tue, 26 Mar 2019 04:48:35 GMT) ==

Improvements:

41% raptor-tp6m-facebook-geckoview android-hw-p2-8-0-android-aarch64 opt 1,268.05 -> 747.70
39% raptor-tp6m-bing-geckoview android-hw-p2-8-0-android-aarch64 opt 199.52 -> 122.03
31% raptor-tp6m-amazon-geckoview android-hw-p2-8-0-android-aarch64 opt 690.90 -> 475.88
25% raptor-tp6m-stackoverflow-geckoview android-hw-p2-8-0-android-aarch64 opt 468.04 -> 350.40
23% raptor-tp6m-google-geckoview loadtime android-hw-p2-8-0-android-aarch64 opt 98.44 -> 75.33
19% raptor-tp6m-wikipedia-geckoview loadtime android-hw-p2-8-0-android-aarch64 opt 211.08 -> 170.33
19% raptor-tp6m-google-geckoview android-hw-p2-8-0-android-aarch64 opt 132.31 -> 107.09
19% raptor-tp6m-ebay-kleinanzeigen-geckoview android-hw-p2-8-0-android-aarch64 opt 870.51 -> 707.71
17% raptor-tp6m-bing-geckoview loadtime android-hw-p2-8-0-android-aarch64 opt 117.33 -> 97.79
17% raptor-tp6m-google-geckoview fcp android-hw-p2-8-0-android-aarch64 opt 150.52 -> 125.54
15% raptor-tp6m-wikipedia-geckoview android-hw-p2-8-0-android-aarch64 opt 267.18 -> 227.74
12% raptor-tp6m-ebay-kleinanzeigen-search-geckoview android-hw-p2-8-0-android-aarch64 opt 1,120.90 -> 983.67
12% raptor-tp6m-stackoverflow-geckoview loadtime android-hw-p2-8-0-android-aarch64 opt 415.12 -> 366.46
11% raptor-tp6m-bing-geckoview fcp android-hw-p2-8-0-android-aarch64 opt 170.04 -> 151.21
8% raptor-tp6m-youtube-watch-geckoview loadtime android-hw-p2-8-0-android-aarch64 opt 561.67 -> 516.88
6% raptor-tp6m-facebook-geckoview loadtime android-hw-p2-8-0-android-aarch64 opt 986.71 -> 928.75

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20105

(In reply to James Willcox (:snorp) (jwillcox@mozilla.com) (he/him) from comment #7)

(In reply to Chris Peterson [:cpeterson] from comment #5)

snorp, should we uplift this BIND_IMPORTANT fix to GV 67 Beta for Fenix? Or is this just an FxR issue?

We should uplift.

+1!

Comment on attachment 9052688 [details]
Bug 1537964 - Call bindService() with BIND_IMPORTANT

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: n/a
• User impact if declined: e10s content process and media process will run with a lower nice level.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This only changes the nice level of two services created on android.
• String changes made/needed: none

Randall, Chris, does this patch also affects Fennec or only Geckoview? If this is GV only, we don't need to uplift it given that Fenix will be based on GV68, right? Thanks

While fennec does not use e10s it does use the media process which is also affected by this patch.

(In reply to Pascal Chevrel:pascalc from comment #12)

Randall, Chris, does this patch also affects Fennec or only Geckoview? If this is GV only, we don't need to uplift it given that Fenix will be based on GV68, right?

Correct. Fenix MVP will use GV 68, so we don't need to uplift this fix for Fenix's benefit.

(In reply to Randall Barker [:rbarker] from comment #13)

While fennec does not use e10s it does use the media process which is also affected by this patch.

Since this fix might improve responsiveness of Fennec's media playback and the patch is a tiny, I would take the uplift. OTOH, Fennec has survived for a long time without this fix and we don't have STR to demonstrate any Fennec improvements.

Comment on attachment 9052688 [details]
Bug 1537964 - Call bindService() with BIND_IMPORTANT

Low risk patch for a perf gain, uplift approved for our next beta, thanks.

https://hg.mozilla.org/releases/mozilla-beta/rev/251103ed375f