Skip to content
This repository has been archived by the owner on Aug 29, 2021. It is now read-only.

Execute module graph synchronously when no top-level await is reached? #43

Closed
littledan opened this issue Feb 8, 2019 · 8 comments
Closed

Comments

@littledan
Copy link
Member

The approach in the draft spec text as well as #33 seems to be based around making a PromisePerformThen happen each time one module imports another. This is an observable change for unmodified programs: Consider the following module graph:

// b.mjs
console.log("1");
Promise.resolve().then(() => console.log("2"));

// a.mjs
import "./b.mjs";
console.log("3");
Promise.resolve().then(() => console.log("4"));

// file.html
<script type=module src="/a.mjs"></script>

Across WebKit, Gecko and Chromium, the console shows "1 3 2 4" as the ordering. However, with this change introducing these additional microtask queue items, the ordering may change to "1 2 3 4".

Stepping back, what this means is: Currently, a module can queue a microtask and be confident that this means, "My microtask will be called after all the modules start up, and before control returns to HTML". These top-level await drafts change that "after all the modules start up" part.

Part of this change is unavoidable: If a top-level await is reached, then we have to wait on it.

But, if no top-level await is reached, should we continue on with things synchronously, as happens today?

@ljharb
Copy link
Member

ljharb commented Feb 8, 2019

It seems pretty important to preserve the ordering of things like shims (or any other synchronous setup like opening a DB connection or initializing a namespace etc) to retain the current ordering in the absence of TLA.

@domenic
Copy link
Member

domenic commented Feb 8, 2019

We've always wanted, and planned for, the flexibility to change the ordering in this way in HTML, in order to break up the module graph into more chunks that can be executed without janking the page. We just haven't done it yet. So I am fine if TLA helps us get to that world faster. I.e. I think the semantic change is not a problem.

See https://docs.google.com/document/d/1MJK0zigKbH4WFCKcHsWwFAzpU_DZppEAOpYJlIW7M7E/edit. Although that document is mostly focused on refuting the ASAP strategy, it notes how we'd be happy to move from the spec's current super-deterministic strategy to a different deterministic-with-yielding strategy.

@zenparsing
Copy link
Member

But, if no top-level await is reached, should we continue on with things synchronously, as happens today?

In general there has been resistance to specifying algorithms in a way that conditionally yields control based on whether something is a promise or not.

@littledan
Copy link
Member Author

@ljharb Could you be more specific about the requirements? I am not sure if anyone is proposing something that would break those invariants.

@littledan
Copy link
Member Author

littledan commented Feb 8, 2019

Let's think through some use cases that relate to @ljharb 's concern.

  • In a shim which does not use top-level await, in a module graph which may or may not use top-level await, and there's an import of the shim near the root of the module graph before other modules are imported, then the evaluation of the shim will start and end before any other modules start executing. This is true whether we go with Variant A or Variant B.
  • In a similarly-situated shim which uses top-level await import() to pull in a module dependency (whether over the network or a built-in module), Variant A will wait for all that to complete before loading others (whether or not we do this extra microtask queue delay), whereas Variant B might have the answer in time if it's a built-in module and we go with this change (actually in your favor! but in a very fragile way that you probably shouldn't depend on), but otherwise would let other modules run before the await completes.
  • In a top-level await to open a database, this will be over the network or in some other way through a higher level task queue (after all microtasks are drained), so one turn through the microtask queue won't really affect anything either. (A vs B effects similar to the previous explanation.)

Overall, it's hard for me to see where this affects practical use cases of modules, whether not they have top-level await. I was more filing the issue to ask if the observable change of existing code is intended, but I really don't see a problem with it.

One reason I filed this issue is because @sokra raised "allow[ing] other microtasks" to intermix being a downside of a proposed semantics for WebAssembly ESM integration in webpack/webpack#8144 (comment) . But, that would be an effect of the Wasm semantics whether top-level await makes this change or not.

@ljharb
Copy link
Member

ljharb commented Feb 8, 2019

Given the first bullet point, maybe there's not a concern here with the ordering changing.

@littledan
Copy link
Member Author

Overall, if we want to make an observable (subtle) change, it is probably easier now than further out in the future. Let's rip the band-aid off now! All else being equal, I agree with @zenparsing about these design principles being important to follow.

littledan added a commit to littledan/proposal-top-level-await that referenced this issue Feb 22, 2019
littledan added a commit to littledan/proposal-top-level-await that referenced this issue Feb 22, 2019
littledan added a commit to littledan/proposal-top-level-await that referenced this issue Feb 22, 2019
littledan added a commit to littledan/proposal-top-level-await that referenced this issue Mar 1, 2019
littledan added a commit to littledan/proposal-top-level-await that referenced this issue Mar 1, 2019
littledan added a commit to littledan/proposal-top-level-await that referenced this issue Mar 19, 2019
This patch is a variant on tc39#49 which determines which module subgraphs
are to be executed synchronously based on syntax (whether the module
contains a top-level await syntactically) and the dependency graph
(whether it imports a module which contains a top-level await,
recursively). This fixed check is designed to be more predictable and
analyzable.

Abstract module record changes:
- The [[Async]] field stores whether this module or dependencies are
  async. It is expected to be initialized by the Linking phase.
- The [[ExecutionPromise]] field stores the Promise related to the
  evaluation of a module whose [[Async]] field is *true*.
- Evaluate() returns a Promise for [[Async]] modules, a completion
  record for sync modules which throw, or undefined otherwise.

Cyclic Module Record changes:
- A new [[ModuleAsync]] field stores whether this particular module
  is asynchronous (dependencies aside).
- The ExecuteModule method on Cyclic Module Records takes an
  argument for the Promise capability, but only if that particular
  module is [[ModuleAsync]].
- The Link/Instantiate phase is used to propagate the [[Async]]
  field up the module graph to dependencies.
- When there's a cycle, with some modules sync and some async, the
  whole cycle is considered async, with the Promise of each module
  set to the entrypoint of the cycle, although the cycle-closing
  edge will not actually be awaited (since this would be a deadlock).

Source Text Module Record changes:
- The check for whether a module contains a top-level await locally
  is in a ContainsAwait algorithm (TBD writing this out, but it
  should be static since await may not appear in a direct eval)
- Module execution works as before if ContainsAwait is false, and
  works like an async function if ContainsAwait is true.

Closes tc39#47, tc39#48, tc39#43
@littledan
Copy link
Member Author

The current semantics (#61) do execute a module graph synchronously when no module which contains a top-level await is part of the graph. This preserves current behavior on current code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants