Closed Bug 1674351 Opened 2 years ago Closed 2 years ago

Split ParserAtomTable into vector and index map

Categories

(Core :: JavaScript Engine, task, P1)

task

Tracking

()

RESOLVED FIXED
84 Branch
Tracking Status
firefox84 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(6 files, 1 obsolete file)

Currently CompilationStencil contains ParserAtomsTable, that is basically a set of ParserAtom pointer.
the "set" property of it isn't necessary while decoding, and we don't have to calculate hash etc there.

Splitting the "set" property and storage ("vector") should help decoding performance.
Also, after that, we can use index into the vector to point the ParserAtom, and replace ParserAtom pointer inside stencil with it.

there's problem around using index from the beginning, that is,
while parsing we create many parser atoms that won't be used by stencil (local names and strings inside lazy function).
if we put all atoms into the single vector, the vector will contain many unused atoms, and index for used atoms will contain many gaps.
we may need to map "all atoms" into "atoms used by stencil" before encoding.

bench from bug 1674306, with WIP patch:
https://hg.mozilla.org/try/pushloghtml?changeset=822ff0d1a71f6487166d6a0c8545c6dbf91f194c

$ js xdr-test/b.js
[before]
7138
1890
3048
2894

$ js --no-off-thread-parse-global xdr-test/b.js
[after]
12742
2898
4811
4849

for the first data, 13% improvement from unpatched tree (14687),
but still it's 80% regression from without-stencil-mvp ([before]).

will look into integration with bug 1668672

Rebased onto bug 1668672, and now the score for the 1st data is 10788.
without-stencil-mvp score also improves to 6667, and now it's 61% regression between before/after.

As a preparation to remove set from stencil, separate the deduplication part
and storage part of ParserAtomsTable.

The later patch will move HashMap out of stencil.

Depends on D95838

Later patch will move ParserAtomsTable field from CompilationStencil to CompilationState.
To simplify the change, add a reference for the CompilationStencil.parserAtoms in CompilationState.
The next patch will rewrite consumers to use the reference field.

Depends on D95839

Now ParserAtomsTable is part of CompilationState, or stack variable inside XDRStencilDecoder.
It's always created after CompilationInfo is stored into Rooted, and now
we don't have to worry about LifoAlloc pointer in move constructor.

Depends on D95841

ParserAtomVectorBuilder is simplified version of ParserAtomsTable for XDR.
This doesn't perform deduplication, and just builds vector.

Depends on D95842

DRParserAtomTable and ParserAtomVector has the same content.
Removed XDRParserAtomTable and rewrote consumers to use ParserAtomVector.

Depends on D95843

Attachment #9185675 - Attachment is obsolete: true
Attachment #9185676 - Attachment description: Bug 1674351 - Part 2: Split HashSet of ParserAtomEntry into Vector and HashMap. r?tcampbell! → Bug 1674351 - Part 1: Split HashSet of ParserAtomEntry into Vector and HashMap. r?tcampbell!
Attachment #9185677 - Attachment description: Bug 1674351 - Part 3: Add CompilationState.parserAtoms field as a preparation. r?tcampbell! → Bug 1674351 - Part 2: Add CompilationState.parserAtoms field as a preparation. r?tcampbell!
Attachment #9185678 - Attachment description: Bug 1674351 - Part 4: Use CompilationState.parserAtoms in parser and emitter. r?tcampbell! → Bug 1674351 - Part 3: Use CompilationState.parserAtoms in parser and emitter. r?tcampbell!
Attachment #9185679 - Attachment description: Bug 1674351 - Part 5: Move ParserAtomVector out of ParserAtomsTable. r?tcampbell! → Bug 1674351 - Part 4: Move ParserAtomVector out of ParserAtomsTable. r?tcampbell!
Attachment #9185680 - Attachment description: Bug 1674351 - Part 6: Add ParserAtomVectorBuilder. r?tcampbell! → Bug 1674351 - Part 5: Add ParserAtomVectorBuilder. r?tcampbell!
Attachment #9185681 - Attachment description: Bug 1674351 - Part 7: Merge XDRParserAtomTable into ParserAtomVectorBuilder. r?tcampbell! → Bug 1674351 - Part 6: Merge XDRParserAtomTable into ParserAtomVectorBuilder. r?tcampbell!
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/autoland/rev/a49a4fc7caad
Part 1: Split HashSet of ParserAtomEntry into Vector and HashMap. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/6af11f8f1983
Part 2: Add CompilationState.parserAtoms field as a preparation. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/7a4ce0db75ba
Part 3: Use CompilationState.parserAtoms in parser and emitter. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/6f1bf8c8117f
Part 4: Move ParserAtomVector out of ParserAtomsTable. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/e4312f01e2d6
Part 5: Add ParserAtomVectorBuilder. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/f0df0bb8123d
Part 6: Merge XDRParserAtomTable into ParserAtomVectorBuilder. r=tcampbell
You need to log in before you can comment on or make changes to this bug.