1917397 - Nightly spends tons of time (14s) around MinorGC on an online JSON-to-HTML converter (followup from bug 1864862)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Mayank Bansal]

This is same as bug 1864862, with a minor difference: You need to run the demo a second time immediately after the first run completes and the page is clickable.

STR:

1. Go to https://www.textcompare.org/json/json-to-html/
2. Paste the sample JSON into the input box
3. Click on "Process" --> Be alert after this step
4. Very Imp: As soon as the page is clickable again. click on "Process" again

AR: https://share.firefox.dev/3MCtJg6 , https://share.firefox.dev/3XfP0kL , https://share.firefox.dev/3AWbM9V

14s of time around MinorGC.
Maybe something to improve?

Comment 1

[Mayank Bansal]

Attachment: compressed sample JSON file.zip

Comment 2

[Mayank Bansal]

Attachment: about:support

Comment 3

[Mayank Bansal]

This is a regression.
Bisection:
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c715b86068059bd614e2bac57bba333b46b090e0&tochange=291d187ba249c56d5c9dc88b5faaf04db2afe9c1
Suspect: bug 1892242 . However, I set javascript.options.mem.nursery.max_kb = 16384 and I could still repro.

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1917397 - Tenure substrings in String.prototype.split where appropriate r?jandem

This adds an AutoSelectGCHeap in SplitHelper so that substrings are allocated
in the tenured heap if we trigger a minor GC.

Also this allocates the result array up front to avoid having to copy the
elements for large arrays.

Comment 5

[Jon Coppeard (:jonco)]

The profile still shows some long minor GCs with the patch applied but it should help.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adb5d1c3cf7c
Tenure substrings in String.prototype.split where appropriate r=jandem

Comment 7

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/adb5d1c3cf7c

Comment 8

[Mayank Bansal]

Profile with the latest Nightly:

1. Run the demo thrice: https://share.firefox.dev/47oCUuk
2. Continue with the run#1, manually trigger "free memory", then run the demo thrice: https://share.firefox.dev/4d0h41v

Profile looks good. (Browser is maybe maybe retaining memory that is not released even after manually doing "free memory". But this may be an existing behaviour. The browser behaviour is quite variable. Sometimes it retains 1.8GB memory, sometimes it can retain upto 3.8GB)

Comment 9

[Mayank Bansal]

Attachment: memory-report.json.gz

run the demo 3 times, then wait, then manually trigger "free memory"

Comment 10

[Mayank Bansal]

Attachment: memory-report.json2.gz

Continue with the previous demo run (i.e. tab is not closed)
Run the demo thrice, wait, manually trigger "free memory", dump the memory report.

Comment 11

[Mayank Bansal]

Sorry, I didnt mean to derail this bug with vague/potentially pre-existing memory bugs.
Please feel free to ignore all of that. I am happy with the fix to the minorGC issue.

Comment 12

[Jon Coppeard (:jonco)]

I've forked bug 1920477 for followup investigation of remaining issues and will close this one.