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

Jump to bottom

clean up isInputPending in Scheduler #28444

Merged
merged 3 commits into from
Feb 27, 2024
Merged

clean up isInputPending in Scheduler #28444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ca65e1
clean up isInputPending
noahlemen Feb 26, 2024
721b335
make frameYieldMs flag static, set to 10ms in www
noahlemen Feb 26, 2024
18e220b
cleanup/fix tests
noahlemen Feb 26, 2024
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
4 changes: 0 additions & 4 deletions packages/scheduler/src/SchedulerFeatureFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,8 @@
*/

export const enableSchedulerDebugging = false;
export const enableIsInputPending = false;
export const enableProfiling = false;
export const enableIsInputPendingContinuous = false;
export const frameYieldMs = 5;
export const continuousYieldMs = 50;
export const maxYieldMs = 300;

export const userBlockingPriorityTimeout = 250;
export const normalPriorityTimeout = 5000;
Expand Down
177 changes: 1 addition & 176 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 @@ -101,23 +101,6 @@ describe('SchedulerBrowser', () => {
this.port2 = port2;
};

const scheduling = {
isInputPending(options) {
if (this !== scheduling) {
throw new Error(
'isInputPending called with incorrect `this` context',
);
}

return (
hasPendingDiscreteEvent ||
(options && options.includeContinuous && hasPendingContinuousEvent)
);
},
};

global.navigator = {scheduling};

function ensureLogIsEmpty() {
if (eventLog.length !== 0) {
throw Error('Log is not empty. Call assertLog before continuing.');
Expand Down Expand Up @@ -218,7 +201,7 @@ describe('SchedulerBrowser', () => {
runtime.assertLog([
'Message Event',
'Task',
'Yield at 5ms',
gate(flags => (flags.www ? 'Yield at 10ms' : 'Yield at 5ms')),
'Post Message',
]);

Expand Down Expand Up @@ -320,164 +303,6 @@ describe('SchedulerBrowser', () => {
runtime.assertLog(['Message Event', 'B']);
});

it('when isInputPending is available, we can wait longer before yielding', () => {
function blockUntilSchedulerAsksToYield() {
while (!Scheduler.unstable_shouldYield()) {
runtime.advanceTime(1);
}
runtime.log(`Yield at ${performance.now()}ms`);
}

// First show what happens when we don't request a paint
scheduleCallback(NormalPriority, () => {
runtime.log('Task with no pending input');
blockUntilSchedulerAsksToYield();
});
runtime.assertLog(['Post Message']);

runtime.fireMessageEvent();
runtime.assertLog([
'Message Event',
'Task with no pending input',
// Even though there's no input, eventually Scheduler will yield
// regardless in case there's a pending main thread task we don't know
// about, like a network event.
gate(flags =>
flags.enableIsInputPending
? 'Yield at 10ms'
: // When isInputPending is disabled, we always yield quickly
'Yield at 5ms',
),
]);

runtime.resetTime();

// Now do the same thing, but while the task is running, simulate an
// input event.
scheduleCallback(NormalPriority, () => {
runtime.log('Task with pending input');
runtime.scheduleDiscreteEvent();
blockUntilSchedulerAsksToYield();
});
runtime.assertLog(['Post Message']);

runtime.fireMessageEvent();
runtime.assertLog([
'Message Event',
'Task with pending input',
// This time we yielded quickly to unblock the discrete event.
'Yield at 5ms',
'Discrete Event',
]);
});

it(
'isInputPending will also check for continuous inputs, but after a ' +
'slightly larger threshold',
() => {
function blockUntilSchedulerAsksToYield() {
while (!Scheduler.unstable_shouldYield()) {
runtime.advanceTime(1);
}
runtime.log(`Yield at ${performance.now()}ms`);
}

// First show what happens when we don't request a paint
scheduleCallback(NormalPriority, () => {
runtime.log('Task with no pending input');
blockUntilSchedulerAsksToYield();
});
runtime.assertLog(['Post Message']);

runtime.fireMessageEvent();
runtime.assertLog([
'Message Event',
'Task with no pending input',
// Even though there's no input, eventually Scheduler will yield
// regardless in case there's a pending main thread task we don't know
// about, like a network event.
gate(flags =>
flags.enableIsInputPending
? 'Yield at 10ms'
: // When isInputPending is disabled, we always yield quickly
'Yield at 5ms',
),
]);

runtime.resetTime();

// Now do the same thing, but while the task is running, simulate a
// continuous input event.
scheduleCallback(NormalPriority, () => {
runtime.log('Task with continuous input');
runtime.scheduleContinuousEvent();
blockUntilSchedulerAsksToYield();
});
runtime.assertLog(['Post Message']);

runtime.fireMessageEvent();
runtime.assertLog([
'Message Event',
'Task with continuous input',
// This time we yielded quickly to unblock the continuous event. But not
// as quickly as for a discrete event.
gate(flags =>
flags.enableIsInputPending
? 'Yield at 10ms'
: // When isInputPending is disabled, we always yield quickly
'Yield at 5ms',
),
'Continuous Event',
]);
},
);

it('requestPaint forces a yield at the end of the next frame interval', () => {
function blockUntilSchedulerAsksToYield() {
while (!Scheduler.unstable_shouldYield()) {
runtime.advanceTime(1);
}
runtime.log(`Yield at ${performance.now()}ms`);
}

// First show what happens when we don't request a paint
scheduleCallback(NormalPriority, () => {
runtime.log('Task with no paint');
blockUntilSchedulerAsksToYield();
});
runtime.assertLog(['Post Message']);

runtime.fireMessageEvent();
runtime.assertLog([
'Message Event',
'Task with no paint',
gate(flags =>
flags.enableIsInputPending
? 'Yield at 10ms'
: // When isInputPending is disabled, we always yield quickly
'Yield at 5ms',
),
]);

runtime.resetTime();

// Now do the same thing, but call requestPaint inside the task
scheduleCallback(NormalPriority, () => {
runtime.log('Task with paint');
requestPaint();
blockUntilSchedulerAsksToYield();
});
runtime.assertLog(['Post Message']);

runtime.fireMessageEvent();
runtime.assertLog([
'Message Event',
'Task with paint',
// This time we yielded quickly (5ms) because we requested a paint.
'Yield at 5ms',
]);
});

it('yielding continues in a new task regardless of how much time is remaining', () => {
scheduleCallback(NormalPriority, () => {
runtime.log('Original Task');
Expand Down
2 changes: 1 addition & 1 deletion packages/scheduler/src/__tests__/SchedulerSetImmediate-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ describe('SchedulerDOMSetImmediate', () => {
runtime.assertLog([
'setImmediate Callback',
'Task',
'Yield at 5ms',
gate(flags => (flags.www ? 'Yield at 10ms' : 'Yield at 5ms')),
'Set Immediate',
]);

Expand Down
73 changes: 2 additions & 71 deletions packages/scheduler/src/forks/Scheduler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,7 @@ import type {PriorityLevel} from '../SchedulerPriorities';
import {
enableSchedulerDebugging,
enableProfiling,
enableIsInputPending,
enableIsInputPendingContinuous,
frameYieldMs,
continuousYieldMs,
maxYieldMs,
userBlockingPriorityTimeout,
lowPriorityTimeout,
normalPriorityTimeout,
Expand Down Expand Up @@ -104,17 +100,6 @@ const localClearTimeout =
const localSetImmediate =
typeof setImmediate !== 'undefined' ? setImmediate : null; // IE and Node.js + jsdom

const isInputPending =
typeof navigator !== 'undefined' &&
// $FlowFixMe[prop-missing]
navigator.scheduling !== undefined &&
// $FlowFixMe[incompatible-type]
navigator.scheduling.isInputPending !== undefined
? navigator.scheduling.isInputPending.bind(navigator.scheduling)
: null;

const continuousOptions = {includeContinuous: enableIsInputPendingContinuous};

function advanceTimers(currentTime: number) {
// Check for tasks that are no longer delayed and add them to the queue.
let timer = peek(timerQueue);
Expand Down Expand Up @@ -468,71 +453,20 @@ let taskTimeoutID: TimeoutID = (-1: any);
// It does not attempt to align with frame boundaries, since most tasks don't
// need to be frame aligned; for those that do, use requestAnimationFrame.
let frameInterval = frameYieldMs;
const continuousInputInterval = continuousYieldMs;
const maxInterval = maxYieldMs;
let startTime = -1;

let needsPaint = false;

function shouldYieldToHost(): boolean {
const timeElapsed = getCurrentTime() - startTime;
if (timeElapsed < frameInterval) {
// The main thread has only been blocked for a really short amount of time;
// smaller than a single frame. Don't yield yet.
return false;
}

// The main thread has been blocked for a non-negligible amount of time. We
// may want to yield control of the main thread, so the browser can perform
// high priority tasks. The main ones are painting and user input. If there's
// a pending paint or a pending input, then we should yield. But if there's
// neither, then we can yield less often while remaining responsive. We'll
// eventually yield regardless, since there could be a pending paint that
// wasn't accompanied by a call to `requestPaint`, or other main thread tasks
// like network events.
if (enableIsInputPending) {
if (needsPaint) {
// There's a pending paint (signaled by `requestPaint`). Yield now.
return true;
}
if (timeElapsed < continuousInputInterval) {
// We haven't blocked the thread for that long. Only yield if there's a
// pending discrete input (e.g. click). It's OK if there's pending
// continuous input (e.g. mouseover).
if (isInputPending !== null) {
return isInputPending();
}
} else if (timeElapsed < maxInterval) {
// Yield if there's either a pending discrete or continuous input.
if (isInputPending !== null) {
return isInputPending(continuousOptions);
}
} else {
// We've blocked the thread for a long time. Even if there's no pending
// input, there may be some other scheduled work that we don't know about,
// like a network event. Yield now.
return true;
}
}

// `isInputPending` isn't available. Yield now.
// Yield now.
return true;
}

function requestPaint() {
if (
enableIsInputPending &&
navigator !== undefined &&
// $FlowFixMe[prop-missing]
navigator.scheduling !== undefined &&
// $FlowFixMe[incompatible-type]
navigator.scheduling.isInputPending !== undefined
) {
needsPaint = true;
}

// Since we yield every frame regardless, `requestPaint` has no effect.
}
function requestPaint() {}
Copy link
Member Author

Choose a reason for hiding this comment

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

i expected to be able to clean this up too, but there are tests that rely on SchedulerMock's implementation of this


function forceFrameRate(fps: number) {
if (fps < 0 || fps > 125) {
Expand Down Expand Up @@ -577,9 +511,6 @@ const performWorkUntilDeadline = () => {
}
}
}
// Yielding to the browser will give it a chance to paint, so we can
// reset this.
needsPaint = false;
};

let schedulePerformWorkUntilDeadline;
Expand Down
6 changes: 0 additions & 6 deletions packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@

export const enableProfiling = __VARIANT__;

export const enableIsInputPending = __VARIANT__;
export const enableIsInputPendingContinuous = __VARIANT__;
export const frameYieldMs = 5;
export const continuousYieldMs = 10;
export const maxYieldMs = 10;

export const userBlockingPriorityTimeout = 250;
export const normalPriorityTimeout = 5000;
export const lowPriorityTimeout = 10000;
7 changes: 2 additions & 5 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 @@ -16,12 +16,9 @@ export const {
userBlockingPriorityTimeout,
normalPriorityTimeout,
lowPriorityTimeout,
enableIsInputPending,
enableIsInputPendingContinuous,
frameYieldMs,
continuousYieldMs,
maxYieldMs,
} = dynamicFeatureFlags;

export const frameYieldMs = 10;
export const enableSchedulerDebugging = true;
export const enableProfiling: boolean =
__PROFILE__ && enableProfilingFeatureFlag;
3 changes: 1 addition & 2 deletions packages/scheduler/src/forks/SchedulerPostTask.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ let deadline = 0;

let currentPriorityLevel_DEPRECATED = NormalPriority;

// `isInputPending` is not available. Since we have no way of knowing if
// there's pending input, always yield at the end of the frame.
// Always yield at the end of the frame.
export function unstable_shouldYield(): boolean {
return getCurrentTime() >= deadline;
}
Expand Down