Optimize length-1 strings in js::ParserAtoms
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox82 | --- | fixed |
People
(Reporter: tcampbell, Assigned: tcampbell)
References
Details
(Keywords: perf-alert)
Attachments
(5 files)
As shown in Bug 1663665, there are some regressions attributable to the ParserAtoms code landing. There are a few causes, but one that is pronounced in the raptor-tp6-netflix-cold case is that ParserAtoms do not have any fast-path for length-1/2 strings. In the normal atomize path we use the StaticStrings mechanism and should do the same.
|
Assignee | |
Comment 1•3 months ago
|
||
In this code we often use 'length' to refer to number of char16_t and this
can be a little confusing with UTF-8 so instead use 'nbytes' as the argument
until we establish what length of the inflated sequence is.
Depends on D90148
|
|
Assignee | |
Comment 2•3 months ago
|
||
Depends on D90149
|
|
Assignee | |
Comment 3•3 months ago
|
||
This lets us specialize how and when well-known lookups are done.
Depends on D90150
|
|
Assignee | |
Comment 4•3 months ago
|
||
Later patches will add more types of known-atoms and MaybeOneOf has a limit
on the number of types.
Depends on D90151
|
|
Assignee | |
Comment 5•3 months ago
|
||
This add a similar optimization as js::StaticStrings for tiny strings to the
ParserAtoms mechanism. This includes (some) length 1 and length 2 atoms for
fast lookup. This is effective for large minified JS files and can speed up
syntax-parsing by up to 20% in some cases. We extend the atomIndex_ Variant
to have new StaticParserString{1,2} types which we use to in toJSAtom to
quickly translate to the corresponding StaticStrings.
Depends on D90152
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8867b79ee8ea Rename ParserAtomsTable::internUtf8 length arg to nbyte r=djvj https://hg.mozilla.org/integration/autoland/rev/7c31676a20cd Reorder ParserAtomsTable methods r=djvj https://hg.mozilla.org/integration/autoland/rev/210475dc1429 Inline ParserAtomsTable::lookupOrInternChar16Seq into callers r=djvj https://hg.mozilla.org/integration/autoland/rev/15482e97aa4a Use mozilla::Variant in js::frontend::ParserAtomEntry r=djvj https://hg.mozilla.org/integration/autoland/rev/71d14ede1fcb Optimize tiny atoms in ParserAtomsTable r=djvj
|
||
Comment 7•2 months ago
|
||
Backed out for valgrind bustages about ParserAtom.cpp.
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=315868064&repo=autoland&lineNumber=66784
Backout link: https://hg.mozilla.org/integration/autoland/rev/b2d8201169eaddec051abaae7685656b2bd2a3fb
|
|
Assignee | |
Comment 8•2 months ago
|
||
Leaking the ParserAtomEntrys used for tiny strings. Moved to UniquePtr and verified valgrind looks good (for jsshell) again. This was a shutdown leak only since they data lives as long as the main JSRuntime, but still should be fixed.
Pushed by tcampbell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b8e17781c000 Rename ParserAtomsTable::internUtf8 length arg to nbyte r=djvj https://hg.mozilla.org/integration/autoland/rev/36a190a3d381 Reorder ParserAtomsTable methods r=djvj https://hg.mozilla.org/integration/autoland/rev/13ace5d469df Inline ParserAtomsTable::lookupOrInternChar16Seq into callers r=djvj https://hg.mozilla.org/integration/autoland/rev/dc0371757070 Use mozilla::Variant in js::frontend::ParserAtomEntry r=djvj https://hg.mozilla.org/integration/autoland/rev/c5bf76bc593f Optimize tiny atoms in ParserAtomsTable r=djvj
|
||
Comment 10•2 months ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/b8e17781c000
https://hg.mozilla.org/mozilla-central/rev/36a190a3d381
https://hg.mozilla.org/mozilla-central/rev/13ace5d469df
https://hg.mozilla.org/mozilla-central/rev/dc0371757070
https://hg.mozilla.org/mozilla-central/rev/c5bf76bc593f
|
||
Comment 11•2 months ago
|
||
== Change summary for alert #27061 (as of Fri, 25 Sep 2020 12:38:40 GMT) ==
Improvements:
4% google-maps loadtime android-hw-g5-7-0-arm7-api-16-shippable opt warm 1,190.19 -> 1,137.58
2% google-maps PerceptualSpeedIndex android-hw-g5-7-0-arm7-api-16-shippable opt warm 1,021.76 -> 999.33
2% google-maps fcp android-hw-g5-7-0-arm7-api-16-shippable opt warm 930.22 -> 910.08
2% google-maps FirstVisualChange android-hw-g5-7-0-arm7-api-16-shippable opt warm 940.81 -> 921.00
2% google-maps SpeedIndex android-hw-g5-7-0-arm7-api-16-shippable opt warm 1,028.48 -> 1,007.58
2% google-maps fcp android-hw-g5-7-0-arm7-api-16-shippable opt warm 930.75 -> 914.67
For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=27061
Description
•