The memory used by the Privileged Content process increases by about 5MB every 5 minutes
Categories
(Firefox :: New Tab Page, defect, P3)
Tracking
()
Performance Impact | low |
People
(Reporter: florian, Unassigned)
References
(Blocks 1 open bug)
Details
I captured a profile of Firefox doing nothing for about an hour. I was surprised by the shape of the memory track for the Privileged Content process. I then captured another similar profile with the 'native allocations' feature enabled, which shows which memory is retained:
https://share.firefox.dev/3YjpISs
The memory is from these lines: https://searchfox.org/mozilla-central/rev/a0d4f8f112c5c792ae272bf6ce50763ddd23ffa2/browser/components/newtab/lib/cache-worker.js#179-182 About half of it is the .replace call, and about half is the JSON.stringify call.
I'm not sure what the cause is, it could either be some code of the worker somehow keeping a reference forever. Or it could be the garbage collector not being triggered in the DOM worker, for some reason.
Independently of the memory use concern, I wonder if this code really needs to run every 5 minutes when there's exactly nothing happening in Firefox.
Reporter | ||
Comment 1•3 years ago
|
||
(In reply to Florian Quèze [:florian] from comment #0)
I'm not sure what the cause is, it could either be some code of the worker somehow keeping a reference forever. Or it could be the garbage collector not being triggered in the DOM worker, for some reason.
Here's a profile where I clicked the minimize memory usage button of about:memory at the end: https://share.firefox.dev/3BznR29
Interestingly, out of the 24MB of additional memory use that added up during the duration of the profile, only 6MB were reclaimed... but the entire worker was terminated!
Comment 2•3 years ago
|
||
I wonder if this code really needs to run every 5 minutes when there's exactly nothing happening in Firefox.
It depends - during those 5 minutes, about:home might be getting updated with new Pocket stories / new content, in which case, this worker is used to refresh the cache.
out of the 24MB of additional memory use that added up during the duration of the profile, only 6MB were reclaimed... but the entire worker was terminated!
Yes, on the memory pressure observer notification, we terminate and throw away the worker: https://searchfox.org/mozilla-central/rev/a0d4f8f112c5c792ae272bf6ce50763ddd23ffa2/browser/components/newtab/AboutNewTabService.jsm#361-362
It is pretty concerning if the memory isn't being reclaimed even after the worker gets shut down...
Reporter | ||
Comment 3•3 years ago
|
||
(In reply to Mike Conley (:mconley) (:⚙️) from comment #2)
I wonder if this code really needs to run every 5 minutes when there's exactly nothing happening in Firefox.
It depends - during those 5 minutes, about:home might be getting updated with new Pocket stories / new content, in which case, this worker is used to refresh the cache.
Is there no way to verify if something has changed before refreshing the cache?
Comment 4•3 years ago
|
||
Maybe... So the way this mechanism works is that a task to refresh the cache is queued anytime the background preloaded about:newtab gets a message to update its state: https://searchfox.org/mozilla-central/rev/a0d4f8f112c5c792ae272bf6ce50763ddd23ffa2/browser/components/BrowserGlue.jsm#6326-6347
I suppose it's possible that a message is being sent that doesn't result in any updates to the page. If we were to fix this, we'd probably want to fix it there - to not actually send any messages to the preloaded about:newtab unless it's to update visual state.
Comment 5•3 years ago
|
||
The severity field is not set for this bug.
:daleharvey, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 6•3 years ago
|
||
5MB sounds like quite a lot but not sure about how we would usually set severity in the face of perf issues, Mike do you suggest any severity here?
Comment 7•3 years ago
|
||
This is a resource usage issue, where it seems that GC'ing isn't being triggered in the Worker, and so memory just climbs. We've got the memory pressure observer that kills the worker in the event of pressure, but the worker never GC'ing sounds like a bigger problem.
Hey smaug, are there known existing problems where Workers aren't GC'ing under certain circumstances?
Comment 8•3 years ago
|
||
There is, something related to use of timers, but I can't seem to find the bug now.
asuth might recall the title of the bug.
Comment 9•3 years ago
|
||
Bug 1216175 is the bug :smaug is thinking about (and which we should attempt to fix given that we believe the complicated platform issues that were preventing the landing are gone). I would presume this is actually a question of cycle collection not running, though, as I would expect GC to still run on demand as memory is allocated? (We can certainly land the fix and see what happens.)
(In reply to Andrew Sutherland [:asuth] (he/him) from comment #9)
I would presume this is actually a question of cycle collection not running, though, as I would expect GC to still run on demand as memory is allocated? (We can certainly land the fix and see what happens.)
On workers, the CC runs synchronously whenever the GC runs, IIRC. It is possible something is keeping things alive (in the CC or GC) when it shouldn't be.
Reporter | ||
Comment 11•3 years ago
|
||
(In reply to Mike Conley (:mconley) (:⚙️) from comment #7)
This is a resource usage issue, where it seems that GC'ing isn't being triggered in the Worker,
From the profile, it looks like GC is happening: https://share.firefox.dev/3ZkEEAi
The GC markers show heap size of 1.17MB. So somehow it seems the new 5MB of memory is not accounted as being part of the worker's heap.
are there known existing problems where Workers aren't GC'ing under certain circumstances?
I don't know if some array buffers are being transferred around between the main thread and the worker, but if that's the case, this bug could be another instance of bug 1407691.
Comment 12•3 years ago
|
||
That does make sense that the GC is happening given that 5 minutes should be a long enough gap between uses of the worker that the limitations of the GC timer state machine should not get tripped up into never running.
Unfortunately, most of what is going on seems like it's in the activity stream bundle. I wonder if there could be calls to console.log()
or something that might be entraining data? There is some use of sync XHR in the worker which always is an interesting thing, but that's a one-off thing whose results get cached and it's just providing strings, so nothing jumps out at me as an obvious problem in the easily accessible code.
Updated•3 years ago
|
Reporter | ||
Comment 13•3 years ago
|
||
I just noticed that the parent process memory also grows regularly at the times we update the new tab data, but slightly slower than the privileged content process: https://share.firefox.dev/3DQvRNp
Updated•2 years ago
|
Description
•