React Rally profiler feedback umbrella issue

[bvaughn]

I did several live demos of the new DevTools profiler with people at React Rally. Some of them used pre-built demo apps and others profiled local builds of their own apps. Over all the feedback was very positive, but I gathered several suggestions and observations from the tests. It's likely that we may not be able to implement all of them, but they are:

Flamegraph

• 49b9e47, 5cf07b2: Flamegraph/ranked charts should enable you to deselect the current fiber so the right hand pane shows commit statistics again. Perhaps this should be a new button in the top of the panel. Clicking in an "empty" space of the flame graph should also deselect.
• e03c7da: Show un-rounded durations on hover in tooltips for more info
• Confirm that rounding isn't lopping off significant digits (all times ended up .00 in one of the demos)

Interactions

• 35c1f2a: Hide the snapshot selector in this view since it's not interactive and it may cause confusion.
• 09edccb: Tie the commits more to the chart views. Maybe use the colors (based on commit duration) for the circles in the interactions view?
• 09edccb: Commits are shown as bars in the charts, but in the interactions tab they're circles. Maybe they should be little colored squares? At least in the "renders" side pane, rather than showing the triangle that fills in, show the colored squares?
• 09edccb: Maybe use black for selected (like we do in the snapshot selector)

Horizontal snapshot selector

• d5047c9, ea8e92c: Make the selected commit more prominent in the horizontal selector than just an alpha transform. Too many people didn't notice it. Maybe add an overlay pattern so it stands out more?
• ad38466: Left/right arrow keys should step through commits/snapshots.
• a2907c8: Make sure jumping from fiber-durations bar chart view to flame/ranked also horizontally scrolls the snapshot selector.
• 0c79424: When dragging, don't leave the dragging state until you release the mouse. (Even if you mouse outside of the selector.)
• Scrollbar gets in the way– makes it hard to see the commits.

Right hand details pane

• 5cf07b2: Make the right hand details pane buttons blue so they stand out more as buttons.
• 6f075a5: Display "committed at" rather than "commit time".
• Profiler: Improve commit "diff" UX #1114: When a fiber+commit is selected, the right details pane could highlight props/state that are different from the previous commit. (This info flashes yellow when you step through commits, but it would be nice to highlight more permanently in this mode.)
• Profiler: Improve "render count" UX #1115: The render count label is confusing since it's a count of renders for the entire app lifetime and not just the window of time when we were recording. (Showing these counts only for when we are profiling might add a lot of weight to the profiler.)
• Profiler: Can't always expand props in right panel #1116: It's confusing that you can't always expand props (or can only expand them a few levels). This might be a bug, or maybe just bad UI.
• It's unclear if "render duration" means duration of the commit, or duration of reconciliation, or both. If it's not commit duration, can/should we add commit duration too?
• Can we add the context menu "Store as global variable" option to props here (like in the Elements tab)? Will this work for mutable data that's old and changed already?

Component durations bar chart

• 5e7de4e: It's not obvious which commits are visible in the component durations bar chart. Perhaps we could dim the commits thate are not visible in the above selector?
• The component durations bar chart was confusing to people at first.

Elements tree view

• e881623: Single-click on the tree view toggle arrow doesn't expand/collapse the tree anymore. (You have to double-click it.)
• b89dbb1: Nodes weren't wrapping correctly in Firefox when the extension was in a detached window. (This also pushed the right props panel off the screen.)
• Consider showing an indicator of which components in the tree are stateful

General

• 27e1f29: "Show native nodes" probably isn't that useful– maybe add a settings/gear icon there and have a profiling preferences popup?
• 802932b, e8c4ecb: Add a preference (in the new profiler preferences popup) to filter commits below a certain duration too (to reduce noise in commit selector).
• Highlight the owner tree when a node is focused #1094: Refactor the breadcrumb UI to remove host components. Preferably only show the owner stack.
• Profiler: Always show profiler tab (even when profiling not supported) #1117: Show Profiler tab always (not just if a profiling capable build of React is present). This tab can be used to up-sell a React upgrade if the version is too old to support profiling.
• Profiler: Add option to go to source (like elements context menu) #1119: Add a way to jump to source from a selected component (similar to the Element tab context menu).
• Profiler: Add import and/or export options #1120: Add an "export" option that dumps profiling data to some JSON format so that it can be shared.
• Profiler: Add import and/or export options #1120: Add an "import" option that can read in exported profiling data and display it. (Perhaps this could be done with a standalone version of the profiler, similar to the DevTools theme browser.)
• Profiler: Support "reload and start profiling" option #1121: Either resume recording if the DevTools (or a web page) are re-loaded while recording is active, or add a restart-and-profile button like Chrome "Performance" tab has. (This may not be possible due to initialization/timing issues.)

[gaearon]

Maybe add a preference (in the new profiler preferences popup) to filter commits below a certain duration too (to reduce noise in commit selector).

That was my concern from watching your videos too. 👍

[bvaughn]

It's also unclear if "render duration" means duration of the commit, or duration of reconciliation, or both. If it's not commit duration, can/should we add commit duration too?

I think I missed a bit of nuance when we were chatting about this yesterday, since I was using my mobile phone and I was out and a bit distracted.

This duration is the time spent in render phase, including lifecycles. We don't actually measure the commit phase. We really only call now() once in the commit phase (in commitRoot) to capture the commit time.

[dan-kez]

Hey Brian, thanks for showing me the profiler at React Rally - it's awesome!

After the suspend API presentations it occurred to me that we don't, from what I could tell, have a central place to see what data has been cached / what we've attempted to "fetch" yet.

It would be nice if there was a way to understand if any particular Resource has been fetched, or is presently fetching.

const UserName = ({ userId }) => (
  const user = UsersResource.read(cache, userId)
  return (<span>{user.name}</span>);
);

In the tools it would be nice to see distinct events for when UsersResource is initially called, when it returned, and subsequent reads from cache based on the arguments to read. It would be especially useful if there was a top level view similar to redux, rather than needing to hunt for the components where these calls are made.

That said, this definitely may be scope creep for the profiler.

[bvaughn]

Hi @dan-kez 👋Thank you, and you're welcome.

I'd suggest filing a new GitHub issue to discuss this ^

Currently, it would not be possible for DevTools to do what you're describing because it doesn't actually have access to the resource/cache. Since we own the simple-cache-provider package, it would be possible to build hooks into that package to enable this sort of functionality– but I'm not sure we would want to do that since it would add some overhead to the package. Seems like it merits a separate discussion to consider pros and cons 😄

[dan-kez]

Sounds great - thanks for the quick response!

I just opened an issue to discuss exposing these hooks while in __DEV__ mode in the react repo.

[gaearon]

We don't actually measure the commit phase. We really only call now() once in the commit phase (in commitRoot) to capture the commit time.

Interesting. My impression was that especially with async rendering, the commit duration will become more important because it can’t be time sliced. Long commits are bad. But overall render duration is less important if it was deferred and time sliced successfully.

[gaearon]

See also facebook/react#9075

[bvaughn]

the commit duration will become more important because it can’t be time sliced

Yeah, this is my understanding too. Commit phase is very time sensitive– but because of this, we also wanted to avoid starting/stopping a bunch of timers and adding/bubbling a bunch of durations.

[jesup]

Concerning import/export of profiles.... I haven't looked at the in-devtools version, but the perf-html.io version has JSON export and imports JSON (and soon linux perf and native mac profiles)

[swyxio]

mini-issue: 1
minor UX issue re: Horizontal snapshot selector - currently the left/right arrow keys wrap around when at either end of the snapshots. (i.e. when i'm at the 1st snapshot and press left, it wraps to the last snapshot). i believe it should not wrap.