Skip to content

Commit

Permalink
clean up isInputPending in Scheduler (#28444)
Browse files Browse the repository at this point in the history 
## Summary

`isInputPending` is not in use. This PR cleans up the flags controlling
its gating and parameters to simplify Scheduler.

Makes `frameYieldMs` feature flag static, set to 10ms in www, which we
found built on the wins provided by a broader yield interval via
`isInputPending`. Flag remains set to 5ms in OSS builds.

## How did you test this change?
`yarn test Scheduler`
  • Loading branch information
@noahlemen
noahlemen authored Feb 27, 2024
1 parent 172a7f6 commit 3bcd2de
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 265 deletions.
4 changes: 0 additions & 4 deletions packages/scheduler/src/SchedulerFeatureFlags.js
View file
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
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
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
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() {}

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
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
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
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

1 comment on commit 3bcd2de

@IVLIU
Copy link

@IVLIU IVLIU commented on 3bcd2de Mar 6, 2024

Choose a reason for hiding this comment

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

Why should it be removed? I think isInputPending brings better responsiveness.

Please sign in to comment.