(Translated by https://www.hiragana.jp/)
Add scheduler.yield() by brendankenny · Pull Request #35960 · mdn/content · GitHub
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

Add scheduler.yield() #35960

Merged
merged 12 commits into from
Sep 25, 2024
Merged

Add scheduler.yield() #35960

merged 12 commits into from
Sep 25, 2024

Conversation

brendankenny
Copy link
Contributor

Description

Chrome 129 adds yield method to the Scheduler interface. This PR adds reference documentation for the new method.

Additional details

@brendankenny brendankenny requested a review from a team as a code owner September 19, 2024 16:45
@brendankenny brendankenny requested review from wbamberg and removed request for a team September 19, 2024 16:45
@github-actions github-actions bot added Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed labels Sep 19, 2024
Copy link
Contributor

github-actions bot commented Sep 19, 2024

Copy link
Contributor

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Overall LGTM.

Have a few (non-blocking) comments where it could potentially be improved further.

files/en-us/web/api/scheduler/yield/index.md Outdated Show resolved Hide resolved

By default, `scheduler.yield()` is run with a [`'user-visible'`](/en-US/docs/Web/API/Prioritized_Task_Scheduling_API#user-visible) priority. However, the continuation from a `scheduler.yield()` has a slightly different behavior than `scheduler.postTask()` tasks of the same `priority`.

`scheduler.yield()` enqueues its task at the front of a priority level's queue, while `scheduler.postTask()` tasks go at the end (in the spec, this is defined by a task queue's [effective priority](https://wicg.github.io/scheduling-apis/#scheduler-task-queue-effective-priority)). This means that with the same priority, the `scheduler.yield()` continuation will come first, allowing additional flexibility in how tasks can be scheduled.
Copy link
Contributor

Choose a reason for hiding this comment

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

Whhhattt! This is news to me!

Even more reason to use yield over postTask then!

Copy link
Contributor

@malchata malchata left a comment

Choose a reason for hiding this comment

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

I think this generally looks fine, but I added a couple of suggestions for prose.

files/en-us/web/api/scheduler/yield/index.md Outdated Show resolved Hide resolved

### Basic usage

Long tasks can be broken up by awaiting `scheduler.yield()`. The function returns a promise, yielding the {{Glossary('main thread')}} to allow the browser to execute other pending work—like responding to user input—if needed.
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this glossary definition should be linked much earlier in the guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha good call. I think it might have migrated down as the article evolved :) Moved to first instance of "main thread".

files/en-us/web/api/scheduler/yield/index.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@brendankenny brendankenny left a comment

Choose a reason for hiding this comment

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

also clarified the priority inheritance to be the best kind of correct, and added mention of requestIdleCallback priority inheritance


### Basic usage

Long tasks can be broken up by awaiting `scheduler.yield()`. The function returns a promise, yielding the {{Glossary('main thread')}} to allow the browser to execute other pending work—like responding to user input—if needed.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha good call. I think it might have migrated down as the article evolved :) Moved to first instance of "main thread".

files/en-us/web/api/scheduler/yield/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/scheduler/yield/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/scheduler/yield/index.md Show resolved Hide resolved

### Inheriting task priorities

A `scheduler.yield()` within a `scheduler.postTask()` task will inherit the task's priority. For example, work after a `scheduler.yield()` within a [`'background'`](/en-US/docs/Web/API/Prioritized_Task_Scheduling_API#user-blocking) low-priority task will also be scheduled as `'background'` by default (but, again, inserted in the boosted `'background'` priority queue, so running before any `'background'` `postTask` tasks).

Choose a reason for hiding this comment

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

I'm not familiar with the MDN style guide, but I noticed that you have single quotes around the string 'background' here but then use double quotes in the code snippet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Damn you prettier. Switching to double quotes for everything 🤮

@brendankenny
Copy link
Contributor Author

@chrisdavidmills I think this is ready for you

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

@brendankenny nice work! I've reviewed this pretty thoroughly and left quite a few comments, but most of them are fairly minor language suggestions/nitpicks.

files/en-us/web/api/scheduler/yield/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/scheduler/yield/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/window/scheduler/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/workerglobalscope/scheduler/index.md Outdated Show resolved Hide resolved
chrisdavidmills and others added 2 commits September 25, 2024 13:19
Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
Co-authored-by: Joshua Chen <sidachen2003@gmail.com>
brendankenny and others added 2 commits September 25, 2024 10:28
Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
@brendankenny
Copy link
Contributor Author

Thanks for the great review @chrisdavidmills! I believe everything should be addressed.

Copy link
Contributor

@chrisdavidmills chrisdavidmills left a comment

Choose a reason for hiding this comment

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

Nice work @brendankenny. Let's get this published, and we can iterate further if needed.

@chrisdavidmills chrisdavidmills merged commit 3c359c6 into mdn:main Sep 25, 2024
8 checks passed
@brendankenny brendankenny deleted the scheduler.yield branch September 25, 2024 16:32
OnkarRuikar pushed a commit to OnkarRuikar/content that referenced this pull request Sep 28, 2024
* Add scheduler.yield()

* Apply suggestions from code review

Co-authored-by: Jeremy Wagner <malchata@users.noreply.github.com>

* review feedback

* clarify priority queues and requestIdleCallback

* fix one more reference to front of the queue behavior

* more review feedback

* Apply suggestions from code review

Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
Co-authored-by: Joshua Chen <sidachen2003@gmail.com>

* Apply suggestions from code review

Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>

* heading changes

* syntax consistency in headings

---------

Co-authored-by: Jeremy Wagner <malchata@users.noreply.github.com>
Co-authored-by: Chris Mills <chrisdavidmills@gmail.com>
Co-authored-by: Joshua Chen <sidachen2003@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:WebAPI Web API docs size/m [PR only] 51-500 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants