1880044 - Support allocating flattened rope characters in the nursery

Status: ASSIGNED

(Core :: JavaScript Engine, task, P2)

Description

[Jan de Mooij [:jandem]]

On a full Speedometer 3 run we call malloc about 200,000 times for rope flattening. Most of these (> 93% according to some quick-and-dirty logging) could instead allocate the buffer in the nursery.

This should be doable now after bug 1853907. I have a WIP patch that passes most tests, but it needs bug 1879918 to be fixed first.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1880044 part 1 - Remove special case for extensible strings in JSString::sizeOfExcludingThis. r?sfink!

This was added when we calculated the size manually, but with mallocSizeOf
we don't need special handling for extensible strings.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1880044 part 2 - Support extensible strings with a nursery buffer in maybeMallocCharsOnPromotion. r?sfink!

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1880044 part 3 - Add static_assert for capacity overflow in AllocCharsForFlatten. r?sfink!

Also fixes/deletes some outdated comments.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1880044 part 4 - Support allocating flattened rope characters in the nursery. r?sfink!

Now that strings can have nursery-allocated chars, we can also use this when
flattening nursery-allocated ropes.

Comment 6

[Jan de Mooij [:jandem]]

Perf comparison: https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=9176283ad4327fba0c64cf49d2890f7109bed210&originalSignature=4586009&newSignature=4586009&framework=13&application=firefox&originalRevision=355e7ae29a9ac994224f0fd963af5de86ebfdda6&page=1

Improves TodoMVC-JavaScript-ES5/Adding100Items/Sync by about 3.6%. Looking at the latest profile from Markus for this test, I do see some rope flattening time (under DOMParser.parseFromString and Template.prototype.show).

Comment 7

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

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jandem, could you have a look please?
If you still have some work to do, you can add an action "Plan Changes" in Phabricator.
For more information, please visit BugBot documentation.

Comment 8

[Jan de Mooij [:jandem]]

Waiting for some other bugs to land first.

Comment 9

[Iain Ireland [:iain]]

Can this land now?

Comment 10

[Jan de Mooij [:jandem]]

(In reply to Iain Ireland [:iain] from comment #9)

Can this land now?

I have to rebase it on top of bug 1914378 which changes the same code.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/971125148077
part 1 - Remove special case for extensible strings in JSString::sizeOfExcludingThis. r=sfink

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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