1674351 - Split ParserAtomTable into vector and index map

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Tooru Fujisawa [:arai]]

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.

Comment 1

[Tooru Fujisawa [:arai]]

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.

Comment 2

[Tooru Fujisawa [:arai]]

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

Comment 3

[Tooru Fujisawa [:arai]]

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.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1674351 - Part 1: Add HashMap::putNew with different lookup. r?jwalden!

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1674351 - Part 1: Split HashSet of ParserAtomEntry into Vector and HashMap. r?tcampbell!

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

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1674351 - Part 2: Add CompilationState.parserAtoms field as a preparation. r?tcampbell!

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

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1674351 - Part 3: Use CompilationState.parserAtoms in parser and emitter. r?tcampbell!

Depends on D95840

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1674351 - Part 4: Move ParserAtomVector out of ParserAtomsTable. r?tcampbell!

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

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1674351 - Part 5: Add ParserAtomVectorBuilder. r?tcampbell!

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

Depends on D95842

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1674351 - Part 6: Merge XDRParserAtomTable into ParserAtomVectorBuilder. r?tcampbell!

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

Depends on D95843

Comment 11

[Pulsebot]

Pushed by [e-mail]:
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

Comment 12

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/a49a4fc7caad
https://hg.mozilla.org/mozilla-central/rev/6af11f8f1983
https://hg.mozilla.org/mozilla-central/rev/7a4ce0db75ba
https://hg.mozilla.org/mozilla-central/rev/6f1bf8c8117f
https://hg.mozilla.org/mozilla-central/rev/e4312f01e2d6
https://hg.mozilla.org/mozilla-central/rev/f0df0bb8123d