Decide what to do with the javascript.options.gc_on_memory_pressure pref on Android
Categories
(Core :: JavaScript: GC, enhancement, P1)
Tracking
()
People
(Reporter: n.nethercote, Assigned: kaya)
References
(Blocks 2 open bugs)
Details
(Keywords: leave-open, Whiteboard: [fxdroid] [foundation] [group1] )
Attachments
(2 files)
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•4 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•4 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•4 months ago
|
||
Oh my, no wonder bug 1892278 isn't useful on Android.
Yes, we should enable the pref on Android
Comment 7•4 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•4 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•4 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•4 months ago
|
||
Updated•4 months ago
|
Assignee | ||
Updated•3 months ago
|
Updated•3 months ago
|
Assignee | ||
Comment 11•24 days 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•24 days ago
|
Comment 12•23 days ago
|
||
Pushed by kkaya@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bb3840ded8be Enable javascript.options.gc_on_memory_pressure pref on Android Nightly. r=smaug
Comment 13•22 days ago
|
||
bugherder |
Comment 14•22 days ago
|
||
Is there a bug for enabling this pref in release?
Assignee | ||
Comment 15•22 days ago
|
||
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.
Updated•22 days ago
|
Assignee | ||
Comment 16•7 days ago
|
||
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•7 days ago
|
||
Pushed by kkaya@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/68681c287b64 Disable javascript.options.gc_on_memory_pressure pref on Android. r=smaug
Comment 18•6 days ago
|
||
bugherder |
Updated•6 days ago
|
Description
•