1692648 - Add Vector/ParserAtomsTable variant of CompilationStencil

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

To achieve bug 1687095, one of the solution is to add Vector/ParserAtomsTable variant of CompilationStencil, and store the initial stencil of that type into XDR encoder associated with ScriptSource,
and merge/accumulate all delazifications into the initial stencil,
and encode the single CompilationStencil into XDR buffer.

This will solve the ScriptStencil duplication, ParserAtom duplication,
and also reduce the time taken by decode+instantiate.

This will also help bug 1692577, by using Vector/ParserAtomsTable variant of CompilationStencil as the base class of CompilationState, and instantiate directly from it,
so that we can skip CompilationState::finish call.

The plan is:

• Templatize BaseCompilationStencil/CompilationStencil for Span/ParserAtomSpan types
• Add Vector/ParserAtomsTable variant of BaseCompilationStencil/CompilationStencil
• Make CompilationState a subclass of the Vector/ParserAtomsTable variant of CompilationStencil
• Accumulate all information into CompilationStencil from parser/emitter (don't store any info into final CompilationStencil)
• Make CompilationState::finish copy/move all data from Vector variant to Span variant
• Support instantiation from Vector/ParserAtomsTable variant of CompilationStencil
• for the case CompilationStencil doesn't escape compilation scope, do not call CompilationState::finish but instantiate from CompilationState

Comment 1

[Tooru Fujisawa [:arai]]

And the plan for bug 1687095:

• In initial compilation, do not finish into Span variant of CompilationStencil, but instantiate from Vector variant of CompilationStencil, and store it into XDRIncrementalStencilEncoder
• In delazification, do not finish into Span variant of CompilationStencil, but instantiate from Vector variant of CompilationStencil, and merge it into initial stencil stored in XDRIncrementalStencilEncoder
  • with Vector/ParserAtomsTable variant of CompilationStencil, previous patches' way should work, with minor modification:
    • https://phabricator.services.mozilla.com/D90574
    • https://phabricator.services.mozilla.com/D90585
• when encoding, encode the merged initial stencil
• when decoding, decode the merged initial stencil

Comment 2

[Tooru Fujisawa [:arai]]

(In reply to Tooru Fujisawa [:arai] from comment #1)

And the plan for bug 1687095:

• In initial compilation, do not finish into Span variant of CompilationStencil, but instantiate from Vector variant of CompilationStencil, and store it into XDRIncrementalStencilEncoder

This will require detaching Vector variant of CompilationStencil from CompilationState, either by moving/copying, or perhaps allocating it on heap instead of using it as base class.

Comment 3

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/jobs?repo=try&revision=15cf44d97397a307b6f10a07e31cecc478adc07f

Comment 4

[Tooru Fujisawa [:arai]]

More plan for bug 1687095:

• once decoding stops using StencilDelazificationSet, remove delazificationSet field
• merge BaseCompilationStencilTemplate and CompilationStencilTemplate

Comment 5

[Ted Campbell [:tcampbell]]

It might be nice for the 'finish' method to be something like CompilationStencil::unshare and have it also support the XDR-decode case if we wanted to the stencil to out-live the XDR buffer in some cases.

Comment 6

[Tooru Fujisawa [:arai]]

in current patch stack, the implementation of finish method is a copy/move from Vector variant to Span variant,
and internally it supports other direction as well (it was to support "decode and start incremental encoding" tho),
so we could use the "Span variant -> Vector variant" to detach stencil from XDR buffer.

Comment 7

[Ted Campbell [:tcampbell]]

I was just imagining from Span to -> Span-over-LifoAlloc as a simple thing.

Comment 8

[Tooru Fujisawa [:arai]]

okay, will look into it.

Comment 9

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/jobs?repo=try&revision=7c9877b2606d8def5e24d894f852904389dd56bb

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 1: Pass LifoAlloc to CompilationState constructor. r=tcampbell!

As a preparation to decouple CompilationState from the final CompilationStencil,
removed the direct dependency to CompilationStencil from the constructor.

The later patch adds yet another LifoAlloc inside CompilationState base class.

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 2: Pass LifoAlloc to ScopeStencil::createFor*. r=tcampbell!

Depends on D105140

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 3: Templatize BaseCompilationStencil/CompilationStencil for Span/ParserAtomSpan type.

To add Vector/ParserAtomsTable variant,
templatize BaseCompilationStencil/CompilationStencil for Span/ParserAtomSpan.

Span type is defined for each element type, to support different Vector inline
storage for each in bug 1689161.

Depends on D105141

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 3: Move CompilationState definition after CompilationStencil. r=tcampbell!

As a preparation to make CompilationState a subclass of
AbstractCompilationStencil.

Depends on D105142

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 5: Make CompilationState a subclass of AbstractCompilationStencil.

Add Vector/ParserAtomsTable variant of AbstractCompilationStencil, and
make CompilationState a subclass of it.

Some fields of
AbstractBaseCompilationStencil/AbstractCompilationStencil are unused,
but the later patch fixes it by decoupling parser/emitter from final
CompilationStencil.

Depends on D105143

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 5: Accumulate all data into CompilationState, and copy/move to CompilationStencil in CompilationState::finish. r=tcampbell!

Instead of direclty stroing into CompilationStencil, store all data into
CompilationState, from Parser and BytecodeEmitter.

Depends on D105144

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 7: Simplify AbstractBaseCompilationStencil::prepareStorageFor.

Now prepareStorageFor is called only for CompilationState.
Move/rename it to CompilationState::prepareSharedDataStorage, and directly
use member fields.

Depends on D105145

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 8: Add ExtensibleCompilationStencil to simplify the constructor.

Depends on D105146

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 9: Support instantiation from ExtensibleCompilationStencil.

Depends on D105147

Comment 19

[Tooru Fujisawa [:arai]]

after this, maybe we can split Span variant of CompilationStencil into 2 classes, one that points XDR buffer, and one that points only LifoAlloc,
sharing the base template class to keep using the same code.

Comment 20

[Tooru Fujisawa [:arai]]

After bug 1687095, we can modify each CompilationStencil consumer to use ExtensibleCompilationStencil, so that extra copy doesn't happen in most case.

Comment 21

[Tooru Fujisawa [:arai]]

(In reply to Ted Campbell [:tcampbell] from comment #7)

I was just imagining from Span to -> Span-over-LifoAlloc as a simple thing.

https://phabricator.services.mozilla.com/D105152 this patch adds "Span/pointer-over-XDR" to "Vector+pointer-over-LifoAlloc" code.
If necessary, we can templatize it to support "Span/pointer-over-XDR" to "Span/pointer-over-LifoAlloc" code as well.

Comment 22

[Ted Campbell [:tcampbell]]

What is your general thought here around future Smoosh/Rust support? I presume if we continue that work we'd want to get away from Frontend2.cpp if possible. Would we simply keep the ScriptStencil stuff Rust-friendly and do a bit of glue to generate the CompilationStencil? Not a serious concern now, but I want to consider a few different futures if it is easy.

Comment 23

[Tooru Fujisawa [:arai]]

IMO, the top-level struct doesn't necessarily be byte-to-byte compatible, and some kind of conversion/map is acceptable.
(of course it's better not having such conversion tho)
For inner pointer-free structs like ScriptStencil, we should keep it XDR-friendly, and that will result in Rust-friendly anyway.

In smoosh binding, currently we perform conversion for all types just because we cannot use single [repr(C)] definition across them, for dependency reason.
Once we make jsparagus crate required for all build, we can create a binding from jsparagus's code and use single struct,
that will let us make top-level struct compatible, or generate conversion method automatically.

For Span vs Vector variation, some kind of it will be necessary anyway also in smoosh,
either by having 2 container types, or adding yet-another container that's compatible with both usage (extensible/own vs static/borrow).
the latter might be simpler if we cross the C++/Rust binding, but former will also work with trait and some macro.
in any way, we would use different container types than MFBT, and some kind of rewrite is necessary around consumer.

Comment 24

[Tooru Fujisawa [:arai]]

Attachment: Bug 1692648 - Part 4: Add ExtensibleCompilationStencil and move stencil-related fields from CompilationState, and make CompilationState subclass of it.

Depends on D105143

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7e752dd5849a
Part 1: Pass LifoAlloc to CompilationState constructor. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/e443f865ee88
Part 2: Pass LifoAlloc to ScopeStencil::createFor*. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/6a6215a33d11
Part 3: Move CompilationState definition after CompilationStencil. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/259111ca1dd0
Part 4: Add ExtensibleCompilationStencil and move stencil-related fields from CompilationState, and make CompilationState subclass of it. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/c4a032c9bf84
Part 5: Accumulate all data into CompilationState, and copy/move to CompilationStencil in CompilationState::finish. r=tcampbell

Comment 26

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7e752dd5849a
https://hg.mozilla.org/mozilla-central/rev/e443f865ee88
https://hg.mozilla.org/mozilla-central/rev/6a6215a33d11
https://hg.mozilla.org/mozilla-central/rev/259111ca1dd0
https://hg.mozilla.org/mozilla-central/rev/c4a032c9bf84