1147048 - Use off thread JS parsing when loading <script defer> scripts from HTML documents

Status: RESOLVED DUPLICATE of bug 1154987

(Core :: JavaScript Engine, defect)

Description

[David Bruant]

Follow up to https://bugzilla.mozilla.org/show_bug.cgi?id=906371#c25

As a developer, I use @defer much more often than I use @async (because of script ordering)
Although there exists ways to engineer a JS application to load initially just what is needed and load the rest afterward, there is rarely time dedicated for this, so the initial load of my application very often are ~1Mb scripts (bundled with browserify). For 1Mb, I'm sure off-thread parsing would offer some benefit.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This will take some restructuring of the scriptloader, unlike async scripts.

The reason is that the way async parsing is hooked up right now is that at the point when we'd go to run the script we try to start an async parse (if allowed).  If that fails (most notably because ther JS engine is only willing to do some script parses offthread, we go ahead and do a sync parse and execute.

Not only that, but the JS heuristic for whether it can do an async parse assumes the alternative is a sync parse, not waiting.

But for defer scripts, the calculus is different.  There the alternative to an async parse is waiting, not a sync parse.  So we'd want to kick off the async parse once the data is there, not when we want to run the script (not that for an async script these are the same point in time, so there is no problem).  And we may need to adjust JS::CanCompileOffThread to understand the different tradeoff here, or simply set forceAsync in the compile options for the defer case....

Comment 2

[Andrew McCreight [:mccr8]]

(The context here is that Chrome recently added off main thread parsing, and they support both async and deferred scripts, according to this blog post: http://blog.chromium.org/2015/03/new-javascript-techniques-for-rapid.html )

Comment 3

[Boris Zbarsky [:bzbarsky]]

To be clear, I think we should do it; it's just a bit more work than just changing an "isAsync" check to "isAsyncOrDefer" check.

Comment 4

[Ting-Yu Chou [:ting] (away)]

I'd like to try this. Will start from reading the code of loading async scripts.

Comment 5

[Ting-Yu Chou [:ting] (away)]

Just traced the code, my rough plan is:

- Modify ProcessPendingRequests() and AttemptAsyncScriptParse() so if any of mDeferRequests has finished loading, call ProcessRequest() on it to initiate off main thread parsing.
- Maybe a different callback for JS::CompileOffThread() or modify NotifyOffThreadScriptLoadCompletedRunnable so the script won't be evaluated after parsing.
- Will need another flag in the request to indicate the script is under parsing, so the deferred request will be evaluated only when it is not loading and parsing.

Comment 6

[Ting-Yu Chou [:ting] (away)]

Attachment: wip

Comment 7

[Boris Zbarsky [:bzbarsky]]

Hmm.  Please coordinate with Kannan, who is in the middle of trying to rearchitect the scriptloader pretty comprehensively to allow streaming parsing of JS...

The attached patch does look like it would, generally speaking, work, but it would cause us to walk the mDeferRequests list a lot for no good reason if they're all kicked off-thread already but the document is not done loading.

Comment 8

[Ting-Yu Chou [:ting] (away)]

(In reply to Not doing reviews right now from comment #7)
> Hmm.  Please coordinate with Kannan, who is in the middle of trying to
> rearchitect the scriptloader pretty comprehensively to allow streaming
> parsing of JS...

Thanks for telling me this.

Kannan, please let me know whether your work does include defer script async parsing. I found a bug 274651 of streaming parsing, but seems not the one you're working on.

> The attached patch does look like it would, generally speaking, work, but it
> would cause us to walk the mDeferRequests list a lot for no good reason if
> they're all kicked off-thread already but the document is not done loading.

Yes, I know, will address that.

Comment 9

[Ting-Yu Chou [:ting] (away)]

There's one thing I noticed on FxOS today: if the script is preloaded (nsScriptLoader::PreloadURI()), OnStreamComplete() will come earlier than ProcessScriptElement() which the reqeust's mIsAsync and mIsDefer hasn't been set yet, and can't start async parsing right away.

Comment 10

[Ting-Yu Chou [:ting] (away)]

Attachment: wip v2

Moved AttemptAsyncScriptParse() from ProcessRequest() to OnStreamComplete(), so the script can be parsed off main thread once it is speculative loaded. Addressed the problem bz pointed out at comment 7.

The remaining works are: 1) comments, 2) decide whether to pass async/defer flags separately or not for PreloadURI()

I'll stop at here before Kannan responding to the NI.

Comment 11

[Kannan Vijayan [:djvj]]

(In reply to Ting-Yu Chou [:ting] from comment #8)
> Thanks for telling me this.
> 
> Kannan, please let me know whether your work does include defer script async
> parsing. I found a bug 274651 of streaming parsing, but seems not the one
> you're working on.

I'm trying to approach this from the angle of moving all parsing off-thread, including sync scripts.  I'm posting my patches on bug 1154987.  It would be great to work together on this.

So far, the patches I have adds a new 'incremental' stream loader which talks to an incremental stream load handler.  The handler incrementally decodes the data and keeps a charbuffer around.

This incrementally decoded charbuffer is not yet consumed by anybody (it just sits on the side).  Ultimately, I think the way background parsing will work for sync scripts is to always have a thread waiting for new sync scripts to parse, and parsing them in turn.  When the parse is done, the sync scripts are executed in order, using the parsed data.

This requires modification to both the way the browser dispatches top-level js execution, as well as how the parser receives incoming data (it'll need to potentially parse from a growing char buffer which may be reallocated and move, instead of a fixed buffer of chars that have been fully decoded).

Comment 12

[Ting-Yu Chou [:ting] (away)]

(In reply to Kannan Vijayan [:djvj] from comment #11)
> I'm trying to approach this from the angle of moving all parsing off-thread,
> including sync scripts.  I'm posting my patches on bug 1154987.  It would be
> great to work together on this.

Sure, please let me know if there's anything I can help.