-
Notifications
You must be signed in to change notification settings - Fork 312
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
Introduce a parallel queue for running Jobs #1229
base: main
Are you sure you want to change the base?
Conversation
This change defines a parallel queue called the service worker manager where the instances of Run Job steps are queued and run in order. Fixes #1224.
|
||
A user agent has an associated <dfn export id="dfn-service-worker-manager">service worker manager</dfn>. | ||
|
||
A user agent *must* [=start a new parallel queue=] when it boots up and set the [=service worker manager=] to the result value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be scoped to the whole user agent, or is per-origin enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, totally happy for this to merge if there's a reason the queue needs to be across the whole browser.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This intentionally suggests to be a parallel execution context across the user agent. This matches pretty much what the implementations do (at least Chromium). We can think of the parallel execution context of the parallel queue as a thread in a browser process in Chromium for instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason the registration of a service worker on origin A should block the registration of a service worker on origin B?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No. They are independent. Per-origin parallel queues would work and provide better concurrency conceptually. In this change, I just tried to match the current implementation, especially Chromium.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the spec should allow for maximum concurrency while maintaining the behaviour we want. Implementations are welcome to be less concurrent, but the spec shouldn't reflect this unless developers come to rely on a particular lack of concurrency.
@annevk is that fair?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For shared workers I went with a note after requiring a single one for the user agent:
Each user agent has a single shared worker manager for simplicity. Implementations could use one per origin; that would not be observably different and enables more concurrency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implementers' feedback would also be great.
/cc @mattto, @wanderview, @mkruisselbrink, @aliams, @hober
|
||
Note: For a register job and an update job, the user agent delays queuing a task for running the job until after a {{Document/DOMContentLoaded}} event has been dispatched to the document that initiated the job. | ||
|
||
1. Else if |job|'s [=job type=] is *unregister*, run [=Unregister=] with |job| [=in parallel=]. | ||
1. Else if |job|'s [=job type=] is *unregister*, run [=Unregister=] with |job|. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good step. Eventually I'd like to get rid of the "job" concept entirely, and replace it with appending steps to the appropriate parallel queue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That'd mean the implementations should create and maintain a separate thread for each scope. That may be an ideal design which allows parallel register/update/unregister even, but a single (off-the-main) thread executing the jobs from all the job queues is what we currently have. I also would like to hear what implementers think on this.
One thing I'd thought of and didn't put in the change was run the Schedule Job's steps in the parallel queue as well. That'd make it more congruent to what Chromium does (with IPC). But I thought it would add somewhat unnecessary complexity to the spec. For now, I think using a separate (off-the-main) thread running jobs without breaking the order in each job queue would be good enough. |
This change defines a parallel queue called the service worker manager
where the instances of Run Job steps are queued and run in order.
Fixes #1224.
Preview | Diff