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

Remove experimental scheduler flags #16672

Merged
merged 1 commit into from
Sep 5, 2019
Merged

Remove experimental scheduler flags #16672

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: 0 additions & 2 deletions packages/scheduler/src/SchedulerFeatureFlags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,5 @@

export const enableSchedulerDebugging = false;
export const enableIsInputPending = false;
export const requestIdleCallbackBeforeFirstFrame = false;
export const requestTimerEventBeforeFirstFrame = false;
export const enableMessageLoopImplementation = true;
export const enableProfiling = __PROFILE__;
2 changes: 0 additions & 2 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 @@ -12,6 +12,4 @@ export const {
} = require('SchedulerFeatureFlags');

export const enableProfiling = __PROFILE__;
export const requestIdleCallbackBeforeFirstFrame = false;
export const requestTimerEventBeforeFirstFrame = false;
export const enableMessageLoopImplementation = true;
50 changes: 0 additions & 50 deletions packages/scheduler/src/forks/SchedulerHostConfig.default.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@

import {
enableIsInputPending,
requestIdleCallbackBeforeFirstFrame as requestIdleCallbackBeforeFirstFrameFlag,
requestTimerEventBeforeFirstFrame,
enableMessageLoopImplementation,
} from '../SchedulerFeatureFlags';

Expand Down Expand Up @@ -87,7 +85,6 @@ if (
const clearTimeout = window.clearTimeout;
const requestAnimationFrame = window.requestAnimationFrame;
const cancelAnimationFrame = window.cancelAnimationFrame;
const requestIdleCallback = window.requestIdleCallback;

if (typeof console !== 'undefined') {
// TODO: Remove fb.me link
Expand All @@ -107,11 +104,6 @@ if (
}
}

const requestIdleCallbackBeforeFirstFrame =
requestIdleCallbackBeforeFirstFrameFlag &&
typeof requestIdleCallback === 'function' &&
typeof cancelIdleCallback === 'function';

if (
typeof performance === 'object' &&
typeof performance.now === 'function'
Expand Down Expand Up @@ -359,50 +351,8 @@ if (
// Start a rAF loop.
isRAFLoopRunning = true;
requestAnimationFrame(rAFTime => {
if (requestIdleCallbackBeforeFirstFrame) {
cancelIdleCallback(idleCallbackID);
}
if (requestTimerEventBeforeFirstFrame) {
clearTimeout(idleTimeoutID);
}
onAnimationFrame(rAFTime);
});

// If we just missed the last vsync, the next rAF might not happen for
// another frame. To claim as much idle time as possible, post a
// callback with `requestIdleCallback`, which should fire if there's
// idle time left in the frame.
//
// This should only be an issue for the first rAF in the loop;
// subsequent rAFs are scheduled at the beginning of the
// preceding frame.
let idleCallbackID;
if (requestIdleCallbackBeforeFirstFrame) {
idleCallbackID = requestIdleCallback(
function onIdleCallbackBeforeFirstFrame() {
if (requestTimerEventBeforeFirstFrame) {
clearTimeout(idleTimeoutID);
}
frameDeadline = getCurrentTime() + frameLength;
performWorkUntilDeadline();
},
);
}
// Alternate strategy to address the same problem. Scheduler a timer
// with no delay. If this fires before the rAF, that likely indicates
// that there's idle time before the next vsync. This isn't always the
// case, but we'll be aggressive and assume it is, as a trade off to
// prevent idle periods.
let idleTimeoutID;
if (requestTimerEventBeforeFirstFrame) {
idleTimeoutID = setTimeout(function onTimerEventBeforeFirstFrame() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since you've removed this timeout, there's now no timeout fallback for the initial rAF call, like there is for subsequent rAFs:

react/packages/scheduler/src/forks/SchedulerHostConfig.default.js

Lines 306 to 311 in ff00645

const onTimeout = () => {
frameDeadline = getCurrentTime() + frameLength / 2;
performWorkUntilDeadline();
rAFTimeoutID = setTimeout(onTimeout, frameLength * 3);
};
rAFTimeoutID = setTimeout(onTimeout, frameLength * 3);

But it's whatever since we're going to disable the rAF stuff anyway.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That timeout should have already been in the else branch of this feature, anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

there's now no timeout fallback for the initial rAF call, like there is for subsequent rAFs

I guess that explains #16629?

if (requestIdleCallbackBeforeFirstFrame) {
cancelIdleCallback(idleCallbackID);
}
frameDeadline = getCurrentTime() + frameLength;
performWorkUntilDeadline();
}, 0);
}
}
}
};
Expand Down