w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
319 stars 55 forks source link

Early design review: scheduler.yield() #827

Closed shaseley closed 11 months ago

shaseley commented 1 year ago

こんにちは TAG-さん!

I'm requesting a TAG review of scheduler.yield().

scheduler.yield() is an API for yielding control to the browser's event loop, which can be used to break up long tasks. Awaiting the promise returned by scheduler.yield() causes the current task to yield, continuing in a new browser task. This can be used to improve responsiveness issues caused by long tasks. Continuations are prioritized to mitigate performance problems of existing alternatives.

Further details:

We'd prefer the TAG provide feedback as (please delete all but the desired option):

🐛 open issues in our GitHub repo for each point of feedback

hober commented 1 year ago

Hi,

@plinss and I took a look at this during our Tokyo F2F this week. Overall, we like the idea of a simple API a developer can use to yield back to the event loop mid-computation. Here are a few specific things we noticed while reviewing the explainer, and one general worry:

A general concern: In isolation, this seems like a very sensible API. But it's part of a much larger project and it's unclear to us if there has been much external review (especially from other implementors) of the Scheduler API project as a whole. Relatedly, this builds on some other API (e.g. TaskController) that are only implemented in Blink. It often worries us when new technology is built on top of other technology that does not yet have multi-implementor buy-in, or better yet, multiple shipping implementations.

Anyway, this strikes us as a simple and ergonomic way for developers to do something quite common. Please ask us to review the "bigger picture" of the Scheduler API, and we'd love to take a look at scheduler.yield() again should it (or its dependencies) significantly change. Thanks!

plinss commented 11 months ago

We're going to close this issue, as Tess asked above, please let us know when the "bigger picture" Scheduler API is ready for review.

shaseley commented 11 months ago

Thanks Peter and Tess for the feedback! I've been actively working on the "bigger picture" explainer -- hoping to have it in a shareable state within ~the next month.

shaseley commented 1 month ago

Hi again,

Just to update here as well: I've drafted a "bigger picture" explainer, and I'll file a new issue for that and a spec review for scheduler.yield() now that the spec has been drafted (no major changes since this).

  • @plinss noticed the "signal" option name might confuse developers because it seems inconsistent with the "signal" option of the Fetch API. Maybe renaming it to "task signal" or something like that would help?

They are related: TaskSignal inherits from AbortSignal, and the signal option in yield() is an AbortSignal. So you can use either a TaskSignal or AbortSignal as the signal argument to yield(), as well as fetch() and postTask(). While it doesn't use it yet, I mentioned in the meta explainer that it could be useful to use the priority bit from the TaskSignal in fetch(), if provided.

Also, note that this mirror's scheduler.postTask()'s signal option, which also went through TAG review: https://github.com/w3ctag/design-reviews/issues/647.

  • Browser engines have very different implementations of the event loop and associated concepts; are the parameters in the options bag hints that the engine can ignore if necessary, or are they required to be respected?

Good question; I think there are several aspects to this:

  1. Aborting. The signal option can be used to abort tasks and continuations, and that seems to me like something that should be compatible regardless of event loop implementation.
  2. Prioritization (consistency within the scheduler API): There is a well-defined ordering of tasks scheduled with the scheduler APIs (postTask and yield). I'd be reluctant to say this can be ignored, as it completely changes developer expectations (e.g. running background tasks earlier than user-blocking tasks would probably be bad).
  3. Prioritization (wrt to other task sources). This is where I think having different event loop implementations is most relevant. While the intent is that "user-blocking" tasks are prioritized within the event loop and "background" tasks are deprioritized, we left this purposely vague since (a) different UAs have different prioritization schemes / event loop capabilities, and (b) the spec already allows UAs to prioritize between various task sources as they see fit. So in this way, I think priority can be seen as a hint for relative ordering with non-scheduler tasks. But part of the benefit here is the event loop integration (see the note in this section), e.g. yielding in a way that doesn't penalize the yielding task with the overhead of running all other queued tasks before continuing.
  4. Inheritance. yield() has an "inherit" option, which enables a "yieldy async task" (a conceptual task spread out amongst multiple HTML tasks) to inherit the original scheduling context (i.e. task signal). Ideally this wouldn't be ignored, but I'm not sure if event loop implementations limit this or not.

Also, if the options bag can be ignored, would it be possible to polyfill this using a noop async function?

Do you mean something like const yieldPolyfill() = async () => Promise.resolve();? The problem with that is that it doesn't actually yield to the event loop, so there's no performance benefit over synchronous code (both can lead to poor responsiveness). But interestingly we've seen some cases where having prioritized continuations (i.e. high priority in the event loop) matters to the point where some developers are likely to use this as a polyfill if yield() isn't available.

But I'd expect a browser implementation to at least be self-consistent in terms of prioritization (ordering guarantees between yield() and postTask() tasks), support aborting, and truly yield to the event loop. I put together a polyfill having these properties -- but it doesn't implement inheritance, which requires hacking around async/await.