Open Bug 931852 Opened 11 years ago Updated 2 years ago

Use off-main-thread parsing for setTimout

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

People

(Reporter: luke, Unassigned)

References

Details

There are currently two cases I know of that seem to never get off-main-thread parsing:
 - inline script async (i.e., <script async>blah</script>)
 - scripted script async (i.e., s = document.createElement('script'); s.innerHTML = "blah"; document.body.appendChild(s))

I don't know if these are both the same root cause or different.  In particular, the latter case is the only way I know of to get the effect of "eval async".  We actually have a user who intends to do this so that they can store their (quite large) scripts in IDB and only send deltas over the wire when there are updates (which is apparently quite frequent).  (Alternatively, does anyone know of a different way to eval code that does hit the off-main-thread path?)
Whiteboard: [games]
> scripted script async (i.e., s = document.createElement('script'); s.innerHTML = "blah";
> document.body.appendChild(s)

I would think this would work.  In particular, the nsIScriptElement ctor does:

36       mForceAsync(aFromParser == mozilla::dom::NOT_FROM_PARSER ||
37                   aFromParser == mozilla::dom::FROM_PARSER_FRAGMENT),

so forthe <script> tag in this case HTMLScriptElement::Async returns true if mForceAsync, so when nsScriptElement::MaybeProcessScript calls FreezeUriAsyncDefer() we'll set mAsync to true, as far as I can tell.  And then GetScriptAsync() should return true in the scriptloader, so we should try to do an off-main-thread compile...  Let me see why this doesn't work.

The inline async case is explicitly excluded in the scriptloader:

769   if (!aRequest->mElement->GetScriptAsync() || aRequest->mIsInline) {

but we could probably just remove that second check, I expect.
So the reason the "scripted script async" case doesn't get off-thread parsing is that it's an inline script, and FreezeUriAsyncDefer will only set mAsync based on mForceAsync if there is an src.

And the reason for _that_ is that this case is not in fact async, because inline scripts always run sync on insertion.  Try this simple testcase:

<body>
  <script>
    var result = "";
    var s = document.createElement("script");
    s.textContent = "result += 'b'"
    document.body.appendChild(s);
    result += 'a';
    window.onload = function() { alert(result); }
  </script>

and note that it shows "ba".

Whoever thinks the code there is giving "the effect of eval async" is confused....
(In reply to Boris Zbarsky [:bz] from comment #2)
Well, right now they are literally using 'eval', and I was wanting to make a recommendation, hence me experimenting and this bug.

So, stepping back, if you have a string of code in JS, is there any way you can think of to execute it such that it'll have async semantics?
You mean async script semantics, or just running async?  Some possible thoughts:

1)  setTimeout(string, 0);
2)  var s = document.createElement("script"); s.src = "data:application/javascript,"+string;
    document.body.appendChild(s);

I think doing s.async = true is ignored on inline scripts.  :(
(In reply to Boris Zbarsky [:bz] from comment #4)
(2) seems to require I encodeURI which isn't great for perf if this is a 30MB string.

setTimeout is promising and seems like it'd generally be a win for the web.  How hard would it be to use off-main-thread parsing for setTimeout?

Another question: does the spec explicitly say to ignore 'async' on inline scripts?  That seems pretty goofy.
> How hard would it be to use off-main-thread parsing for setTimeout?

The hardest part, presumably, is that you have to block when the timer fires if the off-main-thread parse is not done yet.

And have to make sure to not report errors if the timeout is canceled and whatnot...

> does the spec explicitly say to ignore 'async' on inline scripts?

Yes.  Furthermore, an HTML document which has the "async" attribute on a script with "src" is non-conforming.

This might be worth raising as a spec issue.  The intent is that "async" defers execution until the script is "available" but it's talking about availability of the script text (which is always right there for inline scripts), whereas in our case that precedes the availability of something you can run by a possibly significant amount.
(In reply to Boris Zbarsky [:bz] from comment #6)
> This might be worth raising as a spec issue.  The intent is that "async"
> defers execution until the script is "available" but it's talking about
> availability of the script text (which is always right there for inline
> scripts), whereas in our case that precedes the availability of something
> you can run by a possibly significant amount.

Indeed!  I guess one objection people might have changing this is that code could already be depending on the sychronicity already.  Do you think this is likely?  Seems like if someone wanted sync execution, they have eval().
There's totally code depending on inline <script> (with no attributes) insertion being sync.

Whether there's code that does an inline script with @async and then inserts it is the relevant compat question....
(In reply to Boris Zbarsky [:bz] from comment #8)
> There's totally code depending on inline <script> (with no attributes)
> insertion being sync.

Yeah, I knew that :)  I was asking about scripted creation of a script element, setting textContent, then adding it and expecting that to be executed synchronously.
> I was asking about scripted creation of a script element, setting textContent, then
> adding it and expecting that to be executed synchronously.

Yes, that's the case I'm talking about when I say there's code depending on that behavior.
(In reply to Boris Zbarsky [:bz] from comment #10)
Oh, bummer.  So I guess setTimeout is the only reasonable hope for off-main-thread async eval?
How about creating a script element and settings its src to an object url that contains the source code?
> So I guess setTimeout is the only reasonable hope for off-main-thread async eval?

Again, I think we should consider (and raise as a spec issue) honoring the "async" attribute on inline scripts as a way to opt into async handling.
(In reply to Boris Zbarsky [:bz] from comment #13)
What scripted-created script objects?  I thought 'async' was 'true' by default and honoring this would break code expecting synchronous execution.
> I thought 'async' was 'true' by default

Sort of.

It's true by default for non-script created scripts... but is then ignored for inline ones.  But we could make an explicit setting to true (via .async or the attribute) override the ignoring.
Ok, makes sense.
(In reply to Alon Zakai (:azakai) from comment #12)
I just tested and this seems to compile async just fine.  I guess the only downside here is (I assume) the extra copy of the string into the Blob.  Is there any more copying when loading the script via the object URL?  On the bright side, you can iirc store a Blob in IDB and so in the case where you don't have to apply any diffs to the stored code, you can load the Blob directly w/o overhead.
Not sure if there is extra copying in blobs, but yeah, when you use the code as-is you can just load the blob directly, so that sounds reasonable.
Blocks: gecko-games
Depends on: 906371
Based on the fact that Blob + object URLs work great and bz explained why the two examples in comment 0 are not even async scripts, the only idea left in this bug is using async parsing for setTimeout.  This isn't a huge priority for games, though, so unblocking bug 710398.
No longer blocks: gecko-games
Summary: Use off-main-thread parsing for more cases of async scripts → Use off-main-thread parsing for setTimout
Whiteboard: [games]
(In reply to Luke Wagner [:luke] from comment #19)
> the only idea left in this bug is using async parsing for setTimeout.
Based on the fact it's something devs should stay away from, if that was just me, I'd WONTFIX and wait for someone to complain to reopen.
In the case where you have a dynamic string of code that you truly need to eval, setTimeout seems as fine a way to do it as any other and more direct than createElement('script')+Blob+getObjectURL.  But I do agree it's not a priority.
Component: DOM → DOM: Core & HTML
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.