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.

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

Implement pauseExecution, continueExecution, dumpQueue for Scheduler #14053

Merged
merged 3 commits into from
Dec 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
75 changes: 74 additions & 1 deletion fixtures/scheduler/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,21 @@ <h2>Tests:</h2>
</div>
<div> If the counter advanced while you were away from this tab, it's correct.</div>
</li>
<li>
<p>Can pause execution, dump scheduled callbacks, and continue where it left off</p>
<button onClick="runTestEight()">Run Test 8</button>
<div><b>Click the button above, press "continue" to finish the test after it pauses:</b></div>
<button onClick="continueTestEight()">continue</button>
<div><b>Expected:</b></div>
<div id="test-8-expected">
</div>
<div> -------------------------------------------------</div>
<div> If the test didn't progress until you hit "continue" and </div>
<div> you see the same above and below afterwards it's correct.
<div> -------------------------------------------------</div>
<div><b>Actual:</b></div>
<div id="test-8"></div>
</li>
</ol>
<script src="../../build/node_modules/react/umd/react.development.js"></script>
<script src="../../build/node_modules/scheduler/umd/scheduler.development.js"></script>
Expand All @@ -98,7 +113,10 @@ <h2>Tests:</h2>
const {
unstable_scheduleCallback: scheduleCallback,
unstable_cancelCallback: cancelCallback,
unstable_now: now
unstable_now: now,
unstable_getFirstCallbackNode: getFirstCallbackNode,
unstable_pauseExecution: pauseExecution,
unstable_continueExecution: continueExecution,
} = Scheduler;
function displayTestResult(testNumber) {
const expectationNode = document.getElementById('test-' + testNumber + '-expected');
Expand Down Expand Up @@ -215,6 +233,16 @@ <h2>Tests:</h2>
[
// ... TODO
],
[],
[],
// Test 8
[
'Queue size: 0.',
'Pausing... press continue to resume.',
'Queue size: 2.',
'Finishing...',
'Done!',
],
];
function runTestOne() {
// Test 1
Expand Down Expand Up @@ -496,6 +524,51 @@ <h2>Tests:</h2>
}
scheduleCallback(incrementCounterAndScheduleNextCallback);
}

function runTestEight() {
// Test 8
// Pauses execution, dumps the queue, and continues execution
clearTestResult(8);

function countNodesInStack(firstCallbackNode) {
var node = firstCallbackNode;
var count = 0;
if (node !== null) {
do {
count = count + 1;
node = node.next;
} while (node !== firstCallbackNode);
}
return count;
}

scheduleCallback(() => {

// size should be 0
updateTestResult(8, `Queue size: ${countNodesInStack(getFirstCallbackNode())}.`);
updateTestResult(8, 'Pausing... press continue to resume.');
pauseExecution();

scheduleCallback(function () {
updateTestResult(8, 'Finishing...');
displayTestResult(8);
})
scheduleCallback(function () {
updateTestResult(8, 'Done!');
displayTestResult(8);
checkTestResult(8);
})

// new size should be 2 now
updateTestResult(8, `Queue size: ${countNodesInStack(getFirstCallbackNode())}.`);
displayTestResult(8);
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding a new integration test!


function continueTestEight() {
continueExecution();
}

</script type="text/babel">
</body>
</html>
6 changes: 6 additions & 0 deletions packages/react/src/ReactSharedInternals.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ import {
unstable_now,
unstable_scheduleCallback,
unstable_runWithPriority,
unstable_getFirstCallbackNode,
unstable_pauseExecution,
unstable_continueExecution,
unstable_wrapCallback,
unstable_getCurrentPriorityLevel,
} from 'scheduler';
Expand Down Expand Up @@ -49,6 +52,9 @@ if (__UMD__) {
unstable_scheduleCallback,
unstable_runWithPriority,
unstable_wrapCallback,
unstable_getFirstCallbackNode,
unstable_pauseExecution,
unstable_continueExecution,
unstable_getCurrentPriorityLevel,
},
SchedulerTracing: {
Expand Down
24 changes: 24 additions & 0 deletions packages/scheduler/npm/umd/scheduler.development.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,27 @@
);
}

function unstable_getFirstCallbackNode() {
return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_getFirstCallbackNode.apply(
this,
arguments
);
}

function unstable_pauseExecution() {
return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_pauseExecution.apply(
this,
arguments
);
}

function unstable_continueExecution() {
return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_continueExecution.apply(
this,
arguments
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I strongly dislike the fact that the newly proposed API methods only exist on the DEV build.

Do other packages do this? I know that sometimes methods are no-ops in production builds, but having them be missing entirely feels weird.

I also think the fact that these methods exist only for DEV reinforces the notion that you could just use existing browser tooling (like the browser's built-in pause/resume functionality).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have an opinion on this. no-oping in production seems a good solution in my book.


return Object.freeze({
unstable_now: unstable_now,
unstable_scheduleCallback: unstable_scheduleCallback,
Expand All @@ -76,5 +97,8 @@
unstable_runWithPriority: unstable_runWithPriority,
unstable_wrapCallback: unstable_wrapCallback,
unstable_getCurrentPriorityLevel: unstable_getCurrentPriorityLevel,
unstable_continueExecution: unstable_continueExecution,
unstable_pauseExecution: unstable_pauseExecution,
unstable_getFirstCallbackNode: unstable_getFirstCallbackNode,
});
});
18 changes: 18 additions & 0 deletions packages/scheduler/npm/umd/scheduler.production.min.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,21 @@
);
}

function unstable_getFirstCallbackNode() {
return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_getFirstCallbackNode.apply(
this,
arguments
);
}

function unstable_pauseExecution() {
return undefined;
}

function unstable_continueExecution() {
return undefined;
}

return Object.freeze({
unstable_now: unstable_now,
unstable_scheduleCallback: unstable_scheduleCallback,
Expand All @@ -76,5 +91,8 @@
unstable_runWithPriority: unstable_runWithPriority,
unstable_wrapCallback: unstable_wrapCallback,
unstable_getCurrentPriorityLevel: unstable_getCurrentPriorityLevel,
unstable_continueExecution: unstable_continueExecution,
unstable_pauseExecution: unstable_pauseExecution,
unstable_getFirstCallbackNode: unstable_getFirstCallbackNode,
});
});
18 changes: 18 additions & 0 deletions packages/scheduler/npm/umd/scheduler.profiling.min.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,21 @@
);
}

function unstable_getFirstCallbackNode() {
return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.Scheduler.unstable_getFirstCallbackNode.apply(
this,
arguments
);
}

function unstable_pauseExecution() {
return undefined;
}

function unstable_continueExecution() {
return undefined;
}

return Object.freeze({
unstable_now: unstable_now,
unstable_scheduleCallback: unstable_scheduleCallback,
Expand All @@ -76,5 +91,8 @@
unstable_runWithPriority: unstable_runWithPriority,
unstable_wrapCallback: unstable_wrapCallback,
unstable_getCurrentPriorityLevel: unstable_getCurrentPriorityLevel,
unstable_continueExecution: unstable_continueExecution,
unstable_pauseExecution: unstable_pauseExecution,
unstable_getFirstCallbackNode: unstable_getFirstCallbackNode,
});
});
41 changes: 39 additions & 2 deletions packages/scheduler/src/Scheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

/* eslint-disable no-var */

import {enableSchedulerDebugging} from 'shared/ReactFeatureFlags';

// TODO: Use symbols?
var ImmediatePriority = 1;
var UserBlockingPriority = 2;
Expand All @@ -33,6 +35,9 @@ var IDLE_PRIORITY = maxSigned31BitInt;
var firstCallbackNode = null;

var currentDidTimeout = false;
// Pausing the scheduler is useful for debugging.
var isSchedulerPaused = false;
Copy link
Contributor Author

@mrkev mrkev Nov 5, 2018

Choose a reason for hiding this comment

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

@acdlite I think I might want to expose this variable too, since for debugging it's necessary to know if the thing is paused at all in the first place


var currentPriorityLevel = NormalPriority;
var currentEventStartTime = -1;
var currentExpirationTime = -1;
Expand Down Expand Up @@ -173,13 +178,23 @@ function flushImmediateWork() {
}

function flushWork(didTimeout) {
// Exit right away if we're currently paused

if (enableSchedulerDebugging && isSchedulerPaused) {
return;
}

isExecutingCallback = true;
const previousDidTimeout = currentDidTimeout;
currentDidTimeout = didTimeout;
try {
if (didTimeout) {
// Flush all the expired callbacks without yielding.
while (firstCallbackNode !== null) {
while (
firstCallbackNode !== null &&
!(enableSchedulerDebugging && isSchedulerPaused)
) {
// TODO Wrap i nfeature flag
// Read the current time. Flush all the callbacks that expire at or
// earlier than that time. Then read the current time again and repeat.
// This optimizes for as few performance.now calls as possible.
Expand All @@ -189,7 +204,8 @@ function flushWork(didTimeout) {
flushFirstCallback();
} while (
firstCallbackNode !== null &&
firstCallbackNode.expirationTime <= currentTime
firstCallbackNode.expirationTime <= currentTime &&
!(enableSchedulerDebugging && isSchedulerPaused)
);
continue;
}
Expand All @@ -199,6 +215,9 @@ function flushWork(didTimeout) {
// Keep flushing callbacks until we run out of time in the frame.
if (firstCallbackNode !== null) {
do {
if (enableSchedulerDebugging && isSchedulerPaused) {
break;
}
flushFirstCallback();
} while (firstCallbackNode !== null && !shouldYieldToHost());
}
Expand Down Expand Up @@ -342,6 +361,21 @@ function unstable_scheduleCallback(callback, deprecated_options) {
return newNode;
}

function unstable_pauseExecution() {
isSchedulerPaused = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why isn't the browser's built-in pause/resume functionality sufficient for whatever you're using this for?

}

function unstable_continueExecution() {
isSchedulerPaused = false;
if (firstCallbackNode !== null) {
ensureHostCallbackIsScheduled();
}
}

function unstable_getFirstCallbackNode() {
return firstCallbackNode;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of exposing play, pause, and dump-queue– what if we just exposed the queue itself (firstCallbackNode) somewhere globally in DEV mode only? Then we could just write our own tiny helper function that read from it (if that was actually needed).

(This also assumes that we just use the browser's built-in play/pause script execution functionality.


function unstable_cancelCallback(callbackNode) {
var next = callbackNode.next;
if (next === null) {
Expand Down Expand Up @@ -659,5 +693,8 @@ export {
unstable_wrapCallback,
unstable_getCurrentPriorityLevel,
unstable_shouldYield,
unstable_continueExecution,
unstable_pauseExecution,
unstable_getFirstCallbackNode,
getCurrentTime as unstable_now,
};
5 changes: 4 additions & 1 deletion packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;

// Only used in www builds.
export const enableSuspenseServerRenderer = false;
export const enableSuspenseServerRenderer = false; // TODO: __DEV__? Here it might just be false.

// Only used in www builds.
export const enableSchedulerDebugging = __DEV__;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const enableSuspenseServerRenderer = false;
export const disableInputAttributeSyncing = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;
export const enableSchedulerDebugging = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const enableSuspenseServerRenderer = false;
export const disableInputAttributeSyncing = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;
export const enableSchedulerDebugging = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export const enableSchedulerTracing = __PROFILE__;
export const enableSuspenseServerRenderer = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;
export const enableSchedulerDebugging = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const enableSuspenseServerRenderer = false;
export const disableInputAttributeSyncing = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;
export const enableSchedulerDebugging = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.persistent.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const enableSuspenseServerRenderer = false;
export const disableInputAttributeSyncing = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;
export const enableSchedulerDebugging = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export const enableSuspenseServerRenderer = false;
export const disableInputAttributeSyncing = false;
export const enableStableConcurrentModeAPIs = false;
export const warnAboutShorthandPropertyCollision = false;
export const enableSchedulerDebugging = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const enableProfilerTimer = false;
export const enableSchedulerTracing = false;
export const enableSuspenseServerRenderer = false;
export const enableStableConcurrentModeAPIs = false;
export const enableSchedulerDebugging = false;

// Only used in www builds.
export function addUserTimingListener() {
Expand Down
1 change: 1 addition & 0 deletions packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export let enableUserTimingAPI = __DEV__;

export const enableProfilerTimer = __PROFILE__;
export const enableSchedulerTracing = __PROFILE__;
export const enableSchedulerDebugging = __DEV__; // TODO or just true

export const enableStableConcurrentModeAPIs = false;

Expand Down
Loading