1528986 - Investigate OOM crashes in DevTools tests

Status: NEW

(Core :: JavaScript: GC, defect, P2)

Description

[Alexandre Poirot [:ochameau]]

Bug 1517210 introduced crashes in many tests. See bug 1517210 comment 8.
For some reason, the GC seem to run less or not run during these tests, leading to OOM crashes.

Bug 1517210 is about removing the freshCompartment flags from DevTools sandboxes.
DevTools codebase is using a custom loader which loads all JS files into Sandboxes (Cu.Sandbox + MozSubScriptLoader.loadSubScript).
Jan initially worked on a patch to make it so that all globals using the system principal would actually share the exact same compartment (instead of one for each global) (bug 1514210 and bug 1512029). But this ended up breaking chrome debugging, which has been addressed in bug 1515290. Once this last bug landed, we could remove this freshCompartment workaround. This is went we started hitting these OOM crashes.

The main effect of this patch is slightly smaller than what you think. DevTools was actually using only one Sandbox per loader, so that all modules of a given loader would be using the same compartment. But we are using multiple loaders. One for "always alive" modules (this is similar to JSMs) and then one per panel (console, inspector, ...). And we also have the panel themselves, which are also using a system principal but having yet another compartment.
So Jan's patch and the removal of freshCompartment isn't getting rid of cross compartment wrappers between all DevTools modules, but only between modules coming from different loaders, and between modules and panel's documents.
This is still a very significant improvement!!

Comment 1

[Alexandre Poirot [:ochameau]]

Bug 1529306 did fixed the JSTerm tests that used to fail in bug 1517210.

But that didn't address OOM in devtools/client/inspector/rules/test/browser_rules_user-agent-styles.js

And I'm currently getting report of random other OOM intermittent since this patch (bug 1517210) landed.
I'll try to ensure that the other failing tests get listed here if we end up adding Cu.forceShrinkingGC workarounds (I did that for browser_rules_user-agent-styles.js test).