1454352 - [BinAST] Expose the APIs necessary for <script> loading

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P2)

Description

[David Teller [:Yoric] - still alive but not very active]

List provided by jonco:

JS::Compile

JS::CompileForNonSyntacticScope

JS::CompileOffThread   

JS::FinishOffThreadScript

Comment 1

[David Teller [:Yoric] - still alive but not very active]

efaust, it looks to me like these functions fall in your lap, right?

Comment 2

[Eric Faust [:efaust]]

Yeah, that sounds reasonable. I have enough for JS::Compile already as part of my musings. Off-thread stuff should be pretty easy, though I haven't looked into it yet. I'm imagining (read: assuming.) that most of the stuff we want to use from frontend:: uses the appropriate main thread checks already.

Comment 3

[Jon Coppeard (:jonco)]

(In reply to David Teller [:Yoric] (please use "needinfo") from comment #0)
> JS::CompileForNonSyntacticScope

I don't think we need this one - although it's called by the generic nsJSUtils class it's not used by the script loader so we can just assert that this case doesn't arise.

Comment 4

[Eric Faust [:efaust]]

Attachment: offThreadParse.patch

This looks right to me, and would provide the other two, but it seems like it must be supposed to be more complicated. Does this look plausible, Jon?

Comment 5

[Jon Coppeard (:jonco)]

Comment on attachment 8972472 [details] [diff] [review]
offThreadParse.patch

Review of attachment 8972472 [details] [diff] [review]:
-----------------------------------------------------------------

Yes this looks fine.  I'm still working hooking up a test to use this path.  I'll let you know if things don't work out.

Can you add JS::CanDecodeBinASTOffThread() (along the same lines as the existing versions) and add a case for BinAST in CanDoOffThread()?

Comment 6

[Jon Coppeard (:jonco)]

(In reply to Jon Coppeard (:jonco) from comment #5)
Testing was successful.

Comment 7

[Eric Faust [:efaust]]

Attachment: update, apply on top of other patch.

Comment 8

[Jon Coppeard (:jonco)]

Attachment: off-thread-decode-binast

I combined the patches and rebased.

Comment 9

[Jon Coppeard (:jonco)]

Comment on attachment 8974653 [details] [diff] [review]
off-thread-decode-binast

Review of attachment 8974653 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

::: js/src/frontend/BytecodeCompiler.h
@@ +41,5 @@
>  JSScript*
>  CompileGlobalBinASTScript(JSContext *cx, LifoAlloc& alloc,
>                            const ReadOnlyCompileOptions& options,
> +                          const uint8_t* src, size_t len,
> +                          ScriptSourceObject** sourceObjectOut = nullptr);

The last parameter should probably be a |RootedScriptSourceObject*|.  If it wasn't optional you could use a MutableHandle, but I don't think we have a way of making that optional.

Comment 10

[Jon Coppeard (:jonco)]

(In reply to Jon Coppeard (:jonco) from comment #9)
> The last parameter should probably be a |RootedScriptSourceObject*|.  If it
> wasn't optional you could use a MutableHandle, but I don't think we have a
> way of making that optional.

Oh we already do this in a bunch of places.  Never mind, I'll file a separate bug.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91ced8101f99
Add APIs for parallel decoding of BinAST data r=jonco

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/91ced8101f99