Media Panel open on twitch.tv results in a rapid memory leak
Categories
(Core :: Audio/Video: Playback, defect, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | wontfix |
firefox63 | --- | wontfix |
firefox64 | --- | wontfix |
firefox65 | --- | wontfix |
firefox66 | --- | fix-optional |
People
(Reporter: ke5trel, Unassigned)
References
(Blocks 1 open bug)
Details
(Keywords: memory-leak, regression, Whiteboard: [MemShrink:P3])
Attachments
(2 files)
Updated•6 years ago
|
Comment 1•6 years ago
|
||
Comment 2•6 years ago
|
||
Updated•6 years ago
|
Updated•6 years ago
|
(Sorry for the delay, I left this in my backlog, as it didn't seem urgent because it's only triggered by the optional DevTools Media panel extension.)
The media panel starts DecoderDoctorLogger, which collects lots of media data. In this case, it seems Twitch makes us produce a massive amout of logging, which takes more time to process than to generate, and then still takes a lot of memory after that.
There are some safety valves ( https://searchfox.org/mozilla-central/rev/5053031ba7621fa8f63f42de4c204ab3561e4e59/dom/media/doctor/DDMediaLogs.cpp#519-526 ), but they're in minutes, so not adequate for this case.
Running Firefox with MOZ_LOG=DDLogger:3 shows some "yet-unprocessed message buffers" climbing quickly (when there should normally hover in low units).
It seems the media panel doesn't even show these log messages anyway, it's quite a waste!
So as a first step, maybe the extension should not enable logging, i.e., don't call HTMLMediaElement.mozEnableDebugLog()
unless really needed.
Michael, what do you think? Could you delay mozEnableDebugLog()
until the user really wants to see logs?
On top of that, a better solution would be to discard unprocessed logs when they start taking too much memory, but this will be tricky because only DDMediaLogs::ProcessBuffer()
is supposed to access it. adding a reader/discarder would need extra concurrency management.
Maybe as a first step ProcessBuffer()
could notice the memory issue and not actually process each message (which takes time and memory), so we'll recover memory much more quickly, at the cost of incomplete logs -- but then this means the DevTools Media panel may not be that useful on Twitch anyway. 🤔
Another solution would be to make the safety valves more dynamic, discarding more of the processed logs when the pressure mounts.
And an extra thought: I don't think processed messages are immediately cleared, which means those that contain an nsCString or MediaResult may be also holding onto heap memory. So ideally, MultiWriterQueue::PopAll()
should std::move
the message and ProcessBuffer()
should clear it.
I could look into the DDLogger code changes, but officially I've been out of Media Playback for more than a year! I'd be happy to guide someone else with this, otherwise I'll try to work on it when I've got some free time...
Comment 5•6 years ago
|
||
(In reply to Gerald Squelart [:gerald] from comment #4)
It seems the media panel doesn't even show these log messages anyway, it's quite a waste!
So as a first step, maybe the extension should not enable logging, i.e., don't callHTMLMediaElement.mozEnableDebugLog()
unless really needed.
Michael, what do you think? Could you delaymozEnableDebugLog()
until the user really wants to see logs?
I can take a look at delaying or controlling when/if we ask for logs. I think it be some time before it bubbles up to the top of my stack though.
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 6•6 years ago
|
||
Happy to take a patch in nightly 67, or potentially, in beta 66 for this.
I'm marking it fix-optional to remove it from weekly regression triage, since it has a priority assigned.
Updated•6 years ago
|
Updated•2 years ago
|
Description
•