2040924 - Force a shrinking GC between mochitests to discard JIT inline-cache stubs holding shapes from dead realms

Status: RESOLVED FIXED

(Testing :: Mochitest, enhancement)

Description

[Florian Quèze [:florian]]

SpiderMonkey installs CacheIR inline-cache stubs on hot, shared scripts (e.g. PanelMultiView.getViewNode). Those stubs hold cacheir-object strong refs to shapes from the realms they observed. Each shape's baseshape_global keeps the realm's global alive, which in turn keeps the entire chrome window alive once the docshell has been torn down.

In normal Firefox use, a shrinking GC is scheduled automatically after the user has been inactive for some time. GCRuntime::shouldPreserveJITCode returns false for shrinking GCs, so the JIT/IC code (and the shape references it carries) gets discarded. The dead window's cycle then becomes collectible by the CC.

When mochitests run back-to-back there is no idle period, no shrinking-trigger memory pressure, and shouldPreserveJITCode returns true for every regular major GC the harness drives. The IC stubs on shared chrome scripts accumulate shape references to every realm they ever saw, pinning every closed chrome window for the lifetime of the test run. In CI this manifests as ever-growing CC times and eventually OOM crashes (most visibly on the browser/components/aiwindow/ui/test/browser/ suite, but the underlying retention is generic).

Calling Cu.forceShrinkingGC() between browser-chrome tests in Tester.nextTest mimics the inactivity-driven shrinking GC that real users get. A try push confirmed this prevents the per-test window accumulation that drove the symptom.

We might want to file a follow-up platform bugs to investigate if a way to clear these caches after a window is closed would be possible. I did a few attempts in that direction without getting any great success (I got improvements when running locally that didn't result in green try pushes).

Comment 1

[Florian Quèze [:florian]]

Attachment: Bug 2040924 - Force a shrinking GC and CC between mochitests to clear chrome windows held by JIT inline-cache stubs, r=Gijs.

In normal Firefox use, a shrinking GC is scheduled automatically after the
user has been inactive for some time, which eventually causes the JS engine
to discard JIT code (and the cacheir-object strong refs to shapes from
already-destroyed realms that it carries). When mochitests run back-to-back
there is no such idle period, so JIT/IC stubs on hot shared chrome scripts
(e.g. PanelMultiView.getViewNode) keep dead chrome windows alive across
tests, eventually driving up CC times and consuming memory in CI. Force a
shrinking GC followed by a CC between tests in Tester.nextTest to mimic the
inactivity-driven cleanup real users get: the shrinking GC unanchors the
dead realms, and the CC then collects the chrome-window cycles those shapes
were holding onto.

Comment 2

[Jon Coppeard (:jonco)]

(In reply to Florian Quèze [:florian] from comment #0)

SpiderMonkey installs CacheIR inline-cache stubs on hot, shared scripts (e.g. PanelMultiView.getViewNode). Those stubs hold cacheir-object strong refs to shapes from the realms they observed. Each shape's baseshape_global keeps the realm's global alive, which in turn keeps the entire chrome window alive once the docshell has been torn down.

Do you have more specific information on which cache IR is doing this?

The right fix for this problem is for the JIT to hold less stuff alive, not trigger GCs all the time. There's no guarantee this will not affect users as the inactivity timer is relatively long.

This approach is obviously fine if this is causing an acute problem with out testing.

Bug 1894937 is related but about Ion code but this sounds like it's about baseline.

Comment 3

[Florian Quèze [:florian]]

(In reply to Jon Coppeard (:jonco) from comment #2)

(In reply to Florian Quèze [:florian] from comment #0)

SpiderMonkey installs CacheIR inline-cache stubs on hot, shared scripts (e.g. PanelMultiView.getViewNode). Those stubs hold cacheir-object strong refs to shapes from the realms they observed. Each shape's baseshape_global keeps the realm's global alive, which in turn keeps the entire chrome window alive once the docshell has been torn down.

Do you have more specific information on which cache IR is doing this?

I had steps to reproduce locally, then had WIP patches that fixed it locally, and things still failed on try with these patches.
To reproduce locally just applying https://phabricator.services.mozilla.com/D301574 and running ./mach test browser/components/aiwindow/ui/test/browser/browser.toml should be enough. If you apply D301575 on top, you’ll see fewer failures, but they will have reference traces.

I think the stack of patches that worked best locally was https://treeherder.mozilla.org/jobs?repo=try&revision=1bd77762b5f21ec77f906173a2cb1fca7aefb9e2 but it’s quite orange on try, including with jobs that timeout after 90min.

Bug 1894937 is related but about Ion code but this sounds like it's about baseline.

Most of my WIPs tried to collect more stuff during GCs for windows that were no longer alive, I haven't tried to make the references weak in the first place, maybe that would have worked better.

Comment 4

[Jon Coppeard (:jonco)]

I guess what I'm asking is, how do you know it's the cacheir-object edges keeping things alive? Did you produce some log file with this information in it? If so can you share it? It could be very helpful.

What's supposed to happen is that these references are weak and don't hold things alive themselves. We may have missed some cases, or something else entirely could be going wrong.

Comment 5

[Florian Quèze [:florian]]

(In reply to Jon Coppeard (:jonco) from comment #4)

I guess what I'm asking is, how do you know it's the cacheir-object edges keeping things alive? Did you produce some log file with this information in it? If so can you share it? It could be very helpful.

What's supposed to happen is that these references are weak and don't hold things alive themselves. We may have missed some cases, or something else entirely could be going wrong.

https://firefoxci.taskcluster-artifacts.net/JzqhGxu2SZyDNS4i1zPzwQ/0/public/logs/live_backing.log contains many mentions of cacheir-weak-script. I let Claude to a lot of the analysis so it's possible some things were misinterpreted. I can try tomorrow to search the logs of my Claude sessions to see when cacheir-object rather than cacheir-weak-script was mentioned.

The thing I know for sure is that if I push the patch attached to this bug to try, the memory use issue disappears (the comment at https://phabricator.services.mozilla.com/D301565#10463811 was written by hand, not by Claude). The patches I used for analysis are attached as WIP in bug 2040929 as I haven't figured out a way to make them pass yet.

Comment 6

[Jan de Mooij [:jandem]]

Does this have any effect on how long it takes to run the test suite? If we can fix this inside SM that's probably preferable; let me know if I can help with that.

Comment 7

[Jon Coppeard (:jonco)]

(In reply to Florian Quèze [:florian] from comment #5)
First of all I don't want to block landing this bug if it fixes long standing test issues like bug 1984528. However I also don't want to hide real problems that may also affect users. This may be the same reason we have such high memory usage on SP3 which is also a problem.

To get into the details:

• reporting paths in bug 2040929 is a great idea and it's really helpful to see this stuff being reported to the log files
• cacheir weak edges are unlikely to be the culprit because the weak part is about solving this exact problem (bug 1837620)
• other cacheir or JIT edges may be involved here although I don't see that in the logs
• _findPathToRoot only reports the first path it finds with BFS; there may be several
• there may be multiple bugs causing this symptom; only fixing all of them will fully resolve it

One thing I see is that all these paths go through scripts. Maybe that's not too surprising because I'm sure we have a lot of scripts but it makes me wonder if something like bug 894971 is at play here where closures keep more stuff alive than necessary.

Some things that could help diagnose this:

• reporting more paths in leak checker not just the first
• reporting the realm of path elements so we can see which the cross-realm edge is

Comment 8

[Florian Quèze [:florian]]

(In reply to Jan de Mooij [:jandem] from comment #6)

Does this have any effect on how long it takes to run the test suite?

Yes. Resource usage profiles of the Linux Debug bc chunk running browser/components/aiwindow/ui/test/browser/browser.toml:

• on central: https://share.firefox.dev/4wLtHso 22m26s
• on try with the patch: https://share.firefox.dev/4dEPo4A 14m2s
  Without the patch we spend a lot of time in increasingly long cycle collections.

If we can fix this inside SM that's probably preferable; let me know if I can help with that.

I would also prefer that, but it is clearly outside of my expertise area.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/032c6d128cd7
https://hg.mozilla.org/integration/autoland/rev/6560c893adc0
Force a shrinking GC and CC between mochitests to clear chrome windows held by JIT inline-cache stubs, r=Gijs,mccr8.

Comment 10

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/6560c893adc0

Comment 11

[Florin Bilt]

(In reply to Iulian Moraru from comment #10)

https://hg.mozilla.org/mozilla-central/rev/6560c893adc0

Perfherder has detected a devtools performance change from push 6560c893adc00fdcf84acb731b70de370da81850.

No action is required from the author; this comment is provided for informational purposes only.

Improvement	Test	Platform	Options	Absolute values [old vs new]
64%	reload-netmonitor:parent-process objects-with-stacks	linux2404-64		14.00 -> 5.00

Need Help or Information?

If you have any questions, please reach out to [e-mail]. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

Comment 12

[Alex Finder]

(In reply to Pulsebot from comment #9)

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/032c6d128cd7
https://hg.mozilla.org/integration/autoland/rev/6560c893adc0
Force a shrinking GC and CC between mochitests to clear chrome windows held
by JIT inline-cache stubs, r=Gijs,mccr8.

Perfherder has detected a mozperftest performance change from push 6560c893adc00fdcf84acb731b70de370da81850.

No action is required from the author; this comment is provided for informational purposes only.

Improvements	Test	Platform	Options	Absolute values [old vs new]
5%	browser_translations_perf_tiny.js post-gc-parent-process-memory-usage	macosx1015-64-shippable-qr		230.97 MiB -> 219.05 MiB
5%	browser_translations_perf_tiny.js stabilized-parent-process-memory-usage	macosx1015-64-shippable-qr		231.28 MiB -> 219.43 MiB
5%	browser_translations_perf_basememory.js peak-parent-process-memory-usage	linux1804-64-shippable		202.93 MiB -> 192.78 MiB
5%	browser_translations_perf_tiny.js peak-parent-process-memory-usage	linux1804-64-shippable		197.70 MiB -> 187.89 MiB
5%	browser_translations_perf_basememory.js post-gc-parent-process-memory-usage	linux1804-64-shippable		196.39 MiB -> 186.90 MiB
...	...	...	...	...
4%	browser_translations_perf_base.js post-gc-parent-process-memory-usage	macosx1015-64-shippable-qr		232.04 MiB -> 223.66 MiB

Need Help or Information?

If you have any questions, please reach out to [e-mail]. Alternatively, you can find help on Slack by joining #perf-help, and on Matrix you can find help by joining #perftest.

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.