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

Remove scheduler.yield() parameters #100

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

shaseley
Copy link
Collaborator

@shaseley shaseley commented Aug 1, 2024

By default, scheduler.yield() continuations inherit the priority and abort signal from the current task, but this behavior can be overridden by providing a signal or priority option. But it's not clear if this is necessary, and there are questions around the expectations of subsequent continuations (see #96).

To simplify the API, for now we're removing the yield() parameters, instead always using the inherited state. We can revisit this as use cases arise.


Preview | Diff

By default, scheduler.yield() continuations inherit the priority and
abort signal from the current task, but this behavior can be overridden
by providing a signal or priority option. But it's not clear if this is
necessary, and there are questions around the expectations of subsequent
continuations (see WICG#96).

To simplify the API, for now we're removing the yield() parameters,
instead always using the inherited state. We can revisit this as use
cases arise.
Copy link
Collaborator

@mmocny mmocny left a comment

Choose a reason for hiding this comment

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

Change is fine, but will repeat the question from other code review, here:

Is it still possible for "inherit" part of the scheduling options via the postTask API? Should it be?

I don't mind that you cannot control changes via yield() since I don't think you need continuation semantics if you are making changes to priority/signal. But it seems very likely useful to be able to only change one part of the options at a time-- and especially the ability to change priority without changing signal.


An alternative option would be for the developer to just keep a reference to the signal -- or if there was an API to expose a reference to the current signal.

Then, rather than adding the option to "inherit", you would just pass the same reference?

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 6, 2024
By default, scheduler.yield() continuations inherit the priority and
abort signal from the current task, but this behavior can be overridden
by providing a signal or priority option. But it's not clear if this is
necessary, and there are questions around the expectations of subsequent
continuations (see WICG/scheduling-apis#96).

So to simplify the API and prevent shipping behavior we might regret,
we're removing the yield() parameters. If control over a continuation's
priority/abort is needed, a new task can be scheduled using postTask().
We plan to reevaluate this as the API rolls out and developers start
using it in more complex ways.

Related PR: WICG/scheduling-apis#100.

Bug: 40633887
Change-Id: I5e23c9c8cf950cec795cddafd27c962b71eea5f7
@shaseley
Copy link
Collaborator Author

shaseley commented Aug 7, 2024

Change is fine, but will repeat the question from other code review, here:

Is it still possible for "inherit" part of the scheduling options via the postTask API? Should it be?

I don't mind that you cannot control changes via yield() since I don't think you need continuation semantics if you are making changes to priority/signal. But it seems very likely useful to be able to only change one part of the options at a time-- and especially the ability to change priority without changing signal.

An alternative option would be for the developer to just keep a reference to the signal -- or if there was an API to expose a reference to the current signal.

Then, rather than adding the option to "inherit", you would just pass the same reference?

Currently not without passing around a signal/priority. I think we could revisit scheduler.currentTaskSignal or add an inherit option if need be.

@shaseley shaseley linked an issue Aug 7, 2024 that may be closed by this pull request
@shaseley shaseley merged commit ab9bdc9 into WICG:main Aug 7, 2024
2 checks passed
aarongable pushed a commit to chromium/chromium that referenced this pull request Aug 8, 2024
By default, scheduler.yield() continuations inherit the priority and
abort signal from the current task, but this behavior can be overridden
by providing a signal or priority option. But it's not clear if this is
necessary, and there are questions around the expectations of subsequent
continuations (see WICG/scheduling-apis#96).

So to simplify the API and prevent shipping behavior we might regret,
we're removing the yield() parameters. If control over a continuation's
priority/abort is needed, a new task can be scheduled using postTask().
We plan to reevaluate this as the API rolls out and developers start
using it in more complex ways.

Related PR: WICG/scheduling-apis#100.

Bug: 40633887
Change-Id: I5e23c9c8cf950cec795cddafd27c962b71eea5f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5753325
Commit-Queue: Scott Haseley <[email protected]>
Reviewed-by: Michal Mocny <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1338805}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 8, 2024
By default, scheduler.yield() continuations inherit the priority and
abort signal from the current task, but this behavior can be overridden
by providing a signal or priority option. But it's not clear if this is
necessary, and there are questions around the expectations of subsequent
continuations (see WICG/scheduling-apis#96).

So to simplify the API and prevent shipping behavior we might regret,
we're removing the yield() parameters. If control over a continuation's
priority/abort is needed, a new task can be scheduled using postTask().
We plan to reevaluate this as the API rolls out and developers start
using it in more complex ways.

Related PR: WICG/scheduling-apis#100.

Bug: 40633887
Change-Id: I5e23c9c8cf950cec795cddafd27c962b71eea5f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5753325
Commit-Queue: Scott Haseley <[email protected]>
Reviewed-by: Michal Mocny <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1338805}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 8, 2024
By default, scheduler.yield() continuations inherit the priority and
abort signal from the current task, but this behavior can be overridden
by providing a signal or priority option. But it's not clear if this is
necessary, and there are questions around the expectations of subsequent
continuations (see WICG/scheduling-apis#96).

So to simplify the API and prevent shipping behavior we might regret,
we're removing the yield() parameters. If control over a continuation's
priority/abort is needed, a new task can be scheduled using postTask().
We plan to reevaluate this as the API rolls out and developers start
using it in more complex ways.

Related PR: WICG/scheduling-apis#100.

Bug: 40633887
Change-Id: I5e23c9c8cf950cec795cddafd27c962b71eea5f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5753325
Commit-Queue: Scott Haseley <[email protected]>
Reviewed-by: Michal Mocny <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1338805}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Aug 9, 2024
…) parameters, a=testonly

Automatic update from web-platform-tests
Scheduling APIs: Remove scheduler.yield() parameters

By default, scheduler.yield() continuations inherit the priority and
abort signal from the current task, but this behavior can be overridden
by providing a signal or priority option. But it's not clear if this is
necessary, and there are questions around the expectations of subsequent
continuations (see WICG/scheduling-apis#96).

So to simplify the API and prevent shipping behavior we might regret,
we're removing the yield() parameters. If control over a continuation's
priority/abort is needed, a new task can be scheduled using postTask().
We plan to reevaluate this as the API rolls out and developers start
using it in more complex ways.

Related PR: WICG/scheduling-apis#100.

Bug: 40633887
Change-Id: I5e23c9c8cf950cec795cddafd27c962b71eea5f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5753325
Commit-Queue: Scott Haseley <[email protected]>
Reviewed-by: Michal Mocny <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1338805}

--

wpt-commits: edcaaa246cdce621e114ef5bb963296457521bab
wpt-pr: 47481
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this pull request Aug 19, 2024
…) parameters, a=testonly

Automatic update from web-platform-tests
Scheduling APIs: Remove scheduler.yield() parameters

By default, scheduler.yield() continuations inherit the priority and
abort signal from the current task, but this behavior can be overridden
by providing a signal or priority option. But it's not clear if this is
necessary, and there are questions around the expectations of subsequent
continuations (see WICG/scheduling-apis#96).

So to simplify the API and prevent shipping behavior we might regret,
we're removing the yield() parameters. If control over a continuation's
priority/abort is needed, a new task can be scheduled using postTask().
We plan to reevaluate this as the API rolls out and developers start
using it in more complex ways.

Related PR: WICG/scheduling-apis#100.

Bug: 40633887
Change-Id: I5e23c9c8cf950cec795cddafd27c962b71eea5f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5753325
Commit-Queue: Scott Haseley <[email protected]>
Reviewed-by: Michal Mocny <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1338805}

--

wpt-commits: edcaaa246cdce621e114ef5bb963296457521bab
wpt-pr: 47481
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.

Should yield() with explicit priority/signal change the inherited state?
3 participants