Editorial: Explicitly track async evaluation order of pending modules #3353

nicolo-ribaudo · 2024-06-14T21:57:09Z

The execution order of modules that were waiting on a given async module was based on the order in which their [[AsyncEvaluation]] field was set to true.

This PR replaces the [[AsyncEvaluation]] boolean with an actual number that keeps track of ordering in each agent. It also adds a note on when it's safe to reset this order to 0, since:

implementations might want to prevent this number from overflowing, and V8 currently has a bug that resets the number to 0 too early
figuring out when it's safe to reset the number might not be trivial, given the complexity of the module algorithms.

You can verify the numbers in the updated tables for the example using https://nicolo-ribaudo.github.io/es-module-evaluation/.

Fixes #3289

The execution order of modules that were waiting on a given async module was based on the order in which their [[AsyncEvaluation]] field was set to *true*. This PR replaces the [[AsyncEvaluation]] boolean with an actual number that keeps track of ordering in each agent. It also adds a note on when it's safe to reset this order to 0, since: - implementations might want to prevent this number from overflowing, and V8 currently has a bug that resets the number to 0 too early - figuring out when it's safe to reset the number might not be trivial, given the complexity of the module algorithms. I currently updated the example tables to just say that [[AsyncEvaluationOrder]] is set to "an integer" rather than *true*. In the next few days I'll update them to show the actual integer, but I first need to update https://nicolo-ribaudo.github.io/es-module-evaluation/ to verify that I'm not getting it wrong.

syg

This is great, thanks for making this clearer.

As you may have seen in #3356, there's a bug in this machinery. The fix is a one liner so we should fix that bug, and you'll need to rebase this on top of the fix.

syg · 2024-06-21T21:01:43Z

spec.html

@@ -12158,6 +12158,11 @@ <h1>Agents</h1>
          <td>a List of either Objects or Symbols</td>
          <td>Initially a new empty List, representing the list of objects and/or symbols to be kept alive until the end of the current Job</td>
        </tr>
+        <tr>
+          <td>[[AsyncEvaluationCount]]</td>


Suggested change

<td>[[AsyncEvaluationCount]]</td>

<td>[[ModuleAsyncEvaluationCount]]</td>

syg · 2024-06-21T21:26:16Z

spec.html

@@ -12199,6 +12204,21 @@ <h1>AgentCanSuspend ( ): a Boolean</h1>
        <p>In some environments it may not be reasonable for a given agent to suspend. For example, in a web browser environment, it may be reasonable to disallow suspending a document's main event handling thread, while still allowing workers' event handling threads to suspend.</p>
      </emu-note>
    </emu-clause>
+
+    <emu-clause id="sec-getmoduleasyncevaluationcount" type="abstract operation">
+      <h1>GetModuleAsyncEvaluationCount ( ): an integer</h1>


Suggested change

<h1>GetModuleAsyncEvaluationCount ( ): an integer</h1>

<h1>IncrementModuleAsyncEvaluationCount ( ): an integer</h1>

syg · 2024-06-21T21:38:14Z

spec.html

              </td>
              <td>
-                Whether this module is either itself asynchronous or has an asynchronous dependency. Note: The order in which this field is set is used to order queued executions, see <emu-xref href="#sec-async-module-execution-fulfilled"></emu-xref>.
+                If this module is asynchronous or has an asynchronous dependency, this is a number representing the order in which modules that are asynchronous or have an asynchronous dependency start waiting on a pending promise.


I don't find the "start waiting on a pending promise" description helpful, since the sorted list from GatherAvailableAncestors is most directly used to call out to ExecuteModule.

syg · 2024-06-21T21:40:21Z

spec.html

-                1. Assert: _module_.[[AsyncEvaluation]] is *false* and was never previously set to *true*.
-                1. Set _module_.[[AsyncEvaluation]] to *true*.
-                1. NOTE: The order in which module records have their [[AsyncEvaluation]] fields transition to *true* is significant. (See <emu-xref href="#sec-async-module-execution-fulfilled"></emu-xref>.)
+                1. Assert: _module_.[[AsyncEvaluationOrder]] is ~unset~ and was never previously set to an integer.


If you want to make this assert more direct, you could split the value domain into integers, ~unset~, and ~done~.

syg · 2024-06-21T21:42:27Z

spec.html

@@ -26990,7 +27009,7 @@ <h1>Example Cyclic Module Record Graphs</h1>
          </emu-figure>
          <p>Loading and linking happen as before, and all modules end up with [[Status]] set to ~linked~.</p>

-          <p>Calling _A_.Evaluate() calls InnerModuleEvaluation on _A_, _B_, and _D_, which all transition to ~evaluating~. Then InnerModuleEvaluation is called on _A_ again, which is a no-op because it is already ~evaluating~. At this point, _D_.[[PendingAsyncDependencies]] is 0, so ExecuteAsyncModule(_D_) is called and we call _D_.ExecuteModule with a new PromiseCapability tracking the asynchronous execution of _D_. We unwind back to the InnerModuleEvaluation on _B_, setting _B_.[[PendingAsyncDependencies]] to 1 and _B_.[[AsyncEvaluation]] to *true*. We unwind back to the original InnerModuleEvaluation on _A_, setting _A_.[[PendingAsyncDependencies]] to 1. In the next iteration of the loop over _A_'s dependencies, we call InnerModuleEvaluation on _C_ and thus on _D_ (again a no-op) and _E_. As _E_ has no dependencies and is not part of a cycle, we call ExecuteAsyncModule(_E_) in the same manner as _D_ and _E_ is immediately removed from the stack. We unwind once more to the original InnerModuleEvaluation on _A_, setting _C_.[[AsyncEvaluation]] to *true*. Now we finish the loop over _A_'s dependencies, set _A_.[[AsyncEvaluation]] to *true*, and remove the entire strongly connected component from the stack, transitioning all of the modules to ~evaluating-async~ at once. At this point, the fields of the modules are as given in <emu-xref href="#table-module-graph-cycle-async-fields-1"></emu-xref>.</p>
+          <p>Calling _A_.Evaluate() calls InnerModuleEvaluation on _A_, _B_, and _D_, which all transition to ~evaluating~. Then InnerModuleEvaluation is called on _A_ again, which is a no-op because it is already ~evaluating~. At this point, _D_.[[PendingAsyncDependencies]] is 0, so ExecuteAsyncModule(_D_) is called and we call _D_.ExecuteModule with a new PromiseCapability tracking the asynchronous execution of _D_. We unwind back to the InnerModuleEvaluation on _B_, setting _B_.[[PendingAsyncDependencies]] to 1 and _B_.[[AsyncEvaluationOrder]]. We unwind back to the original InnerModuleEvaluation on _A_, setting _A_.[[PendingAsyncDependencies]] to 1. In the next iteration of the loop over _A_'s dependencies, we call InnerModuleEvaluation on _C_ and thus on _D_ (again a no-op) and _E_. As _E_ has no dependencies and is not part of a cycle, we call ExecuteAsyncModule(_E_) in the same manner as _D_ and _E_ is immediately removed from the stack. We unwind once more to the original InnerModuleEvaluation on _A_, setting _C_. Now we finish the loop over _A_'s dependencies, set _A_.[[AsyncEvaluationOrder]], and remove the entire strongly connected component from the stack, transitioning all of the modules to ~evaluating-async~ at once. At this point, the fields of the modules are as given in <emu-xref href="#table-module-graph-cycle-async-fields-1"></emu-xref>.</p>


Please also say the value for B.[[AsyncEvaluationorder]].

nicolo-ribaudo changed the title ~~Explicitly track async evaluation order of pending modules~~ Editorial: Explicitly track async evaluation order of pending modules Jun 14, 2024

nicolo-ribaudo force-pushed the async-evaluation-order branch from 493ab39 to d600852 Compare June 14, 2024 22:00

nicolo-ribaudo force-pushed the async-evaluation-order branch from d600852 to f96a304 Compare June 14, 2024 22:20

michaelficarra requested a review from syg June 17, 2024 17:47

syg added the editorial change label Jun 20, 2024

syg reviewed Jun 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editorial: Explicitly track async evaluation order of pending modules #3353

Editorial: Explicitly track async evaluation order of pending modules #3353

nicolo-ribaudo commented Jun 14, 2024 •

edited

Loading

syg left a comment

syg Jun 21, 2024

syg Jun 21, 2024

syg Jun 21, 2024

syg Jun 21, 2024

syg Jun 21, 2024

	<td>[[AsyncEvaluationCount]]</td>
	<td>[[ModuleAsyncEvaluationCount]]</td>

	<h1>GetModuleAsyncEvaluationCount ( ): an integer</h1>
	<h1>IncrementModuleAsyncEvaluationCount ( ): an integer</h1>

Editorial: Explicitly track async evaluation order of pending modules #3353

Are you sure you want to change the base?

Editorial: Explicitly track async evaluation order of pending modules #3353

Conversation

nicolo-ribaudo commented Jun 14, 2024 • edited Loading

syg left a comment

Choose a reason for hiding this comment

syg Jun 21, 2024

Choose a reason for hiding this comment

syg Jun 21, 2024

Choose a reason for hiding this comment

syg Jun 21, 2024

Choose a reason for hiding this comment

syg Jun 21, 2024

Choose a reason for hiding this comment

syg Jun 21, 2024

Choose a reason for hiding this comment

nicolo-ribaudo commented Jun 14, 2024 •

edited

Loading