Closed Bug 1714072 Opened 3 years ago Closed 3 years ago

[meta] Massive memory use + wasm compilation time with console open

Categories

(Core :: JavaScript: WebAssembly, defect, P3)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: lth, Unassigned)

References

(Blocks 3 open bugs)

Details

(Keywords: meta)

FF91 Nightly local release build, Fedora 33, x64.

Set up the TC from bug 1656549. Open a new tab (with no console open), reload a few times, observe that this is quick and smooth. In a process viewer, the memory for the content process will increase for a time but then fall back down to about 200MB.

Open a console in that tab, and reload. Recompilation takes a very long time (10-15s), memory use grows to ~1.5GB, and there's significant CPU use (40% in my case) for a long time. The increase in space and time is due to bug 1714086; ignore that problem, it's just helpful here anyway. Reload again, and observe that memory use continues to grow to ~3GB (2.8 here). Wait. Observe that memory use does not fall. Now measure with about:memory, observe that wasm accounts for more than 2GB: 1.4GB malloc/misc and 600MB code.

Do not close the console, but click on "minimize memory usage" in about:memory. Observe that no wasm memory is reclaimed in the content process.

Close the console and click "minimize memory usage". Observe that all the wasm memory is reclaimed in the content process.

(Actually just closing the console and reloading the page - it'll be quick - is enough to trigger a GC and drop memory down to where it should be.)

A non-e10s run has exactly the same behavior.

(Moved to bug 1714086.)

(Moved to bug 1714086.)

Depends on: 1714086

(Moved.)

I suspect that the leak is related to not properly clearing the console on reload, but it's not straightforward. The console calls ResetWindow() to do the clearing, which ends up in ClearMessagesForWindowID(). That function matches some messages but not others, which is weird. But even replacing ResetWindow() with a full Reset() does not mitigate the problem, so something more than that is going on.

More likely than not this is an instance of bug 1476869.

An additional question is whether wasm debug code & metadata should be emitted just because the console is open (the debugger is different). I suspect probably not.

Enabling debug support for wasm requires a reload currently. When the console is open, there is a fair change that the user wants to use the debugger soon, so not enabling debug support would be kind of annoying.

(In reply to bjorn3 from comment #7)

Enabling debug support for wasm requires a reload currently. When the console is open, there is a fair change that the user wants to use the debugger soon, so not enabling debug support would be kind of annoying.

That's a point we can take into consideration. Probably it comes down to use cases and what the devtools people have thought about this. Personally I think it's more problematic to be presented with code that runs 4-10x slower and takes a long time and a lot of memory to load just because I wanted to see the console.log messages, and that seems to be the reality with the current structure. As it is, I can't instrument my code and spew performance data to the console (say) without being careful to keep the console closed when I load the page, because if I'm careless I'll get slow debug-enabled baseline code.

Maybe it could be a config option what needs to be open for wasm to be debuggable (debugger, devtools, nothing)? Or a checkbox in the debugger if wasm should be debuggable for the current tab while the devtools are open.

Depends on: 1654590

(In reply to Lars T Hansen [:lth] from comment #6)

An additional question is whether wasm debug code & metadata should be emitted just because the console is open (the debugger is different). I suspect probably not.

Indeed this seems to be bug 1654590, which attempts to get rid of an unconditional 'observeAsmJS:true' attribute that was introduced earlier, which, if it is present, causes the wasm module to be compiled in debug mode. Commenting out that attribute makes the TC load very quickly even with the console open.

Edit: Actually bug 1719615.

Depends on: 1719615

This bug is fixed for code that does not use reference types, ie, all current large C/C++/Rust code bases. It is not fixed for code that actively uses reference types; we need to fix bug 1719615 for that.

No longer depends on: 1654590
Depends on: 1715456
Keywords: meta
Summary: Massive memory use + wasm compilation time with console open → [meta] Massive memory use + wasm compilation time with console open
Assignee: lhansen → nobody
Status: ASSIGNED → NEW
Priority: P2 → P3
Depends on: 1756030
Blocks: 1756030
No longer depends on: 1756030
No longer depends on: 1715456
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.