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
Combine Profiler data sources #22529
Comments
|
I use Chrome Browser all the time and I run into that problem also. My
browser want load or it throws a error message (505 ) or a (404) i have got
a error that was a (808) i don't know we're the 808 came from but the other
two was Browser issues. If i can find the reports on it I'll send them to
ya..
James Loftin
…On Fri, Oct 8, 2021, 10:56 AM Brian Vaughn ***@***.***> wrote:
The scheduling profiler currently shows component *names* for things like
state-updates (e.g. "state update scheduled by Foo") but it does not
include component or call stacks. These would both be useful because:
- Component stack would help identify *which instance of foo* was e.g.
scheduling the update
- Call stack would show e.g. which piece of state was being updated
(and by what)
The reason we don't include this information is because we won't want to
pay the cost of string serializing it all and logging it as a User Timing
mark.
What if we took a step back and revisited how the scheduling profiler
worked?
Currently, the profiler is entirely driven off of Chrome performance
profile data, which includes several React-specific user timing marks for
things like updates, rendering/committing, suspense, etc. Recording a
profile requires a user to do the following:
1. Click "record" in Chrome
2. Use the app
3. Click "stop" in Chrome
4. Export the profile JSON
5. Import it into the React DevTools
It would be nice if DevTools could start/stop recording and import the
data itself, but the only way to do this is using the Chrome DevTools
Protocol which would *require* the use of an extremely powerful
permission that I don't think we would want to ask users for.
This approach also has the downside of not working with React Native (or
non-Chrome browsers).
So why do we do it? We do it because the profile gives us a lot of nice
extra data: CPU samples of the JavaScript stack, Network requests,
screenshots (if enabled), user events (e.g. "click").
But we don't *need* this data. The scheduling profiler could still be a
useful tool even if included only the React specific marks.
My proposal then is that we consider doing this:
- Replace the user timing API with a direct React-to-DevTools API
(where these marks are logged to DevTools directly).
- When profiling starts in React DevTools, generate a unique ID string
- Log this unique ID to the User Profiling API along with each commit
(e.g. "react-sync-marker---")
- Allow users to export/import additional native profiling information
(which can be aligned using the sync marks) *but is not required*.
Doing this has a few benefits:
- It would make the profiler easier to use in the simple case (if you
didn't want/need native profiler info) since you could start/stop it from
within React DevTools.
- It would let us add component and call stacks without the extra
serialization overhead.
It would have the following downside though:
- Recording profiles with native information would be a little more
complicated, since you'd need to start React DevTools *and then* start
the browser profiler. (We could show instructions for doing this in the
DevTools profiler though.)
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#22529>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKVRGWUPTQADIGNOFYRAZ4DUF4IDDANCNFSM5FT5G3LQ>
.
|
|
Note that this issue is related to #22090. |
bvaughn
added a commit
that referenced
this issue
Oct 21, 2021
…nt) (#22578) DevTools (and its profilers) should not require users to be familiar with React internals. Although the scheduling profiler includes a CPU sample flame graph, it's there for advanced use cases and shouldn't be required to identify common performance issues. This PR proposes adding new marks around component effects. This will enable users to identify components with slow effect create/destroy functions without requiring them to dig through the call stack. (Once #22529 lands, these new marks will also include component stacks, making them more useful still.) For example, here's a profile with a long-running effect. Without this change, it's not clear why the effects phase takes so long. After this change, it's more clear why that the phase is longer because of a specific component. We may consider adding similar marks around render phase hooks like useState, useReducer, useMemo. I avoided doing that in this PR because it would be a pretty pervasive change to the ReactFiberHooks file. Note that this change should have no effect on production bundles since everything is guarded behind a profiling feature flag. Going to tag more people than I normally would for this pR, since it touches both reconciler and DevTools packages. Feel free to ignore though if you don't have strong feelings. Note that although this PR adds new marks to the scheduling profiler, it's done in a way that's backwards compatible for older profiles.
KamranAsif
added a commit
to KamranAsif/react
that referenced
this issue
Nov 4, 2021
…nt) (facebook#22578) DevTools (and its profilers) should not require users to be familiar with React internals. Although the scheduling profiler includes a CPU sample flame graph, it's there for advanced use cases and shouldn't be required to identify common performance issues. This PR proposes adding new marks around component effects. This will enable users to identify components with slow effect create/destroy functions without requiring them to dig through the call stack. (Once facebook#22529 lands, these new marks will also include component stacks, making them more useful still.) For example, here's a profile with a long-running effect. Without this change, it's not clear why the effects phase takes so long. After this change, it's more clear why that the phase is longer because of a specific component. We may consider adding similar marks around render phase hooks like useState, useReducer, useMemo. I avoided doing that in this PR because it would be a pretty pervasive change to the ReactFiberHooks file. Note that this change should have no effect on production bundles since everything is guarded behind a profiling feature flag. Going to tag more people than I normally would for this pR, since it touches both reconciler and DevTools packages. Feel free to ignore though if you don't have strong feelings. Note that although this PR adds new marks to the scheduling profiler, it's done in a way that's backwards compatible for older profiles.
bvaughn
added a commit
that referenced
this issue
Jan 13, 2022
Until now, DEV and PROFILING builds of React recorded Timeline profiling data using the User Timing API. This commit changes things so that React records this data by calling methods on the DevTools hook. (For now, DevTools still records that data using the User Timing API, to match previous behavior.) This commit is large but most of it is just moving things around: * New methods have been added to the DevTools hook (in "backend/profilingHooks") for recording the Timeline performance events. * Reconciler's "ReactFiberDevToolsHook" has been updated to call these new methods (when they're present). * User Timing method calls in "SchedulingProfiler" have been moved to DevTools "backend/profilingHooks" (to match previous behavior, for now). * The old reconciler tests, "SchedulingProfiler-test" and "SchedulingProfilerLabels-test", have been moved into DevTools "TimelineProfiler-test" to ensure behavior didn't change unexpectedly. * Two new methods have been added to the injected renderer interface: injectProfilingHooks() and getLaneLabelMap(). Relates to #22529.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Time sensitive API changes (ideally these should land before 18.0 release):
(DevTools will decide whether or not to store the data or log User Timing marks, since it knows when it's profiling.)
Additional, non-blocking changes:
Motivation
There are currently two React profilers: the "legacy" profiler (which reads data from Fibers during the commit phase) and the "scheduling" profiler (which reads data in the form of User Timing marks in a Chrome performance profile). This separation is confusing as both profilers live in the same extension/app but import/export different types of data.
Let's take a step back and revisit how the scheduling profiler works...
Recording a profile currently requires a user to do the following:
It would be nice if DevTools could start/stop recording and import the data itself, but the only way to do this is using the Chrome DevTools Protocol which would require the use of an extremely powerful permission that I don't think we would want to ask users for.
This approach also has the downside of not working with React Native (or non-Chrome browsers).
So why do we do it? We do it because the profile gives us a lot of nice extra data: CPU samples of the JavaScript stack, Network requests, screenshots (if enabled), user events (e.g. "click").
But we don't need this data. The scheduling profiler could still be a useful tool even if included only the React specific marks.
My proposal then is that we consider doing this:
"react-sync-marker-<uid>-<index>-<timestamp>")Doing this has a few benefits:
It would have the following downside though:
A note about component and call stacks
The scheduling profiler currently shows component names for things like state-updates (e.g. "state update scheduled by Foo") but it does not include component or call stacks. These would both be useful because:
The reason we don't include this information is because we won't want to pay the cost of string serializing it all and logging it as a User Timing mark.
The text was updated successfully, but these errors were encountered: