1450787 - Decide what to do with the javascript.options.gc_on_memory_pressure pref on Android

Status: ASSIGNED

(Core :: JavaScript: GC, enhancement, P1)

Description

[Nicholas Nethercote [inactive]]

javascript.options.gc_on_memory_pressure is currently true on desktop and false on Android. It possibly shouldn't be false on Android. Bug 1449833 has some discussion about this.

Comment 1

[Jason Orendorff [:jorendorff]]

In bug 1449833 comment 4, jonco wrote:
> Oh wow, I wasn't aware of this.  If possible we should make this
> work the same as the desktop browser, but someone who works on the
> mobile browser should check this is OK.

I don't know whom to needinfo on this. dbolter, who knows about memory management on Android?

Comment 2

[David Bolter [:davidb] (NeedInfo me for attention)]

James any ideas?

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Fennec does do gc on memory-pressure currently, it just doesn't use a pref. https://searchfox.org/mozilla-central/source/mobile/android/chrome/content/MemoryObserver.js#15

Comment 4

[Gregory Pappas [:gregp]]

I came across this bug while auditing prefs for bug 1773039,

It looks like comment 3 is not true in GeckoView. I'm not able to locate any custom memory-pressure handling that's similar to Fennec's MemoryObserver.js
https://searchfox.org/mozilla-central/rev/e999ae1989ac4156e3f0ae7faccb8d0c57a7ef28/mobile/android/chrome/content/MemoryObserver.js

Should this pref be re-enabled on Android?

Comment 5

[Kayacan Kaya [:kaya]]

Thanks for pointing this out :gregp.

TL;DR
I believe we must enable that pref as we are already notifying observers of memory-pressure, and it looks like it is a no-op (except on gfx layer) unless we enable it.

--
As far as I can tell, Android is trying to notify the observers of memory-pressure topic under these conditions:
1- When Android OS sends a memory pressure callback
2- When process is sent to the background - this was a recent pref-ed off change to test pressure levels based on another pref, ni'ing Olli as I believe we need javascript.options.gc_on_memory_pressure enabled to see this change's impact. Please correct me if I'm wrong.
3- When ContentParent receives it and ContentChild forwards the message to the code running in the content process

As in the scope of this ticket, due to the pref being disabled on Android, this if check will never be satisfied and all these three cases will have no impact.

There's also some handling going on in the Graphics end which I believe should already be working on Android. (cc'ing Jaime in case I'm wrong.)

I could not spot any reference to zombifying a tab as Fennec was doing, but I believe what we are doing when we receive a memory pressure callback from the Android OS on Fenix is doing smth similar: it suspends tabs which unlinks the engine session from the tabs and then closes those sessions that at the end releases the memory held on Java and native layers. "Unzombifying" corresponds to tab reloading and restoring the engine session.

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

Oh my, no wonder bug 1892278 isn't useful on Android.

Yes, we should enable the pref on Android

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

I think it would be good time now to enable the pref on Android. We'd get experience on how it behavior in Nightly-Fenix.

Comment 8

[Jamie Nicol [:jnicol]]

Agreed we should try enabling this.

From a gfx perspective it seems like we should be responding to memory pressure correctly. Additionally to the mechanisms you've noted above (OS memory pressure callback, and process background), we also emit a memory pressure when a GeckoSession is set inactive. (There could be bugs, of course...)

Comment 9

[Kayacan Kaya [:kaya]]

Thanks Jamie, I missed that compositor notification!

I'll now put up a patch and we will be testing/profiling it with Olli; and once we have no concerns about this change, we will land it and bake it on Nightly.

Comment 10

[Kayacan Kaya [:kaya]]

Attachment: Bug 1450787 - Enable javascript.options.gc_on_memory_pressure pref on Android Nightly.

Comment 11

[Kayacan Kaya [:kaya]]

Updates:

Enabling the pref causes a Jank in all the processes due to the non-incremental GC operations. This happens when the processes are getting into the cached/backgrounded state. We will try enabling the pref on Nightly and observe some perf metrics to follow up on the performance impacts of this change, and then decide to ride the trains or not.

Pref disabled profile: https://share.firefox.dev/4bRY3hh
Pref enabled profile: https://share.firefox.dev/3QXZlPR

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb3840ded8be
Enable javascript.options.gc_on_memory_pressure pref on Android Nightly. r=smaug

Comment 13

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/bb3840ded8be

Comment 14

[Gregory Pappas [:gregp]]

Is there a bug for enabling this pref in release?

Comment 15

[Kayacan Kaya [:kaya]]

Sorry, forgot to leave this ticket open. I am reopening it to track the impact of this change on Nightly. Once we make the final decision for this pref, we can then decide to either handle the other channels in this ticket or create another ticket to enable it on other channels as well.

Comment 16

[Kayacan Kaya [:kaya]]

Attachment: Bug 1450787 - Disable javascript.options.gc_on_memory_pressure pref on Android.

This pref was enabled on Android Nightly and an increase in the background CPU consumption was observed. This change backs it out to investigate the performance impact more.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68681c287b64
Disable javascript.options.gc_on_memory_pressure pref on Android. r=smaug

Comment 18

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/68681c287b64