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

[Scheduler] Always yield to native macro tasks when a virtual task completes #31787

Merged
merged 1 commit into from
Dec 17, 2024
+53 −11
Merged

[Scheduler] Always yield to native macro tasks when a virtual task completes #31787

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
2 changes: 2 additions & 0 deletions packages/scheduler/src/SchedulerFeatureFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@ export const userBlockingPriorityTimeout = 250;
export const normalPriorityTimeout = 5000;
export const lowPriorityTimeout = 10000;
export const enableRequestPaint = true;

export const enableAlwaysYieldScheduler = __EXPERIMENTAL__;
19 changes: 15 additions & 4 deletions packages/scheduler/src/__tests__/Scheduler-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,10 @@ describe('SchedulerBrowser', () => {
runtime.assertLog([
'Message Event',
'Task',
SchedulerFeatureFlags.enableRequestPaint
? 'Yield at 0ms'
: `Yield at ${SchedulerFeatureFlags.frameYieldMs}ms`,
gate(flags => flags.enableAlwaysYieldScheduler) ||
!SchedulerFeatureFlags.enableRequestPaint
? gate(flags => (flags.www ? 'Yield at 10ms' : 'Yield at 5ms'))
: 'Yield at 0ms',
'Post Message',
]);

Expand All @@ -220,7 +221,13 @@ describe('SchedulerBrowser', () => {
});
runtime.assertLog(['Post Message']);
runtime.fireMessageEvent();
runtime.assertLog(['Message Event', 'A', 'B']);
if (gate(flags => flags.enableAlwaysYieldScheduler)) {
runtime.assertLog(['Message Event', 'A', 'Post Message']);
runtime.fireMessageEvent();
runtime.assertLog(['Message Event', 'B']);
} else {
runtime.assertLog(['Message Event', 'A', 'B']);
}
});

it('multiple tasks with a yield in between', () => {
Expand Down Expand Up @@ -267,6 +274,10 @@ describe('SchedulerBrowser', () => {
runtime.assertLog(['Message Event', 'Oops!', 'Post Message']);

runtime.fireMessageEvent();
if (gate(flags => flags.enableAlwaysYieldScheduler)) {
runtime.assertLog(['Message Event', 'Post Message']);
runtime.fireMessageEvent();
}
runtime.assertLog(['Message Event', 'Yay']);
});

Expand Down
24 changes: 21 additions & 3 deletions packages/scheduler/src/__tests__/SchedulerSetImmediate-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,13 @@ describe('SchedulerDOMSetImmediate', () => {
});
runtime.assertLog(['Set Immediate']);
runtime.fireSetImmediate();
runtime.assertLog(['setImmediate Callback', 'A', 'B']);
if (gate(flags => flags.enableAlwaysYieldScheduler)) {
runtime.assertLog(['setImmediate Callback', 'A', 'Set Immediate']);
runtime.fireSetImmediate();
runtime.assertLog(['setImmediate Callback', 'B']);
} else {
runtime.assertLog(['setImmediate Callback', 'A', 'B']);
}
});

it('multiple tasks at different priority', () => {
Expand All @@ -200,7 +206,13 @@ describe('SchedulerDOMSetImmediate', () => {
});
runtime.assertLog(['Set Immediate']);
runtime.fireSetImmediate();
runtime.assertLog(['setImmediate Callback', 'B', 'A']);
if (gate(flags => flags.enableAlwaysYieldScheduler)) {
runtime.assertLog(['setImmediate Callback', 'B', 'Set Immediate']);
runtime.fireSetImmediate();
runtime.assertLog(['setImmediate Callback', 'A']);
} else {
runtime.assertLog(['setImmediate Callback', 'B', 'A']);
}
});

it('multiple tasks with a yield in between', () => {
Expand Down Expand Up @@ -246,7 +258,13 @@ describe('SchedulerDOMSetImmediate', () => {
runtime.assertLog(['setImmediate Callback', 'Oops!', 'Set Immediate']);

runtime.fireSetImmediate();
runtime.assertLog(['setImmediate Callback', 'Yay']);
if (gate(flags => flags.enableAlwaysYieldScheduler)) {
runtime.assertLog(['setImmediate Callback', 'Set Immediate']);
runtime.fireSetImmediate();
runtime.assertLog(['setImmediate Callback', 'Yay']);
} else {
runtime.assertLog(['setImmediate Callback', 'Yay']);
}
});

it('schedule new task after queue has emptied', () => {
Expand Down
17 changes: 13 additions & 4 deletions packages/scheduler/src/forks/Scheduler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
lowPriorityTimeout,
normalPriorityTimeout,
enableRequestPaint,
enableAlwaysYieldScheduler,
} from '../SchedulerFeatureFlags';

import {push, pop, peek} from '../SchedulerMinHeap';
Expand Down Expand Up @@ -196,9 +197,11 @@ function workLoop(initialTime: number) {
currentTask !== null &&
!(enableSchedulerDebugging && isSchedulerPaused)
) {
if (currentTask.expirationTime > currentTime && shouldYieldToHost()) {
// This currentTask hasn't expired, and we've reached the deadline.
break;
if (!enableAlwaysYieldScheduler) {
if (currentTask.expirationTime > currentTime && shouldYieldToHost()) {
// This currentTask hasn't expired, and we've reached the deadline.
break;
}
}
// $FlowFixMe[incompatible-use] found when upgrading Flow
const callback = currentTask.callback;
Expand Down Expand Up @@ -242,6 +245,12 @@ function workLoop(initialTime: number) {
pop(taskQueue);
}
currentTask = peek(taskQueue);
if (enableAlwaysYieldScheduler) {
if (currentTask === null || currentTask.expirationTime > currentTime) {
// This currentTask hasn't expired we yield to the browser task.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can't really rely on this once we switch to postTask. React already doesn't rely on this because of batching + Suspense, we have our own expiration times that switch to using the microtask queue once the time has elapsed.

So this is really just for anything at Meta that relies on the starvation protection. Maybe once this lands, that could be its own flag, too, and then Meta can try rolling it out to see what happens.

break;
}
}
}
// Return whether there's additional work
if (currentTask !== null) {
Expand Down Expand Up @@ -459,7 +468,7 @@ let frameInterval = frameYieldMs;
let startTime = -1;

function shouldYieldToHost(): boolean {
if (enableRequestPaint && needsPaint) {
if (!enableAlwaysYieldScheduler && enableRequestPaint && needsPaint) {
// Yield now.
return true;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/scheduler/src/forks/SchedulerFeatureFlags.www.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ export const frameYieldMs = 10;
export const userBlockingPriorityTimeout = 250;
export const normalPriorityTimeout = 5000;
export const lowPriorityTimeout = 10000;

export const enableAlwaysYieldScheduler = false;
Loading