1687095 - Merge delazification stencil into initial stencil before encoding

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

In incremental stencil XDR, we encode initial stencil first, and then encode each delazification after that.

it means that, if a script is delazified, we have both lazy script data and delazified script data inside XDR.
ultimately, the lazy script data isn't necessary in the instantiated GC objects.

if we can somehow dedup the info, or drop unnecessary info, we can reduce the XDR size.

Comment 1

[Tooru Fujisawa [:arai]]

the lazy script data is necessary for the current instantiation steps, because we first instantiate initial stencil,
and then apply delazification stencils' instantiation, just like normal delazification.

if we somehow drop the first lazy data, the steps needs to be modified.

we had an idea about merging stencils on decode, before instantiation.
https://phabricator.services.mozilla.com/D90574
https://phabricator.services.mozilla.com/D90585
we might be able to revisit it.

Comment 2

[Tooru Fujisawa [:arai]]

With current structure, we cannot remove a certain lazy script data from XDR data with simple move/copy operation, because data is stored into multiple spans,
and removing an item breaks the remaining part's indices (or, if we don't shift the items, it doesn't affect the space).

So, to remove the space taken by lazy script, we need to decode XDR data, and then remove the data taken by the lazy script (while keeping the lazy script's index itself as placeholder), and renumber remaining things, and then re-encode it at linearlize step. This needs to be operated recursively for all delazifications.
And when decoding, we need to merge all stencils into one, while ignoring the removed data.
The cost would almost be same as just decoding, merging, and re-encoding all stencils at linearlize step.

Either way, it's not good because it's performed on main thread, and taking long there results in long interruption.

So, if we want to reduce XDR size with such way, we should think about offloading the linearlize or the post-process work to helper thread.

Comment 3

[Tooru Fujisawa [:arai]]

If we can bring initial CompilationStencil (or gc-free subset) around with ScriptSorce or somewhere while incremental encoding,
we can merge delazification into the initial stencil there, and encode the entire stencil at once when finalizing.
so that there's no duplication around script data, and also bug 1687094 will also be solved at the same time.

Comment 4

[Tooru Fujisawa [:arai]]

Initial CompilationStencil's fields that is necessary for merging and encoding:

• all BaseCompilationStencil fields
  • delazifications are merged into them
• CompilationStencil.alloc
  • all delazification should use this allocator, given the result will be merged and held by initial stencil
• CompilationStencil.input.options.instrumentationKinds
  • just to check if the stencil is encodable. this can be done earlier
• CompilationStencil.moduleMetadata
• CompilationStencil.scriptExtra

In addition to that, it would be nice to reuse initial compilation's CompilationState.parserAtoms in delazification compilation,
so that atom indices created in delazifications doesn't require remapping when merging.

Comment 5

[Tooru Fujisawa [:arai]]

the difference between the previous merge patch and the current situation is:

• CompilationStencil was using Vector, but now it uses Span, and it's not extensible
• CompilationStencil was using ParserAtomsTable but now it uses ParserAtomSpan, and cannot intern/add ParserAtom
• most data was using SystemAllocPolicy, but now all data is stored in LifoAlloc

so, if we keep using Span + LifoAlloc, merging (appending/concatenating) on each delazification isn't efficient.
it's better not to append/concatenate the span content on each delazification, but keep those Spans in "vector of Span",
and when finishing, concatenate all spans into single Vector, with mapping indices.

another solutions would be:

• add non-Span variant of CompilationStencil (Vector + ParserAtomsTable), and use the previous way for encoding
• make CompilationState portable, and store it in ScriptSource, and reuse it across initial and delazification, and call CompilationState::finish when finishing encoding

Comment 6

[Tooru Fujisawa [:arai]]

The reason why we chose Span was:

• a) to avoid alloc/ctor/dtor cost of Vector
• b) to directly use XDR buffer

for (a), if we don't store delazifications in XDR, the cost of Vector alloc/ctor/dtor on decode would be negligible,
but (b) still holds and we need Span, or at least variant.

Comment 7

[Tooru Fujisawa [:arai]]

about reusing ParserAtomsTable:

if we reuse ParserAtomsTable across initial and delazification, we can get consistent TaggedParserAtomIndex, but
instantiation has problem, that delazification has to instantiate more atoms than current way, given
we don't have info that which atom is used by current compilation (delazification),
and also we don't keep atomCache across initial/delazification.

Comment 8

[Tooru Fujisawa [:arai]]

the simplest way would be, just collect all CompilationStencil/BaseCompilationStencil references in the encoder,
and merge them while encoding.

we can use StencilDelazificationSet (or RefPtr/UniquePtr variant) for collecting them

Comment 9

[Tooru Fujisawa [:arai]]

With WIP patch that merges stencils before encoding, the size of raptor yahoo-mail, mail_app_es6_09* XDR is reduced by 14%.
there, AtomTable is reduced by 51%, and ScriptData is also reduced by 51%.

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 1: Move Vector-variant of CompilationStencil out of CompilationState. r=tcampbell

To return Vector-variant of CompilationStencil as the result of compilation,
it needs to be allocated separately from other CompilationState.

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 2: Support encoding ExtensibleCompilationStencil.

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 3: Use ExtensibleCompilationStencil as a compilation output in self-hosting.

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 2: Make CmpilationStencil and ExtensibleCompilationStencil convertible each other. r=tcampbell

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 2: Add CompilationStencilMerger. r=tcampbell!

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 3: Use CompilationStencilMerger in XDRIncrementalStencilEncoder. r=tcampbell!

Also add XDRStencilEncoder for non-incremental case, and
cleanup XDRStencilDecoder.

StencilDelazificationSet and gcOutputForDelazification become unused,
and will be removed by later patches.

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 4: Remove StencilDelazificationSet. r=tcampbell

Now all stencils don't have associated delazification.

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 5: Remove gcOutputForDelazification. r=tcampbell

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 6: Merge BaseCompilationStencil and CompilationStencil. r=tcampbell

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 9: Use ExtensibleCompilationStencil as off-thread-script-compilation output. r=tcampbell!

Depends on D105157

Comment 20

[Tooru Fujisawa [:arai]]

After removing BaseCompilationStencil, we could make the instantiation related methods non-static again.

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 4: Add BorrowingCompilationStencil.

BorrowingCompilationStencil provides a CompilationStencil instance that
borrows all ExtensibleCompilationStencil data,

Not-borrow-able fields need to be moved to the temporary CompilationStencil,
and BorrowingCompilationStencil destructor returns them.

Depends on D105153

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687095 - Part 1: Add {CompilationStencil,ExtensibleCompilationStencil}::steal. r=tcampbell

To make it possible to convert them each other.

Depends on D105908

Comment 23

[Ted Campbell [:tcampbell]]

Attachment: Bug 1687095 - Use CompilationStencil& argument to incremental-encoder. r=arai

Since merging stencils is a read-only operation for the source delazification
stencil and we already have a borrowed stencil at caller, it is more
consistent with our conventions to pass a CompilationStencil& instead of an
ExtensibleCompilationStencil&.

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07d2db86c609
Part 1: Add {CompilationStencil,ExtensibleCompilationStencil}::steal. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/6ab4cbb8f4a2
Part 2: Add CompilationStencilMerger. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/8e59cd75371f
Part 3: Use CompilationStencilMerger in XDRIncrementalStencilEncoder. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/d1fc4ee5aa50
Part 4: Remove StencilDelazificationSet. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/1ad324a72551
Part 5: Remove gcOutputForDelazification. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/13b97543f899
Part 6: Merge BaseCompilationStencil and CompilationStencil. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/efff9a5c7a1f
Use CompilationStencil& argument to incremental-encoder. r=arai

Comment 25

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/07d2db86c609
https://hg.mozilla.org/mozilla-central/rev/6ab4cbb8f4a2
https://hg.mozilla.org/mozilla-central/rev/8e59cd75371f
https://hg.mozilla.org/mozilla-central/rev/d1fc4ee5aa50
https://hg.mozilla.org/mozilla-central/rev/1ad324a72551
https://hg.mozilla.org/mozilla-central/rev/13b97543f899
https://hg.mozilla.org/mozilla-central/rev/efff9a5c7a1f

Comment 26

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for frequent crashes in Nightly:

https://hg.mozilla.org/mozilla-central/rev/432c586951243c132694fa9988a691fbf5198cca

Example of crashes: bp-6d1d6c1a-c45e-4277-9d9b-12c3d0210303

 0 	xul.dll	static js::frontend::CompilationStencil::instantiateStencilAfterPreparation(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&)	js/src/frontend/Stencil.cpp:1341
1 	xul.dll	js::frontend::InstantiateStencils(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&)	js/src/frontend/BytecodeCompiler.cpp:386
2 	xul.dll	js::ParseTask::instantiateStencils(JSContext*)	js/src/vm/HelperThreads.cpp:722
3 	xul.dll	js::ScriptDecodeTask::parse(JSContext*)	js/src/vm/HelperThreads.cpp:836
4 	xul.dll	js::ParseTask::runHelperThreadTask(js::AutoLockHelperThreadState&)	js/src/vm/HelperThreads.cpp:619
5 	xul.dll	static js::HelperThread::ThreadMain(void*)	js/src/vm/HelperThreads.cpp:2366
6 	xul.dll	static js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start(void*)	js/src/threading/Thread.h:205
7 	ucrtbase.dll	thread_start<unsigned int (__cdecl*)(void*), 1>	
8 	kernel32.dll	BaseThreadInitThunk	
9 	ntdll.dll	RtlUserThreadStart	
10 	kernelbase.dll	TerminateProcessOnMemoryExhaustion	

Nightly updates have been halted and will resume once the new nightly builds with the backout are available.

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aef34d5f65e6
Part 1: Add {CompilationStencil,ExtensibleCompilationStencil}::steal. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/34a37bdb73ac
Part 2: Add CompilationStencilMerger. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/1f89cffbd3d5
Part 3: Use CompilationStencilMerger in XDRIncrementalStencilEncoder. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/4f27f5ca8656
Part 4: Remove StencilDelazificationSet. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/075c72b787a2
Part 5: Remove gcOutputForDelazification. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/d7815415080c
Part 6: Merge BaseCompilationStencil and CompilationStencil. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/9419ba69b445
Use CompilationStencil& argument to incremental-encoder. r=arai

Comment 28

[Atila Butkovits]

Backed out for causing Spidermonkey failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/f953699c07a294ea44ed655ca0cee3b547509c56

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&selectedTaskRun=eloqS4w6T6W9VsZ6ARmMoQ.0&revision=9ea9e1c55a78f439718532f7188cf42f1622adac&searchStr=linux%2Cdebug%2Cspidermonkey%2Cbuilds%2Cspidermonkey-sm-arm-sim-linux32%2Fdebug%2Carm

Failure log: https://treeherder.mozilla.org/logviewer?job_id=331962401&repo=autoland

Comment 29

[Tooru Fujisawa [:arai]]

it's bug 1696205 patch.
will fix there.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19f9592d23c1
Part 1: Add {CompilationStencil,ExtensibleCompilationStencil}::steal. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/cf67566b48ab
Part 2: Add CompilationStencilMerger. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/21e179ac5547
Part 3: Use CompilationStencilMerger in XDRIncrementalStencilEncoder. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/b2c2e8ec8016
Part 4: Remove StencilDelazificationSet. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/de2a4e293b88
Part 5: Remove gcOutputForDelazification. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/2ccfa2d73d3c
Part 6: Merge BaseCompilationStencil and CompilationStencil. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/0e8c6b3bd1e2
Use CompilationStencil& argument to incremental-encoder. r=arai

Comment 31

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/19f9592d23c1
https://hg.mozilla.org/mozilla-central/rev/cf67566b48ab
https://hg.mozilla.org/mozilla-central/rev/21e179ac5547
https://hg.mozilla.org/mozilla-central/rev/b2c2e8ec8016
https://hg.mozilla.org/mozilla-central/rev/de2a4e293b88
https://hg.mozilla.org/mozilla-central/rev/2ccfa2d73d3c
https://hg.mozilla.org/mozilla-central/rev/0e8c6b3bd1e2