Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove compound microtasks #4437

Merged
merged 4 commits into from
Mar 29, 2019
Merged

Remove compound microtasks #4437

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0991cc
Remove compound microtasks
annevk Mar 19, 2019
be0ea32
spin the event loop can hit microtasks, but only one level of nesting…
annevk Mar 20, 2019
ac6bbc0
oops
annevk Mar 20, 2019
2a3120a
move part of note, turn remainder into an assert
annevk Mar 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 14 additions & 53 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89978,17 +89978,7 @@ dictionary <dfn>PromiseRejectionEventInit</dfn> : <span>EventInit</span> {

<p>Each <span>event loop</span> has a <dfn>microtask queue</dfn>. A <dfn
data-export="">microtask</dfn> is a <span data-x="concept-task">task</span> that is originally to
be queued on the <span>microtask queue</span> rather than a <span>task queue</span>. There are two
kinds of <span data-x="microtask">microtasks</span>: <dfn data-x="solitary callback
microtask" data-lt="solitary callback microtask" data-export="">solitary callback
microtasks</dfn>, and <dfn data-x="compound microtask" data-lt="compound microtask"
data-export="">compound microtasks</dfn>.</p>

<p class="note">This specification only has <span data-x="solitary callback microtask">solitary
callback microtasks</span>. Specifications that use <span data-x="compound microtask">compound
microtasks</span> have to take extra care to <span data-x="execute a compound microtask
subtask">wrap callbacks</span> to handle <span data-x="spin the event loop">spinning the event
loop</span>.</p>
be queued on the <span>microtask queue</span> rather than a <span>task queue</span>.</p>

<p>When an algorithm requires a <span>microtask</span> to be <dfn data-x="queue a
microtask" data-lt="queue a microtask" data-export="">queued</dfn>, it must be appended to the
Expand Down Expand Up @@ -90043,28 +90033,6 @@ dictionary <dfn>PromiseRejectionEventInit</dfn> : <span>EventInit</span> {
<li><p>Set the <span>performing a microtask checkpoint</span> flag to false.</p></li>
</ol>

<p>If, while a <span>compound microtask</span> is running, the user agent is required to
<dfn data-export="">execute a compound microtask subtask</dfn> to run a series of steps, the user
agent must run the following steps:</p>

<ol>

<li><p>Let <var>parent</var> be the <span>event loop</span>'s <span>currently running
task</span> (the currently running <span>compound microtask</span>).</p></li>

<li><p>Let <var>subtask</var> be a new <span data-x="concept-task">task</span> that
consists of running the given series of steps. The <span>task source</span> of such a
<span>microtask</span> is the <span>microtask task source</span>. This is a <dfn>compound
microtask subtask</dfn>.</p></li>

<li><p>Set the <span>event loop</span>'s <span>currently running task</span> to <var>subtask</var>.</p></li>

<li><p>Run <var>subtask</var>.</p></li>

<li><p>Set the <span>event loop</span>'s <span>currently running task</span> back to <var>parent</var>.</p></li>

</ol>

<hr>

<p>When an algorithm running <span>in parallel</span> is to <dfn>await a stable state</dfn>, the
Expand All @@ -90089,19 +90057,11 @@ dictionary <dfn>PromiseRejectionEventInit</dfn> : <span>EventInit</span> {
<p>When an algorithm says to <dfn>spin the event loop</dfn> until a condition <var>goal</var> is met, the user agent must run the following steps:</p>

<ol>
<li><p>Let <var>task</var> be the <span>event loop</span>'s <span>currently running
task</span>.</p></li>

<li>

<p>Let <var>task</var> be the <span>event loop</span>'s <span>currently running
task</span>.</p>

<p class="note">This might be a <span>microtask</span>, in which case it is a <span>solitary
callback microtask</span>. It could also be a <span>compound microtask subtask</span>, or a
regular <span data-x="concept-task">task</span> that is not a <span>microtask</span>. It will
<em>not</em> be a <span>compound microtask</span>.</p> <!-- well, not unless we messed up in the
speccing, anyway... -->

</li>
<li><p>Assert: <var>task</var> is a <span data-x="concept-task">task</span> or a
<span>microtask</span>.</p></li>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make this say "This is a task or a microtask." I'm not convinced the remainder adds clarity. Thoughts? (We could perhaps Assert it instead.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the remainder adds some clarity. It could be rephrased as an assert but I'm fine with it as-is.

Editorial nit: "If this is a microtask, then the..."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the problem with the remainder is that (as happened in this thread) folks might assume the flag remains set.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I think assert is pretty clear that it's a point-in-time check, then. Or, you could add a note to the "Perform a microtask checkpoint" step which notes that it unsets the flag. (Or even an assert afterward that it's unset.)

Copy link
Member Author

@annevk annevk Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not where it gets unset, right? It gets unset after you return to the caller when you stop the microtask but resume the steps that invoked it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK here is what I would do. Remove the remainder from here. Instead, in the "Perform a microtask checkpoint" step, add a note specifically saying "This step will be a no-op if task is a microtask, since in that case the performing a microtask checkpoint flag will be set." That seems to capture the actually-interesting part of it being set.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I find "assert" a bit strong here since it's more of a reminder that microtasks are a type of task. But, I'm OK keeping it like this.

<li><p>Let <var>task source</var> be <var>task</var>'s <span>task
source</span>.</p></li>
Expand All @@ -90111,17 +90071,19 @@ dictionary <dfn>PromiseRejectionEventInit</dfn> : <span>EventInit</span> {

<li><p>Empty the <span>JavaScript execution context stack</span>.</p></li>

<li><p><span>Perform a microtask checkpoint</span>.</p></li>

<li>
<p><span>Perform a microtask checkpoint</span>.</p>

<p>Stop <var>task</var>, allowing whatever algorithm that invoked it to resume, but
continue these steps <span>in parallel</span>.</p>
<p class="note">If <var>task</var> is a <span>microtask</span> this step will be a no-op due to
the <span>performing a microtask checkpoint</span> flag being set.</p>
</li>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not worth fixing here, but I think indentation could help here (i.e. the modern style that does not "continue running these steps in parallel"). Plus perhaps a note about how this has such drastic effects on the caller. But maybe that's better addressed by just doing more refactorings like #4440.

<p class="note">This causes one of the following algorithms to continue: the <span>event
loop</span>'s main set of steps, the <span>perform a microtask checkpoint</span> algorithm, or
the <span>execute a compound microtask subtask</span> algorithm.</p>
<li>
<p>Stop <var>task</var>, allowing whatever algorithm that invoked it to resume, but continue
these steps <span>in parallel</span>.</p>

<p class="note">This causes the <span>event loop</span>'s main set of steps or the <span>perform
a microtask checkpoint</span> algorithm to continue.</p>
</li>

<li><p>Wait until the condition <var>goal</var> is met.</p></li>
Expand All @@ -90134,7 +90096,6 @@ dictionary <dfn>PromiseRejectionEventInit</dfn> : <span>EventInit</span> {
stack</var>.</p></li>

<li><p>Return to the caller.</p></li>

</ol>

<hr>
Expand Down