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

Jump to bottom

Introduce a parallel queue for running Jobs #1229

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Introduce a parallel queue for running Jobs #1229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 13 additions & 5 deletions docs/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,14 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
Note: A user agent may use a separate task source for each functional event type in order to avoid a head-of-line blocking phenomenon for certain functional events.
</section>

<section>
<h3 id="user-agent-bootup">User Agent Boot up</h3>

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.
Copy link
Contributor

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?

Copy link
Contributor

@jakearchibald jakearchibald Nov 20, 2017 •
edited
Loading

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.

Copy link
Collaborator Author

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.

Copy link
Contributor

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?

Copy link
Collaborator Author

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.

Copy link
Contributor

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?

Copy link
Member

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.

Copy link
Collaborator Author

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

</section>

<section>
<h3 id="user-agent-shutdown">User Agent Shutdown</h3>

Expand Down Expand Up @@ -2223,15 +2231,15 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
: Output
:: none

1. Assert: |jobQueue| [=queue/is not empty=].
1. [=Queue a task=] to run these steps:
1. [=Enqueue the following steps=] to the [=service worker manager=]:
1. Assert: |jobQueue| [=queue/is not empty=].
1. Let |job| be the first [=queue/item=] in |jobQueue|.
1. If |job|'s [=job type=] is *register*, run [=Register=] with |job| [=in parallel=].
1. Else if |job|'s [=job type=] is *update*, run [=Update=] with |job| [=in parallel=].
1. If |job|'s [=job type=] is *register*, run [=Register=] with |job|.
1. Else if |job|'s [=job type=] is *update*, run [=Update=] with |job|.

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|.
Copy link
Contributor

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.

Copy link
Collaborator Author

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.

</section>

<section algorithm>
Expand Down
18 changes: 13 additions & 5 deletions docs/v1/index.bs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,14 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
Note: A user agent may use a separate task source for each functional event type in order to avoid a head-of-line blocking phenomenon for certain functional events.
</section>

<section>
<h3 id="user-agent-bootup">User Agent Boot up</h3>

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.
</section>

<section>
<h3 id="user-agent-shutdown">User Agent Shutdown</h3>

Expand Down Expand Up @@ -2143,15 +2151,15 @@ spec: webappsec-referrer-policy; urlPrefix: https://w3c.github.io/webappsec-refe
: Output
:: none

1. Assert: |jobQueue| [=queue/is not empty=].
1. [=Queue a task=] to run these steps:
1. [=Enqueue the following steps=] to the [=service worker manager=]:
1. Assert: |jobQueue| [=queue/is not empty=].
1. Let |job| be the first [=queue/item=] in |jobQueue|.
1. If |job|'s [=job type=] is *register*, run [=Register=] with |job| [=in parallel=].
1. Else if |job|'s [=job type=] is *update*, run [=Update=] with |job| [=in parallel=].
1. If |job|'s [=job type=] is *register*, run [=Register=] with |job|.
1. Else if |job|'s [=job type=] is *update*, run [=Update=] with |job|.

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|.
</section>

<section algorithm>
Expand Down