Decide what to do with the javascript.options.gc_on_memory_pressure pref on Android
Categories
(Core :: JavaScript: GC, enhancement, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox61 | --- | affected |
People
(Reporter: n.nethercote, Assigned: kaya)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [fxdroid] [foundation] [group1] )
Attachments
(1 file)
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.
Reporter | ||
Updated•7 years ago
|
Comment 1•7 years ago
|
||
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?
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
Updated•6 years ago
|
Updated•2 years ago
|
Comment 4•3 months ago
|
||
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?
Assignee | ||
Comment 5•3 months ago
•
|
||
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•3 months ago
|
||
Oh my, no wonder bug 1892278 isn't useful on Android.
Yes, we should enable the pref on Android
Comment 7•3 months ago
|
||
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•3 months ago
|
||
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...)
Assignee | ||
Comment 9•3 months ago
•
|
||
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.
Assignee | ||
Comment 10•3 months ago
|
||
Updated•3 months ago
|
Assignee | ||
Updated•2 months ago
|
Updated•2 months ago
|
Assignee | ||
Comment 11•18 hours ago
|
||
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
Updated•18 hours ago
|
Description
•