Editorial: Explicitly track async evaluation order of pending modules #3353
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolo-ribaudo
changed the title
nicolo-ribaudo
force-pushed
the
async-evaluation-order
branch
from
493ab39
to
d600852
Compare
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.
nicolo-ribaudo
force-pushed
the
async-evaluation-order
branch
from
d600852
to
f96a304
Compare
syg
reviewed
Jun 21, 2024
| @@ -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> | |||
There was a problem hiding this comment.
Suggested change
| <td>[[AsyncEvaluationCount]]</td> | |
| <td>[[ModuleAsyncEvaluationCount]]</td> |
| @@ -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> | |||
There was a problem hiding this comment.
Suggested change
| <h1>GetModuleAsyncEvaluationCount ( ): an integer</h1> | |
| <h1>IncrementModuleAsyncEvaluationCount ( ): an integer</h1> |
| </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. |
There was a problem hiding this comment.
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.
| 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. |
There was a problem hiding this comment.
If you want to make this assert more direct, you could split the value domain into integers, ~unset~, and ~done~.
| @@ -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> | |||
There was a problem hiding this comment.
Please also say the value for B.[[AsyncEvaluationorder]].
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
You can verify the numbers in the updated tables for the example using https://nicolo-ribaudo.github.io/es-module-evaluation/.
Fixes #3289