1915328 - Temporary 2.8GB memory spike on a slightly modified Codepen demo

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Mayank Bansal]

Attachment: index.html

Open the attched slightly modified testcase (Original at https://codepen.io/yuanchuan/pen/PorpjOQ)

AR: Memory spikes to 2.8GB during loading. However the additional memory is release after a few seconds or if you put the testcase in a background tab. Found out by watching the Windows Task manager for memory use.
ER: Prior to the regression, the memory use never went over 300MB-600MB

Bisection:
Bug 1907891 part 2 - Create shared string buffer for long strings allocated by js::StringBuffer. r=sfink,emilio
Differential Revision: https://phabricator.services.mozilla.com/D216694

(Some regressions may be inevitable based on https://bugzilla.mozilla.org/show_bug.cgi?id=1910420#c6)

Comment 1

[Mayank Bansal]

Profile1: https://share.firefox.dev/4dwulzP (the blip at the 9.5s mark is when the 2.5GB memory was released)
Profile2 with memory tracking: https://share.firefox.dev/4cEhGcZ
Profile3 with a "40x40" grid size: https://share.firefox.dev/4cImu0M . My machine probably hit some RAM limit in this case.

---

You can also look at this demo to see a testcase which has become slower post-regression. Every 2 seconds the demo refreshes. During refresh the memory use is high, and it takes longer to refresh compared to pre-regression.
Profile4: https://share.firefox.dev/4fUJfkW

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1907891

:jandem, since you are the author of the regressor, bug 1907891, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Mayank Bansal]

On a slight tangential note, for similar demo the profiler shows that the memcopy and vector stuff as "ion" time. (Shouldn the category be consistent with the profiles in comment #2 and comment #1?)
https://codepen.io/yuanchuan/pen/QWQzbzb : https://share.firefox.dev/3X6rsyu / https://share.firefox.dev/3Z4NkwW

Comment 4

[Mayank Bansal]

deleted

Comment 5

[Jan de Mooij [:jandem]]

Great find. Thanks Mayank.

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1915328 - Update mallocedBufferBytes for string buffers used by nursery strings. r?jonco!

This lets us trigger a minor GC if we're allocating many long strings. This avoids
a memory spike on the test case reported in the bug.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1907891

Comment 8

[Jan de Mooij [:jandem]]

Mayank, just a heads-up that this might revert some of the changes on JetStream because this makes us match the previous behavior better.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/39e5feb1b0b8
Update mallocedBufferBytes for string buffers used by nursery strings. r=jonco

Comment 10

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/39e5feb1b0b8

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jandem, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox131 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

[Mayank Bansal]

This is fixed for me in the latest Nightly. The memory never went over 200MB-300MB.
Profile (30x30 grid) just for reference: https://share.firefox.dev/3XsOVvj
Profile (40x40 grid): https://share.firefox.dev/3Zd2IaF

Comment 13

[Mayank Bansal]

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

Mayank, just a heads-up that this might revert some of the changes on JetStream because this makes us match the previous behavior better.

23.8% regression on Jetstream2-espree-wtb-Average and other espree tests

Comment 14

[Monica Chiorean]

I was able to reproduce the issue on Win11x64 using FF build 131.0a1(2024-08-30).
Verified as fixed on Win11x64 using FF build 132.0b2 and 133.0a1.