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.

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

Make task queue selection per agent rather than per scheduler #87

Merged
merged 1 commit into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
26 changes: 17 additions & 9 deletions spec/patches.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,33 @@ queue=].
With: For each [=event loop=], every [=task source=] that is not a [=scheduler task source=] must be
associated with a specific [=task queue=].

Add: An [=event loop=] has a numeric <dfn for="event loop">next enqueue order</dfn> which is
initialized to 1.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Given the note about timestamps as an alternative (below), would this value be entirely optional?

Similarly, in the scheduler task definition, there is an "enqueue order" which is specifically initialized to 0, and then algorithms specifying how to assign and increment -- all these need to be timestamps in that case I think.

While this alternative is likely easily implied by the note below, just checking if you should make explicit reference to all the changes that would be made with that alternative or not.

I wonder if that would be easier if you just have an algorithm for "get next enqueue order value" or something like that. It could be implemented to increase a counter, or to return a unique+increasing timestamp.

Then, perhaps the rest of the references would not care if its a timestamp or number.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, yeah it's optional if you choose to use timestamps. I chose to use an enqueue order since it's easy to understand and matches Chrome's implementation. I also removed the note about using a timestamp since I'm not sure how much that adds -- it's clear in the spec where the value is set, and implementations in general can make different choices as long as the observable behavior is the same.


Note: The [=event loop/next enqueue order=] is a strictly increasing number that is used to
determine task execution order across [=scheduler task queues=] of the same {{TaskPriority}} across
all {{Scheduler}}s associated with the same [=event loop=]. A timestamp would also suffice as long
as it is guaranteed to be strictly increasing and unique.

### <a href="https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model">Event loop: processing model</a> ### {#sec-patches-html-event-loop-processing}

Add the following steps to the event loop processing steps, before step 1:
Add the following steps to the event loop processing steps, before step 2:

1. Let |queues| be the [=set=] of the [=event loop=]'s [=task queues=] that contain at least one
[=task/runnable=] [=task=].
1. Let |schedulers| be the [=set=] of all {{Scheduler}} objects whose [=relevant agent's=]
[=agent/event loop=] is this event loop and that [=have a runnable task=].
1. If |schedulers| and |queues| are both [=list/empty=], skip to the <code>microtasks</code> step
below.
1. Let |schedulerQueue| be the result of [=selecting the next scheduler task queue from all
schedulers=].

Modify step 2 to read:

Modify step 1 to read:
1. If |schedulerQueue| is not null or |queues| is not [=list/empty=]:

Modify step 2.1 to read:

1. Let |taskQueue| be one of the following, chosen in an [=implementation-defined=] manner:
* If |queues| is not [=list/empty=], one of the [=task queues=] in |queues|, chosen in an
[=implementation-defined=] manner.
* If |schedulers| is not [=list/empty=], the result of [=selecting the task queue of the next
scheduler task=] from one of the {{Scheduler}}s in |schedulers|, chosen in an
[=implementation-defined=] manner.
* |schedulerQueue|'s [=scheduler task queue/tasks=], if |schedulerQueue| is not null.

Issue: The |taskQueue| in this step will either be a [=set=] of [=tasks=] or a [=set=] of
[=scheduler tasks=]. The steps that follow only [=set/remove=] an [=set/item=], so they are
Expand Down
44 changes: 20 additions & 24 deletions spec/scheduling-tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,8 @@ option that is null or is an {{AbortSignal}} &mdash; are placed in these queues,
An alternative, and logicially equivalent implementation, would be to maintain a single
per-{{TaskPriority}} [=scheduler task queue=], and move tasks between [=scheduler task queues=] in
response to a {{TaskSignal}}'s [=TaskSignal/priority=] changing, inserting based on [=scheduler
task/enqueue order=]. This approach would simplify [=selecting the task queue of the next scheduler
task=], but make priority changes more complex.


A {{Scheduler}} object has a numeric <dfn for="Scheduler">next enqueue order</dfn> which is
initialized to 1.

Note: The [=Scheduler/next enqueue order=] is a strictly increasing number that is used to determine
task execution order across [=scheduler task queues=] of the same {{TaskPriority}} within the same
{{Scheduler}}. A logically equivalent alternative would be to place the [=Scheduler/next enqueue
order=] on the [=event loop=], since the only requirements are that the number be strictly
increasing and not be repeated within a {{Scheduler}}.

Issue: Would it be simpler to just use a timestamp here?
task/enqueue order=]. This approach would simplify [=selecting the next scheduler task queue from
all schedulers=], but make priority changes more complex.

The <dfn method for=Scheduler title="postTask(callback, options)">postTask(|callback|, |options|)</dfn>
method steps are to return the result of [=scheduling a postTask task=] for [=this=] given
Expand Down Expand Up @@ -319,8 +307,9 @@ Issue: [=Run steps after a timeout=] doesn't necessarily account for suspension;
1. Let |global| be the [=relevant global object=] for |scheduler|.
1. Let |document| be |global|'s <a attribute for="Window">associated `Document`</a> if |global| is
a {{Window}} object; otherwise null.
1. Let |enqueue order| be |scheduler|'s [=Scheduler/next enqueue order=].
1. Increment |scheduler|'s [=Scheduler/next enqueue order=] by 1.
1. Let |event loop| be |scheduler|'s [=relevant agent's=] [=agent/event loop=].
1. Let |enqueue order| be |event loop|'s [=event loop/next enqueue order=].
1. Increment |event loop|'s [=event loop/next enqueue order=] by 1.
1. Set |handle|'s [=task handle/task=] to the result of [=queuing a scheduler task=] on |handle|'s
[=task handle/queue=] given |enqueue order|, the [=posted task task source=], and |document|,
and that performs the following steps:
Expand All @@ -329,7 +318,7 @@ Issue: [=Run steps after a timeout=] doesn't necessarily account for suspension;
1. Run |handle|'s [=task handle/task complete steps=].

Issue: Because this algorithm can be called from [=in parallel=] steps, parts of this and other
algorithms are racy. Specifically, the [=Scheduler/next enqueue order=] should be updated
algorithms are racy. Specifically, the [=event loop/next enqueue order=] should be updated
atomically, and accessing the [=scheduler task queues=] should occur atomically. The latter also
affects the event loop task queues (see [this issue](https://github.com/whatwg/html/issues/6475)).
</div>
Expand All @@ -355,21 +344,28 @@ Issue: [=Run steps after a timeout=] doesn't necessarily account for suspension;
</div>

<div algorithm>
The result of <dfn>selecting the task queue of the next scheduler task</dfn> for {{Scheduler}}
|scheduler| is a [=set=] of [=scheduler tasks=] as defined by the following steps:

1. Let |queues| be the result of [=getting the runnable task queues=] for |scheduler|.
To <dfn>select the next scheduler task queue from all schedulers</dfn> given an [=event loop=]
|event loop|, perform the following steps. They return a [=scheduler task queue=] or null if no
{{Scheduler}} associated with the |event loop| [=has a runnable task=].

1. Let |queues| be an empty [=set=].
1. Let |schedulers| be the [=set=] of all {{Scheduler}} objects whose [=relevant agent's=]
[=agent/event loop=] is |event loop| and that [=have a runnable task=].
1. For each |scheduler| in |schedulers|, [=list/extend=] |queues| with the result of [=getting the
runnable task queues=] for |scheduler|.
1. If |queues| is [=list/empty=] return null.
1. [=set/Remove=] from |queues| any |queue| such that |queue|'s [=scheduler task queue/priority=]
is [=TaskPriority/less than=] any other [=set/item=] of |queues|.
1. Let |queue| be the [=scheduler task queue=] in |queues| whose [=scheduler task queue/first
runnable task=] is the [=scheduler task/older than|oldest=].
<br/><span class=note>Two tasks cannot have the same age since [=scheduler task/enqueue order=]
is unique.</span>
1. Return |queue|'s [=scheduler task queue/tasks=].
1. Return |queue|.

Note: The next task to run is the oldest, highest priority [=task/runnable=] [=scheduler task=]
from all {{Scheduler}}s associated with the [=event loop=].
</div>

Note: The next task to run is the oldest, highest priority [=task/runnable=] [=scheduler task=].
</div>

## Examples ## {#sec-scheduling-tasks-examples}

Expand Down