1734098 - Move instantiation step out of off-thread compile/decode API

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

Currently single-script off-thread API returns JSScript as the result of the task,
either by performing the instantiation off-thread/on-main-thread.

Once off-thread instantiation is removed by bug 1687973, we should be able to make the API fully based on stencil, and let the consumer instantiate as a separate step.

Comment 1

[Tooru Fujisawa [:arai]]

Things to do:

• Rewrite JS::CompileOffThread consumer to use JS::CompileToStencilOffThread
• Add JS::InstantiateGlobalStencilAndStartIncrementalEncoding API and rewriteJS::FinishOffThreadScriptAndStartIncrementalEncoding` consumer to use it
• Remove JS::CompileOffThread and related APIs
• Add Stencil variant of JS::CompileOffThreadModule and rewrite consumers
• Remove JS::CompileOffThreadModule and related APIs
• Add Stencil variant of JS::DecodeOffThreadScript and rewrite consumers
• Remove JS::DecodeOffThreadScript and related APIs
• Cleanup ParseTask methods/properties, by removing instantiation-specific part

Comment 2

[Tooru Fujisawa [:arai]]

There's a problem around *StartIncrementalEncoding with CompilationStencil vs ExtensibleCompilationStencil.
currently, ScriptParseTask uses ExtensibleCompilationStencil for instantiation and incremental encoding, and there's no not-borrowing CompilationStencil.

current public API doesn't have a concept of "extensible or not" and ExtensibleCompilationStencil is not exposed.

that means, if we return stencil to the caller before starting incremental encoding,
CompilationStencil::steal is called when returning stencil from JS::FinishOffThreadCompileToStencil, and
then ExtensibleCompilationStencil::steal is called when starting incremental encoding, that performs 2 extra copies.

in term of performance, we'd better exposing ExtensibleCompilationStencil to public API as well, to reduce the possible copy,
but for simplicify, it's better having single type for stencil.

Comment 3

[Tooru Fujisawa [:arai]]

Possible Options:

• expose ExtensibleCompilationStencil in public API, and add "extensible" variable for some APIs, and add JS::StartIncrementalEncoding API
• for "finish-off-thread-compilation + instantiate + start-incremental-encoding", keep using single compound API, given it's special case, but refactor the backend, by receiving JS::InstantiateOptions instead carrying it from ParseTask

Comment 4

[Tooru Fujisawa [:arai]]

maybe we can take the latter option, and revisit the design when we hit another instance that requires ExtensibleCompilationStencil in public API

Comment 5

[Tooru Fujisawa [:arai]]

converting JS::FinishOffThreadScript to JS::FinishOffThreadCompileToStencil with JS::InstantiateGlobalStencil also requires 1 extra copy

Comment 6

[Tooru Fujisawa [:arai]]

bug 1743357 will help the API design by avoid unnecessary copy

Comment 7

[Tooru Fujisawa [:arai]]

with bug 1743357 fixed, we can return ExtensibleCompilationStencil without copy, by storing it in CompilationStencil.ownedBorrowStencil.

Comment 8

[Tooru Fujisawa [:arai]]

There's another problem.
in the current JSScript-based off-thread API, we're performing frontend::PrepareForInstantiate off-main thread.
If we simply decouple the instantiation from the off-thread API, frontend::PrepareForInstantiate cannot be performed,
and the CompilationGCOutput allocation is moved to the main thread.

Comment 9

[Tooru Fujisawa [:arai]]

If we're to move the body of the off-thread task to the embedding side, the preparation step should also be exposed.
so, in either case, we need a public struct that wraps CompilationGCOutput, or the buffer that can be stolen by CompilationGCOutput vectors.

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 1: Use consistent function name for off-thread stencil API. r?nbp!

Make the stencil-based off-thread API names follow the convention:

• ...OffThread
• Finish...OffThread
• Cancel...OffThread

This patch stack adds more functions for module/decode, with the above naming
convention.

Depends on D133041

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 2: Add JS::StartIncrementalEncoding. r?nbp!

This is to support incremental encoding with stencil-based off-thread API.

Depends on D133042

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 3: Use JS::CompileToStencilOffThread instead of JS::CompileOffThread in ScriptLoader. r?nbp!

Depends on D133043

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 4: Remove stencil option mismatch check in testing function. r?nbp!

Now there's no code that uses laziness during instantiation, remove the
remaining check for the options

Depends on D133044

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 5: Remove shell offThreadCompileScript. r?nbp!

Rewrite tests to use stencil-based testing functions:

• offThreadCompileScript => offThreadCompileToStencil
• runOffThreadScript => finishOffThreadCompileToStencil+evalStencil

Depends on D133045

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 6: Remove JS::CompileOffThread. r?nbp!

Depends on D133046

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 7: Add JS::CompileModuleToStencilOffThread. r?tcampbell!

Depends on D133047

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 8: Replace shell offThreadCompileModule with offThreadCompileModuleToStencil. r?tcampbell!

• offThreadCompileModule => offThreadCompileModuleToStencil
• finishOffThreadModule => finishOffThreadCompileModuleToStencil+instantiateModuleStencil

Depends on D133048

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 9: Remove JS::CompileOffThreadModule. r?tcampbell!

Depends on D133049

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 10: Fix storageType for decoded stencil without borrowBuffer. r?tcampbell!

Depends on D133050

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 11: Add JS::DecodeStencilOffThread. r?tcampbell!

Also make JS::CanDecodeOffThread to receive JS::DecodeOptions.

Depends on D133051

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 12: Replace shell offThreadDecodeScript with offThreadDecodeStencil. r?tcampbell!

• offThreadDecodeScript => offThreadDecodeStencil
• runOffThreadDecodedScript => finishOffThreadDecodeStencil+evalStencil

Depends on D133052

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 13: Remove JS::DecodeOffThreadScript. r?tcampbell!

Depends on D133053

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 14: Remove JSScript-related methods from ParseTask. r?tcampbell!

Depends on D133054

Comment 24

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 15: Use consistent name for decode multi stencils API. r?tcampbell!

• DecodeMultiOffThreadStencils => DecodeMultiStencilsOffThread
  • FinishMultiOffThreadStencilDecoder => FinishDecodeMultiStencilsOffThread
  • CancelMultiOffThreadScriptsDecoder => CancelDecodeMultiStencilsOffThread

Also make JS::DecodeMultiStencilsOffThread to receive JS::DecodeOptions,
and make ScriptPreloader to call JS::CanDecodeOffThread.

Depends on D133055

Comment 25

[Tooru Fujisawa [:arai]]

Attachment: Bug 1734098 - Part 16: Use CompilationStencil in helper threads. r?tcampbell!

Depends on D133056

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/48aa5ca15135
Part 1: Use consistent function name for off-thread stencil API. r=nbp
https://hg.mozilla.org/integration/autoland/rev/532328aedaaa
Part 2: Add JS::StartIncrementalEncoding. r=nbp
https://hg.mozilla.org/integration/autoland/rev/c3b9c8221059
Part 3: Use JS::CompileToStencilOffThread instead of JS::CompileOffThread in ScriptLoader. r=nbp
https://hg.mozilla.org/integration/autoland/rev/cfb19671fd4e
Part 4: Remove stencil option mismatch check in testing function. r=nbp
https://hg.mozilla.org/integration/autoland/rev/3174f1ecfd03
Part 5: Remove shell offThreadCompileScript. r=nbp
https://hg.mozilla.org/integration/autoland/rev/ffd72fa3fc59
Part 6: Remove JS::CompileOffThread. r=nbp
https://hg.mozilla.org/integration/autoland/rev/0d07931e6bc8
Part 7: Add JS::CompileModuleToStencilOffThread. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/a85846f79eb5
Part 8: Replace shell offThreadCompileModule with offThreadCompileModuleToStencil. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/cf458ad5345a
Part 9: Remove JS::CompileOffThreadModule. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/323af9d3e0e1
Part 10: Fix storageType for decoded stencil without borrowBuffer. r=nbp
https://hg.mozilla.org/integration/autoland/rev/da83b4571cf2
Part 11: Add JS::DecodeStencilOffThread. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/630055366dac
Part 12: Replace shell offThreadDecodeScript with offThreadDecodeStencil. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/7b7c59fc644b
Part 13: Remove JS::DecodeOffThreadScript. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/02d7ca7fc45c
Part 14: Remove JSScript-related methods from ParseTask. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/cfdf7ed9a5a2
Part 15: Use consistent name for decode multi stencils API. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/08070838c7d2
Part 16: Use CompilationStencil in helper threads. r=tcampbell

Comment 27

[Iulian Moraru]

Backed out (bug 1744178, bug 1734098) for causing leaks.

Affected platform Linux 18.04 x64 WebRender asan opt

Push with failures

Failure log for wd leaks
Failure log for bc leaks
Other jobs have the same failure log as bc leaks.

Backout link

Also this V-swr bustages, failure log: https://treeherder.mozilla.org/logviewer?job_id=360715600&repo=autoland&lineNumber=71123
and
this py3 mch failures, failure log: https://treeherder.mozilla.org/logviewer?job_id=360720852&repo=autoland&lineNumber=181

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ce44063a7b80
Part 1: Use consistent function name for off-thread stencil API. r=nbp
https://hg.mozilla.org/integration/autoland/rev/03f97dcd2bc2
Part 2: Add JS::StartIncrementalEncoding. r=nbp
https://hg.mozilla.org/integration/autoland/rev/cd08744153c1
Part 3: Use JS::CompileToStencilOffThread instead of JS::CompileOffThread in ScriptLoader. r=nbp
https://hg.mozilla.org/integration/autoland/rev/8c30710801e8
Part 4: Remove stencil option mismatch check in testing function. r=nbp
https://hg.mozilla.org/integration/autoland/rev/2e8c1adde5bb
Part 5: Remove shell offThreadCompileScript. r=nbp
https://hg.mozilla.org/integration/autoland/rev/fdbb03b6d16d
Part 6: Remove JS::CompileOffThread. r=nbp
https://hg.mozilla.org/integration/autoland/rev/34f47b8fad24
Part 7: Add JS::CompileModuleToStencilOffThread. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/b5604dcc16a4
Part 8: Replace shell offThreadCompileModule with offThreadCompileModuleToStencil. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/0fb4e685ee60
Part 9: Remove JS::CompileOffThreadModule. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/46ffc06461cc
Part 10: Fix storageType for decoded stencil without borrowBuffer. r=nbp
https://hg.mozilla.org/integration/autoland/rev/e05713ddea2b
Part 11: Add JS::DecodeStencilOffThread. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/d35d239bf1cf
Part 12: Replace shell offThreadDecodeScript with offThreadDecodeStencil. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/4d15a17ce581
Part 13: Remove JS::DecodeOffThreadScript. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/8de1c9e14cf3
Part 14: Remove JSScript-related methods from ParseTask. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/1c2988344c44
Part 15: Use consistent name for decode multi stencils API. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/1a07d4426d65
Part 16: Use CompilationStencil in helper threads. r=tcampbell

Comment 29

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/ce44063a7b80
https://hg.mozilla.org/mozilla-central/rev/03f97dcd2bc2
https://hg.mozilla.org/mozilla-central/rev/cd08744153c1
https://hg.mozilla.org/mozilla-central/rev/8c30710801e8
https://hg.mozilla.org/mozilla-central/rev/2e8c1adde5bb
https://hg.mozilla.org/mozilla-central/rev/fdbb03b6d16d
https://hg.mozilla.org/mozilla-central/rev/34f47b8fad24
https://hg.mozilla.org/mozilla-central/rev/b5604dcc16a4
https://hg.mozilla.org/mozilla-central/rev/0fb4e685ee60
https://hg.mozilla.org/mozilla-central/rev/46ffc06461cc
https://hg.mozilla.org/mozilla-central/rev/e05713ddea2b
https://hg.mozilla.org/mozilla-central/rev/d35d239bf1cf
https://hg.mozilla.org/mozilla-central/rev/4d15a17ce581
https://hg.mozilla.org/mozilla-central/rev/8de1c9e14cf3
https://hg.mozilla.org/mozilla-central/rev/1c2988344c44
https://hg.mozilla.org/mozilla-central/rev/1a07d4426d65