From 3bcd2de01b5716202eabe8faa338f51bdc59ce26 Mon Sep 17 00:00:00 2001 From: Noah Lemen Date: Tue, 27 Feb 2024 10:39:43 -0500 Subject: [PATCH] clean up isInputPending in Scheduler (#28444) ## 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` --- .../scheduler/src/SchedulerFeatureFlags.js | 4 - .../scheduler/src/__tests__/Scheduler-test.js | 177 +----------------- .../__tests__/SchedulerSetImmediate-test.js | 2 +- packages/scheduler/src/forks/Scheduler.js | 73 +------- .../SchedulerFeatureFlags.www-dynamic.js | 6 - .../src/forks/SchedulerFeatureFlags.www.js | 7 +- .../scheduler/src/forks/SchedulerPostTask.js | 3 +- 7 files changed, 7 insertions(+), 265 deletions(-) diff --git a/packages/scheduler/src/SchedulerFeatureFlags.js b/packages/scheduler/src/SchedulerFeatureFlags.js index 5d207922ca968..c86de5e35dc2a 100644 --- a/packages/scheduler/src/SchedulerFeatureFlags.js +++ b/packages/scheduler/src/SchedulerFeatureFlags.js @@ -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; diff --git a/packages/scheduler/src/__tests__/Scheduler-test.js b/packages/scheduler/src/__tests__/Scheduler-test.js index 14ef25f3ad4f5..0dc9763ac6b0b 100644 --- a/packages/scheduler/src/__tests__/Scheduler-test.js +++ b/packages/scheduler/src/__tests__/Scheduler-test.js @@ -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.'); @@ -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', ]); @@ -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'); diff --git a/packages/scheduler/src/__tests__/SchedulerSetImmediate-test.js b/packages/scheduler/src/__tests__/SchedulerSetImmediate-test.js index 52b71b569f415..bf521b15d3664 100644 --- a/packages/scheduler/src/__tests__/SchedulerSetImmediate-test.js +++ b/packages/scheduler/src/__tests__/SchedulerSetImmediate-test.js @@ -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', ]); diff --git a/packages/scheduler/src/forks/Scheduler.js b/packages/scheduler/src/forks/Scheduler.js index e4eb17b37a2c9..a785bec95320c 100644 --- a/packages/scheduler/src/forks/Scheduler.js +++ b/packages/scheduler/src/forks/Scheduler.js @@ -14,11 +14,7 @@ import type {PriorityLevel} from '../SchedulerPriorities'; import { enableSchedulerDebugging, enableProfiling, - enableIsInputPending, - enableIsInputPendingContinuous, frameYieldMs, - continuousYieldMs, - maxYieldMs, userBlockingPriorityTimeout, lowPriorityTimeout, normalPriorityTimeout, @@ -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); @@ -468,12 +453,8 @@ 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) { @@ -481,58 +462,11 @@ function shouldYieldToHost(): boolean { // 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) { @@ -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; diff --git a/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js b/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js index a96accb914207..25d88aa4b903d 100644 --- a/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js +++ b/packages/scheduler/src/forks/SchedulerFeatureFlags.www-dynamic.js @@ -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; diff --git a/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js b/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js index 2641ac8b57c99..93d1b25b99c52 100644 --- a/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js +++ b/packages/scheduler/src/forks/SchedulerFeatureFlags.www.js @@ -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; diff --git a/packages/scheduler/src/forks/SchedulerPostTask.js b/packages/scheduler/src/forks/SchedulerPostTask.js index 547f165ff9632..a029fce0cbfcc 100644 --- a/packages/scheduler/src/forks/SchedulerPostTask.js +++ b/packages/scheduler/src/forks/SchedulerPostTask.js @@ -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; }