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

Add a Redux dispatch perf analysis panel #9776

Closed
wants to merge 2 commits into from

Conversation

markerikson
Copy link
Collaborator

This PR:

  • Is stacked on top of Rewrite "React Queued Renders" panel for better perf, and make clear it's experimental #9775 , and also supersedes Analyze React+Redux perf timings using recording analysis #9410
  • Adds a new "Redux Dispatch Perf" experimental panel and associated logic
    • Adds caches that find the location of the Redux dispatch method and calls to dispatch. (This heavily overlaps with the existing Redux DevTools panel, but works even if there are no annotations or Redux DevTools integration. It does rely on sourcemaps having a node_modules/redux/redux.js-type file available)
    • Adds caches that fetch hits for specific lines of code within the Redux dispatch method, and uses those to calculate durations for the reducer execution and subscriber notifications
    • Renders a virtual list of Redux dispatch entries
    • Allows selecting a dispatch entry and showing further details in a bottom panel
  • The details panel shows:
    • timing breakdowns for the dispatch
    • estimates for how many components we think were queued to re-render by the dispatch, based on calls to scheduleUpdateOnFiber during the subscriber notifications
    • Tries to find a React render that occurred right after the dispatch, and if found, estimates its duration

image

Future Work

There's a lot of things I've proved that I can do that are not in the current version of this PR:

  • Finding all Redux selectors used in an app
  • Counting how many selectors got called during a set of subscriber notifications, and calculating their durations
  • Showing the red "you are here" line in the list

@vercel
Copy link

vercel bot commented Sep 28, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
devtools ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 12, 2023 9:42pm

@replay-io
Copy link

replay-io bot commented Sep 28, 2023

E2E Tests

85 replays were recorded for beafee8.

image 0 Failed
image 85 Passed

View test run on Replay ↗︎

Snapshot Tests

100 replays were recorded for beafee8.

image 0 Failed
image 100 Passed

View test run on Replay ↗︎

@bvaughn
Copy link
Contributor

bvaughn commented Oct 2, 2023

My personal vote is that we should not land this feature.

  1. The list data is very close to the existing Redux panel. I don't think it makes sense to add a new, separate section for it.
  2. I don't think the information in the details pane is very useful in its current format. (I know you say it's useful for you Mark, but I don't think it's useful to a wide enough % of our users– and I think the bar for shipping something should be that it's useful for many/most Replay users.)

Given that it's essentially all new code, I think we should just keep it in a fork.

@markerikson
Copy link
Collaborator Author

FWIW I'm not tied to the specific UI location or implementation - most of this was just copy-pasting the setup and layout logic from the "React Render" panel because they're both experimental and it was the easiest way to get something on screen.

I still absolutely think there is value in this info, but I can understand the argument that not enough people would care about it for now.

@markerikson markerikson force-pushed the feature/FE-1631-react-perf-panel branch 2 times, most recently from e94541d to 9a60782 Compare October 12, 2023 21:05
Base automatically changed from feature/FE-1631-react-perf-panel to main October 12, 2023 21:21
@markerikson markerikson force-pushed the feature/FE-1631-redux-perf-panel branch from 83b54c9 to beafee8 Compare October 12, 2023 21:38
@replay-delta
Copy link

replay-delta bot commented Oct 12, 2023

@markerikson
Copy link
Collaborator Author

Okay, I've rebased this on top of main as of landing #9775 . The preview branch is here:

https://devtools-git-feature-fe-1631-redux-perf-panel-recordreplay.vercel.app/

I'll go ahead and close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants