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

Define when and how regular tasks are processed wrt the processing model #2301

Merged
merged 3 commits into from Feb 19, 2021

Conversation

padenot
Copy link
Member

@padenot padenot commented Feb 11, 2021

A first cut for this, after reading lots of WHATWG text. Step 3.1 is necessary to avoid DOSing the render thread. I don't know if we want to be more precise for dispatching tasks, but considering the existence of an implied event loop, we're probably good.

This fixes #2008.


Preview | Diff

@padenot padenot requested a review from hoch February 11, 2021 19:49
Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

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

Thank you so much for working on this! 👍

A general comment: We have 1) the main thread task queue, 2) context's control message queue, and 3) render thread's message queue. Probably it's better to clarify them in this context.

L.10088 ~ L.10090 seems like a great place to add a section for this clarification.

index.bs Show resolved Hide resolved
index.bs Outdated
<a>rendering thread</a> has another task queue for microtasks for any microtask
operation such as resolution of {{Promise}}s in the {{AudioWorkletGlobalScope}}.
filling up the requested buffer size. Along with the primary message queue, each
{{AudioContext}} has a regular task queue for tasks that are posted to
Copy link
Member

Choose a reason for hiding this comment

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

Same: What are primary message queue and regular task queue here? We might want to clarify these terms.

Copy link
Member Author

Choose a reason for hiding this comment

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

The message queue is a queue of messages, and a task queue is a queue of tasks.

index.bs Outdated
have been queued during the execution of the `process` methods of
{{AudioWorkletProcessor}}.

All tasks posted from an {{AudioWorkletNode}} are posted to this task queue.
Copy link
Member

Choose a reason for hiding this comment

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

this task queue means control message queue. Right?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, those are different.

index.bs Outdated Show resolved Hide resolved
index.bs Outdated
@@ -11147,7 +11157,15 @@ operation such as resolution of {{Promise}}s in the {{AudioWorkletGlobalScope}}.

2. Remove the [=oldest message=] of <var>Q<sub>rendering</sub></var>.

3. Process a render quantum.

3. Process the regular event queue.
Copy link
Member

Choose a reason for hiding this comment

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

regular event queue - I think it's better to define 3 different types of queues somewhere. Also it seems like we are using different terms interchangeably. (e.g. task queue, message queue, and event queue)

Let's make a section and stick to it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. The terms are not used interchangeably however.

index.bs Outdated Show resolved Hide resolved
1. Let <var>task count</var> be the number of tasks in the task queue of
the task queue of the {{AudioContext}} being rendered.

2. Spin the event loop until <var>task count</var> tasks have been executed.
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need task count? Can we just say:

"Run the event loop until all queued tasks in Q are executed."

(Q can be defined somewhere)

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 haven't found a way to define this more cleanly, this is unambiguous. In particular, I haven't found a way to define Q. I'm happy to hear any idea though.

Copy link
Member Author

@padenot padenot left a comment

Choose a reason for hiding this comment

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

Here is a new iteration. I've tried to use linking to make it clear. I've also fixed the typos and other issues.

index.bs Outdated
have been queued during the execution of the `process` methods of
{{AudioWorkletProcessor}}.

All tasks posted from an {{AudioWorkletNode}} are posted to this task queue.
Copy link
Member Author

Choose a reason for hiding this comment

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

No, those are different.

1. Let <var>task count</var> be the number of tasks in the task queue of
the task queue of the {{AudioContext}} being rendered.

2. Spin the event loop until <var>task count</var> tasks have been executed.
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 haven't found a way to define this more cleanly, this is unambiguous. In particular, I haven't found a way to define Q. I'm happy to hear any idea though.

index.bs Show resolved Hide resolved
index.bs Outdated
<a>rendering thread</a> has another task queue for microtasks for any microtask
operation such as resolution of {{Promise}}s in the {{AudioWorkletGlobalScope}}.
filling up the requested buffer size. Along with the primary message queue, each
{{AudioContext}} has a regular task queue for tasks that are posted to
Copy link
Member Author

Choose a reason for hiding this comment

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

The message queue is a queue of messages, and a task queue is a queue of tasks.

index.bs Outdated
@@ -11147,7 +11157,15 @@ operation such as resolution of {{Promise}}s in the {{AudioWorkletGlobalScope}}.

2. Remove the [=oldest message=] of <var>Q<sub>rendering</sub></var>.

3. Process a render quantum.

3. Process the regular event queue.
Copy link
Member Author

Choose a reason for hiding this comment

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

Sure. The terms are not used interchangeably however.

Copy link
Member

@hoch hoch left a comment

Choose a reason for hiding this comment

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

Thanks! The new iteration looks good to me.

index.bs Outdated
The audio callback is also queued as a task in the
<a href="#control-message-queue">control message queue</a>. The UA MUST perform
The audio callback is also queued as a task in the <a
href="#control-message-queue">control message queue</a>. The UA MUST perform
Copy link
Member

Choose a reason for hiding this comment

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

nit: do we need to indent here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No idea. I'll check locally by running bikeshed and merge as is if needed, or fixed if not. Thanks for the quick re-review!

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

(bikeshed accepts it).

@padenot padenot merged commit bf8b6aa into main Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integration between event-loop of AudioWorkletGlobalScope and rendering loop
2 participants