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

Scheduler microtasks support #304

Merged
merged 3 commits into from
Sep 23, 2021

Conversation

kolodziejczak-sz
Copy link
Contributor

No description provided.

@yisar
Copy link
Collaborator

yisar commented Sep 23, 2021

Great pr, I like it very much, which means we can use microtask as the default option in the future, thank you! Let me merge it, and then I may refactor the code here, maybe the message channel is no longer needed, using setTimeout as a backup should be enough.

@yisar yisar merged commit 2730e42 into frejs:master Sep 23, 2021
@yisar yisar changed the title Scheduler macrotasks support Scheduler microtasks support Sep 23, 2021
@@ -44,10 +56,10 @@ const flush = (): void => {
}

export const shouldYield = (): boolean => {
if (options.sync) return false
return (
const isInputPending =
(navigator as any)?.scheduling?.isInputPending() || getTime() >= deadline
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've run some benchmarks and it seems that chrome is actually doing worse with isInputPending:

Firefox create 1k rows (20x): ~573ms
Chrome (without isInputPending) create 1k rows (20x) : ~642ms
Chrome (with isInputPending) create 1k rows (20x) : ~730ms

It would be nice if someone could confirm this as well;
Another case with it is that isInputPending function call creates ugly code that all other browsers needs to handle:

"use strict";
var _a, _b;
const isInputPending = ((_b = (_a = navigator) === null || _a === void 0 ? void 0 : _a.scheduling) === null || _b === void 0 ? void 0 : _b.isInputPending()) || getTime() >= deadline;

Copy link
Collaborator

Choose a reason for hiding this comment

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

In fact, there is no need to isInputPending, and the calculation using time slice is enough. You can delete it.
https://github.com/WICG/scheduling-apis
This is a new set of scheduling APIs, but it is not yet mature.
We can remove them first and then add it when the time is ripe.

@kolodziejczak-sz
Copy link
Contributor Author

Great pr, I like it very much, which means we can use microtask as the default option in the future, thank you! Let me merge it, and then I may refactor the code here, maybe the message channel is no longer needed, using setTimeout as a backup should be enough.

https://youtu.be/8eHInw9_U8k?t=494
"MessageChannel is more reliable way with no timeout clamping or anything to interfere to schedule a task"

@yisar
Copy link
Collaborator

yisar commented Sep 23, 2021

"MessageChannel is more reliable way with no timeout clamping or anything to interfere to schedule a task"

Although setTimeout has 4ms delay, it doesn't matter whether the time slice is 5ms or 9ms (4ms comes from the delay of setTimeout). Both of them can keep the responsive.

So in fact, setTimeout is enough.

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.

2 participants