1898280 - Prevent chains of dependent strings from LSubstr

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[Jan de Mooij [:jandem]]

There are two known ways to get dependent => dependent strings, (1) the JIT's LSubstr code and (2) an extensible string with a dependent string that is later also turned into a dependent string during rope flattening.

For (1) we could emit code to unwrap dependent strings, similar to what we already do in CreateDependentString::generate. Now that we have the new DEPENDED_ON_BIT, we can probably use it to prevent (2).

Dependent strings are complicated and this invariant would help simplify them a bit.

Comment 1

[Jan de Mooij [:jandem]]

Steve, do you have cycles to look into this? If not I could take a stab at it.

Comment 2

[Steve Fink [:sfink] [:s:]]

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

Steve, do you have cycles to look into this? If not I could take a stab at it.

It would be at least a week out for me right now (probably longer, given my history of estimates). Please go ahead if you have time.

Comment 3

[Jan de Mooij [:jandem]]

I've implemented this and it does work: there are no chains of dependent strings if we fix the two cases in comment 0. This lets us simplify nursery GC code a bit where we currently have to handle chains of base strings.

What's unfortunate though is that if we flatten a rope tree, any interior ropes become dependent strings with the original root as base. This means we can then never reuse the root's extensible string buffer because that would create dependent => dependent chains. This is true in about 50% of the cases on Speedometer 3 where we are currently able to reuse the buffer (the extensible-string optimization isn't very hot on speedometer 3: a few hundred hits for a full run).

So for now I'll just fix the LSubstr case so that JIT code is at least consistent with the C++ code.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1898280 - Avoid creating trivial dependent string chains in LSubstr codegen. r?iain!

We were unwrapping dependent strings (one level) in the C++ code and in
CreateDependentString::generate, but not for LSubstr.

Also share more code between CreateDependentString and LSubstr.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/222de2bd49bd
Avoid creating trivial dependent string chains in LSubstr codegen. r=iain

Comment 6

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/222de2bd49bd

Comment 7

[Mayank Bansal]

This bug probably lead to improvements on awfy-jetstream2-offline-assembler subtests:

24% on offline-assembler-average
22% on OfflineAssembler-Geometric
38% on OfflineAssembler-Worst

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Mayank Bansal from comment #7)

This bug probably lead to improvements on awfy-jetstream2-offline-assembler subtests:

Nice. I wasn't expecting big performance improvements from this, but this test has a lot of regular expression code and a number of str.slice calls so that's definitely possible. I might take a closer look at it.

Comment 9

[Jan de Mooij [:jandem]]

Markus created a profile comparison: https://github.com/jrmuizel/js-profile-compare/blob/main/reports/JetStream2-OfflineAssembler-FirefoxLinux-Bug1898280.md

The performance improvement comes from a lot less Minor GC time because we're no longer keeping a potentially very long chain of dependent strings alive.