Prevent chains of dependent strings from LSubstr
Categories
(Core :: JavaScript Engine, enhancement, P2)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox129 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
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.
|
Assignee | |
Comment 1•1 month ago
|
||
Steve, do you have cycles to look into this? If not I could take a stab at it.
|
||
Comment 2•1 month ago
|
||
(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.
|
||
Updated•28 days ago
|
|
|
Assignee | |
Updated•6 days ago
|
|
|
Assignee | |
Comment 3•5 days ago
|
||
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.
|
|
Assignee | |
Comment 4•5 days ago
|
||
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.
Pushed by jdemooij@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/222de2bd49bd Avoid creating trivial dependent string chains in LSubstr codegen. r=iain
|
||
Comment 6•1 day ago
|
||
| bugherder | ||
|
||
Comment 7•12 hours ago
|
||
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
|
|
Assignee | |
Comment 8•11 hours ago
|
||
(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.
Description
•