1905117 - Allocate more JS strings with a StringBuffer instead of raw malloc

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P2)

Description

[Jan de Mooij [:jandem]]

I have some patches that add support for StringBuffer to JSString::OwnedChars, and then uses this to allocate more JS strings with string buffers instead of raw malloc.

Comment 1

[Jan de Mooij [:jandem]]

Attachment: Bug 1905117 part 1 - Tidy up checkStringCharsArena checks. r?sfink!

This patch adds a usesStringBuffer argument to checkStringCharsArena so that
we can have this check and comment in one place.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1905117 part 2 - Refactor OwnedChars to prepare for StringBuffer support. r?sfink!

This adds an enum to OwnedChars to make it easier to add a new kind of allocation in
the next patch.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1905117 part 3 - Add StringBuffer support to OwnedChars. r?sfink!

Also removes the new_ and newValidLength overloads that were added for string
buffers because we can now use the OwnedChars version.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1905117 part 4 - Allocate a StringBuffer in AllocChars for longer strings. r?sfink!

If there are more than 500 bytes of string data, use a StringBuffer instead of raw malloc.
This helps avoid memory usage regressions because for shorter strings StringBuffer has
relatively more overhead.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15b620ae11bf
part 1 - Tidy up checkStringCharsArena checks. r=sfink
https://hg.mozilla.org/integration/autoland/rev/1e290c561e40
part 2 - Refactor OwnedChars to prepare for StringBuffer support. r=sfink
https://hg.mozilla.org/integration/autoland/rev/ee041ddb584c
part 3 - Add StringBuffer support to OwnedChars. r=sfink

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/15b620ae11bf
https://hg.mozilla.org/mozilla-central/rev/1e290c561e40
https://hg.mozilla.org/mozilla-central/rev/ee041ddb584c

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1905117 part 5 - Use JS string arena for StringBuffers allocated in SpiderMonkey. r?sfink!,emilio!

This avoids some regressions on a few Speedometer 3 (async) sub tests.

The problem is likely more jemalloc lock contention during GC when using the
main malloc heap. Using SpiderMonkey's string buffer arena avoids that and
matches previous behavior better.

Comment 8

[Mayank Bansal]

Ill let the sheriffs confirm, but this change appear to have caused 15% regressions on some tests: https://treeherder.mozilla.org/perfherder/alerts?id=995&hideDwnToInv=0

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6fe78640acb3
part 4 - Allocate a StringBuffer in AllocChars for longer strings. r=sfink
https://hg.mozilla.org/integration/autoland/rev/0584adf37451
part 5 - Use JS string arena for StringBuffers allocated in SpiderMonkey. r=sfink,emilio

Comment 10

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/6fe78640acb3
https://hg.mozilla.org/mozilla-central/rev/0584adf37451