[Scheduler] Profiling features #16145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
React: size: 🔺+2.7%, gzip: 🔺+2.5% Details of bundled changes.Comparing: 62b04cf...d374a95 react
scheduler
Generated by 🚫 dangerJS |
acdlite
force-pushed
the
scheduler-user-timing-events
branch
10 times, most recently
from
e826e9f
to
9f9e490
Compare
acdlite
changed the title
acdlite
force-pushed
the
scheduler-user-timing-events
branch
from
9f9e490
to
84a03db
Compare
acdlite
force-pushed
the
scheduler-user-timing-events
branch
2 times, most recently
from
a22e71c
to
807db7f
Compare
acdlite
force-pushed
the
scheduler-user-timing-events
branch
from
807db7f
to
4e772ce
Compare
Marks when Scheduler starts and stops running a task. Also marks when a task is initially scheduled, and when Scheduler is waiting for a callback, which can't be inferred from a sample-based JavaScript CPU profile alone. The plan is to use the user-timing events to build a Scheduler profiler that shows how the lifetime of tasks interact with each other and with unscheduled main thread work. The test suite works by printing an text representation of a Scheduler flamegraph.
acdlite
force-pushed
the
scheduler-user-timing-events
branch
from
4e772ce
to
161aba9
Compare
Array contains - the priority Scheduler is currently running - the size of the queue - the id of the currently running task
acdlite
changed the title
acdlite
force-pushed
the
scheduler-user-timing-events
branch
3 times, most recently
from
917d899
to
72f6bca
Compare
sebmarkbage
approved these changes
Aug 13, 2019
Events are written to an array buffer using a custom instruction format. For now, this is only meant to be used during page start up, before the profiler worker has a chance to start up. Once the worker is ready, call `stopLoggingProfilerEvents` to return the log up to that point, then send the array buffer to the worker. Then switch to the sampling based approach.
acdlite
force-pushed
the
scheduler-user-timing-events
branch
from
8993f6e
to
e2d2709
Compare
acdlite
force-pushed
the
scheduler-user-timing-events
branch
2 times, most recently
from
5a3e528
to
0f2cc1a
Compare
Each synchronous block of Scheduler work is given a unique run ID. This is different than a task ID because a single task will have more than one run if it yields with a continuation.
acdlite
force-pushed
the
scheduler-user-timing-events
branch
from
0f2cc1a
to
d374a95
Compare
gaearon
pushed a commit
to gaearon/react
that referenced
this pull request
Aug 14, 2019
* [Scheduler] Mark user-timing events Marks when Scheduler starts and stops running a task. Also marks when a task is initially scheduled, and when Scheduler is waiting for a callback, which can't be inferred from a sample-based JavaScript CPU profile alone. The plan is to use the user-timing events to build a Scheduler profiler that shows how the lifetime of tasks interact with each other and with unscheduled main thread work. The test suite works by printing an text representation of a Scheduler flamegraph. * Expose shared array buffer with profiling info Array contains - the priority Scheduler is currently running - the size of the queue - the id of the currently running task * Replace user-timing calls with event log Events are written to an array buffer using a custom instruction format. For now, this is only meant to be used during page start up, before the profiler worker has a chance to start up. Once the worker is ready, call `stopLoggingProfilerEvents` to return the log up to that point, then send the array buffer to the worker. Then switch to the sampling based approach. * Record the current run ID Each synchronous block of Scheduler work is given a unique run ID. This is different than a task ID because a single task will have more than one run if it yields with a continuation.
gaearon
reviewed
Aug 14, 2019
| @@ -144,5 +160,9 @@ | |||
| return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED | |||
| .Scheduler.unstable_UserBlockingPriority; | |||
| }, | |||
| get unstable_sharedProfilingBuffer() { | |||
| return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED | |||
| .Scheduler.unstable_getFirstCallbackNode; | |||
There was a problem hiding this comment.
Is this a copy paste mistake?
gaearon
reviewed
Aug 14, 2019
| @@ -138,5 +154,9 @@ | |||
| return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED | |||
| .Scheduler.unstable_UserBlockingPriority; | |||
| }, | |||
| get unstable_sharedProfilingBuffer() { | |||
| return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED | |||
| .Scheduler.unstable_getFirstCallbackNode; | |||
gaearon
reviewed
Aug 14, 2019
| @@ -138,5 +154,9 @@ | |||
| return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED | |||
| .Scheduler.unstable_UserBlockingPriority; | |||
| }, | |||
| get unstable_sharedProfilingBuffer() { | |||
| return global.React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED | |||
| .Scheduler.unstable_getFirstCallbackNode; | |||
gaearon
added a commit
that referenced
this pull request
Aug 14, 2019
gaearon
added a commit
to gaearon/react
that referenced
this pull request
Aug 15, 2019
…ebook#16392)" This reverts commit 4ba1412.
This was referenced Aug 15, 2019
gaearon
reviewed
Aug 21, 2019
| typeof SharedArrayBuffer === 'function' | ||
| ? new SharedArrayBuffer(profilingStateSize * Int32Array.BYTES_PER_ELEMENT) | ||
| : // $FlowFixMe Flow doesn't know about ArrayBuffer | ||
| new ArrayBuffer(profilingStateSize * Int32Array.BYTES_PER_ELEMENT); |
There was a problem hiding this comment.
IE9 is still technically supported by us (with polyfills) but AFAIK doesn't have ArrayBuffer. Let's be careful adding new runtime dependencies in unconditional paths, especially for DEV features.
gaearon
reviewed
Aug 21, 2019
| @@ -152,6 +190,8 @@ function flushWork(hasTimeRemaining, initialTime) { | |||
| } | |||
| // Return whether there's additional work | |||
| if (currentTask !== null) { | |||
| markSchedulerSuspended(currentTime); | |||
| isHostCallbackScheduled = true; | |||
There was a problem hiding this comment.
I've found this factoring very counterintuitive.
It's like Scheduler knows too much about the host config. It was surprising to see isHostCallbackScheduled = true here without any requests, and only later realize it "knows" host callback will be scheduled because it returned true, and it trusts host config to respect that.
There's similar implicitness in the error code path.
There was a problem hiding this comment.
The error code path seems inconsistent with what this code seems to imply.
The host config does schedule a callback in the error case (if this throws).
But the catch below doesn't have the corresponding lines:
markSchedulerSuspended(currentTime);
isHostCallbackScheduled = true;Should it?
gaearon
added a commit
to gaearon/react
that referenced
this pull request
Aug 22, 2019
…ebook#16392)" This reverts commit 4ba1412.
sebmarkbage
pushed a commit
that referenced
this pull request
Aug 22, 2019
* Revert "Revert "[Scheduler] Profiling features (#16145)" (#16392)" This reverts commit 4ba1412. * Fix copy paste mistake * Remove init path dependency on ArrayBuffer * Add a regression test for cancelling multiple tasks * Prevent deopt from adding isQueued later * Remove pop() calls that were added for profiling * Verify that Suspend/Unsuspend events match up in tests This currently breaks tests. * Treat Suspend and Resume as exiting and entering work loop Their definitions used to be more fuzzy. For example, Suspend didn't always fire on exit, and sometimes fired when we did _not_ exit (such as at task enqueue). I chatted to Boone, and he's saying treating Suspend and Resume as strictly exiting and entering the loop is fine for their use case. * Revert "Prevent deopt from adding isQueued later" This reverts commit 9c30b0b. Unnecessary because GCC * Start counter with 1 * Group exports into unstable_Profiling namespace * No catch in PROD codepath * No label TODO * No null checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds two new profiling features to Scheduler.
Sample-based approach
The main feature is a way to read the current state of the Scheduler from a worker, using a shared array buffer. The shared array buffer contains (only in the profiling and development builds)
Regarding the last point each synchronous block of Scheduler work is given a unique run ID. This is different than a task ID because a single task will have more than one run if it yields with a continuation.
The worker thread can use this information to construct a flamegraph. The advantage of the sample-based approach is that it scales in proportion to the available resources — if the CPU slows down, the sample rate slows down accordingly.
Event-based approach
The disadvantage of the sample-based approach is that it doesn't during the very first part of page initialization, before the worker has initialized.
To address this problem, there's a separate feature to write to a log of events:
startLoggingProfilerEventsandstopLoggingProfilerEvents.stopLoggingProfilerEventsreturns a typed, 32-bit integer array buffer (though not a shared array buffer). The log conforms to a custom event format. A profiler can then convert this log into a flamegraph.An array buffer is used so that the log can be sent to a worker thread via
postMessageand the structured clone algorithm.The test suite works by printing a text representation of a Scheduler flamegraph. Example:
Both approaches combined
The suggested approach is to use the event log during page start up, then switch to the sample-based approach once the profiler worker is ready.