Closed Bug 1664312 Opened 3 months ago Closed 2 months ago

Optimize length-1 strings in js::ParserAtoms

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

RESOLVED FIXED
82 Branch
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.

Depends on: 1664826

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

Depends on D90149

This lets us specialize how and when well-known lookups are done.

Depends on D90150

Later patches will add more types of known-atoms and MaybeOneOf has a limit
on the number of types.

Depends on D90151

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

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.

Flags: needinfo?(tcampbell)
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

== 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

Keywords: perf-alert
You need to log in before you can comment on or make changes to this bug.