1380218 - use HTTP OMT data delivery while loading JS script.

Status: NEW

(Core :: DOM: Core & HTML, enhancement, P3)

Description

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

ScriptLoader uses nsIncrementalStreamLoader to read script file from channel [1], however it doesn't utilize the OMT ODA mechanism (i.e. doesn't call nsIThreadRetargetableRequest::RetargetDeliveryTo in OnStartRequest [2]).

Bug 1154987 made JS parsing OMT and recent PBg-Http work made HTTP ODA OMT available in content process.

We should be able to make the both JS loading and parsing OMT.

[1] https://searchfox.org/mozilla-central/rev/5dadcbe55b4ddd1e448c06c77390ff6483aa009b/dom/script/ScriptLoader.cpp#1032
[2] https://searchfox.org/mozilla-central/rev/31311070d9860b24fe4a7a36976c14b328c16208/netwerk/base/nsIThreadRetargetableRequest.idl#34

Comment 1

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

To enable ODA OMT, nsIncrementalStreamLoader needs to invoke RetargetDeliveryTo (with event target provided) in OnStartRequest, see example in nsHtml5StreamParser.
https://searchfox.org/mozilla-central/rev/5dadcbe55b4ddd1e448c06c77390ff6483aa009b/parser/html/nsHtml5StreamParser.cpp#973

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

Just to clarify, you are talking about the window script loader, not worker script loader, right?

Comment 3

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Ben Kelly [reviewing, but slowly][:bkelly] from comment #2)
> Just to clarify, you are talking about the window script loader, not worker
> script loader, right?

Yes I mean window script loader. I'm aware that dom/worker/ScriptLoader is using RetargetDeliveryTo already.
https://searchfox.org/mozilla-central/rev/cef8389c687203085dc6b52de2fbd0260d7495bf/dom/workers/ScriptLoader.cpp#1740

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #3)
> Yes I mean window script loader. I'm aware that dom/worker/ScriptLoader is
> using RetargetDeliveryTo already.
> https://searchfox.org/mozilla-central/rev/
> cef8389c687203085dc6b52de2fbd0260d7495bf/dom/workers/ScriptLoader.cpp#1740

Ha!  I wasn't aware we did that. :-)  I was thinking it was something we should fix in the future.

Comment 5

[Nicolas B. Pierron [:nbp]]

(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #0)
> […], however it doesn't utilize the OMT ODA mechanism (i.e. doesn't call
> nsIThreadRetargetableRequest::RetargetDeliveryTo in OnStartRequest [2]).

What does ODA means?

Comment 6

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

(In reply to Nicolas B. Pierron [:nbp] from comment #5)
> (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment
> #0)
> > […], however it doesn't utilize the OMT ODA mechanism (i.e. doesn't call
> > nsIThreadRetargetableRequest::RetargetDeliveryTo in OnStartRequest [2]).
> 
> What does ODA means?
OnDataAvailable, which is the async callback for nsIChannel data delivery. nsIncrementalStreamLoader handle this callback and translate to nsIIncrementalStreamLoaderObserver callback, which ScriptLoader used.

Comment 7

[Greg Mierzwinski [:sparky]]

:jesup, could you elaborate on why the performance impact was set to ? ? Is there a blocker bug that could provide more context?

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

This would move JS loading off MainThread (OMT), which should have a performance impact (either on pageload or responsiveness, or both), though it's uncertain if it will be measurable.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

It is very much unclear how much this affects page load.
Depending on the guess the calculator gives estimate "performance impact none" or "performance impact high".
We do already speculative loading and then omt parsing, so adding omt loading might or might not be significant. Of course the loading itself is happening in another process, this would be just how the data is consumed in a content process.
And since OnStopRequest needs to happen on the main thread, we still need to hop into that thread before doing omt parsing.
This might become more effective once OnStopRequest can happen omt.

I think we should do some more investigation here, and then re-evaluate the impact.
jesup, could you perhaps provide some profiles which hint at least a bit how much this could improve page load?
Or ask if denis has any guesses?

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

I'll look into this as part of the Necko OMT effort currently starting. We do plan to move OnStartRequest and OnStopRequest to OMT where possible, which may help here.