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

Proposal for a React performance table #801

Open
wwwillchen opened this Issue Jun 6, 2017 · 17 comments

Comments

Projects
None yet
7 participants
@wwwillchen

wwwillchen commented Jun 6, 2017

Hey everyone - I'm one of the engineers on the Chrome DevTools team and I built a small proof-of-concept React performance Chrome DevTools extension as part of my 20% time.

I've been very excited about React for a while and saw some interesting React performance tooling such as the Performance User Timings and react-addons-perf, and saw the potential value of creating a view that summarizes component-based lifecycle timings in a table.

This tool could highlight interesting performance insights:

  • Which components are taking the longest time on load?
  • What's taking the most time after certain user actions (e.g. scrolling a list-type view, clicking on a new route)?
  • How many component instances are there at a given moment?

I've created a repo here with the POC and usage docs:
➡️ https://github.com/wwwillchen/react-perf-tool

Demo GIF

react-perf-tool-smaller-2


I'd personally love to see tooling like this available right in React DevTools. Additionally, on the Chrome DevTools team, we're interested in how we can adapt a mature version of this into a generalized component-based profiling view that can be in the DevTools' Performance panel.

I'm not sure if I'm the best person to upstream this tool into React DevTools, but I'm happy to help anyone who's interested in that.

@paulirish

This comment has been minimized.

Show comment
Hide comment

paulirish commented Jun 6, 2017

@bvaughn

This comment has been minimized.

Show comment
Hide comment
@bvaughn

bvaughn Jun 6, 2017

Contributor

I'm not sure if I'm the best person to upstream this tool into React DevTools, but I'm happy to help anyone who's interested in that.

I'd be happy to help!

Contributor

bvaughn commented Jun 6, 2017

I'm not sure if I'm the best person to upstream this tool into React DevTools, but I'm happy to help anyone who's interested in that.

I'd be happy to help!

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Jun 6, 2017

Member

This looks great!

For context: We removed our ReactPerf programmatic API in the React Fiber implementation scheduled to be released in React v16. We were planning on doing a new take on it that takes into account React Fiber's scheduling primitives.

In that model, the timing may be aborted and resumed in ways that might be a bit misleading when viewed as absolute numbers. Think of it as if something really heavy happens to run in the background at the same time as a particular component runs.

There are also other considerations for perf. E.g. at certain points (such as our "commit" phase) performance matters more than during async step. So we'd want to highlight such things rather than absolute time to execute.

We're still thinking about what these new primitives means for perf tooling. We'd love to chat with you more about our thoughts.

In the meantime this certainly looks like a great way to visualize React trees that run in synchronous mode.

Member

sebmarkbage commented Jun 6, 2017

This looks great!

For context: We removed our ReactPerf programmatic API in the React Fiber implementation scheduled to be released in React v16. We were planning on doing a new take on it that takes into account React Fiber's scheduling primitives.

In that model, the timing may be aborted and resumed in ways that might be a bit misleading when viewed as absolute numbers. Think of it as if something really heavy happens to run in the background at the same time as a particular component runs.

There are also other considerations for perf. E.g. at certain points (such as our "commit" phase) performance matters more than during async step. So we'd want to highlight such things rather than absolute time to execute.

We're still thinking about what these new primitives means for perf tooling. We'd love to chat with you more about our thoughts.

In the meantime this certainly looks like a great way to visualize React trees that run in synchronous mode.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 6, 2017

Member

This is very very neat. 😍 We were hoping that somebody would build this when we started to use the platform user timing APIs. Particularly because ReactPerf was a bit hard to maintain, and it seemed to make more sense to build tools like this around a more generic API.

We don’t plan to ship ReactPerf with Fiber, and I’m excited this could be a viable replacement before we start figuring out what a more sophisticated profiler focused on Fiber (and especially its async mode which isn’t enabled by default yet) could look like.

I’m pretty sure we want to get something like this into React DevTools. My wishes would be that:

  • We write this as a part of this repo rather than a standalone extension.
  • It will probably need a record-like mechanism to only calculate when necessary.
  • It should focus on React 16 compatibility (react@next + react-dom@next) which also uses User Timing API although with a slightly different measurement format. It’s fine to not support versions below.

I have some concerns too:

  • React is slower with the extension on. We need to check if that has a bad effect on measurements. (It’s important to test on React 16 specifically because it performs extension work at different time, and maybe this won’t be a problem.)
  • We need to ensure it’s organic and doesn’t clutter UI.
  • We need to figure out how to keep it viable/performant for React Native which doesn’t have a native performance implementation (but I’m adding a simple polyfill soon).
Member

gaearon commented Jun 6, 2017

This is very very neat. 😍 We were hoping that somebody would build this when we started to use the platform user timing APIs. Particularly because ReactPerf was a bit hard to maintain, and it seemed to make more sense to build tools like this around a more generic API.

We don’t plan to ship ReactPerf with Fiber, and I’m excited this could be a viable replacement before we start figuring out what a more sophisticated profiler focused on Fiber (and especially its async mode which isn’t enabled by default yet) could look like.

I’m pretty sure we want to get something like this into React DevTools. My wishes would be that:

  • We write this as a part of this repo rather than a standalone extension.
  • It will probably need a record-like mechanism to only calculate when necessary.
  • It should focus on React 16 compatibility (react@next + react-dom@next) which also uses User Timing API although with a slightly different measurement format. It’s fine to not support versions below.

I have some concerns too:

  • React is slower with the extension on. We need to check if that has a bad effect on measurements. (It’s important to test on React 16 specifically because it performs extension work at different time, and maybe this won’t be a problem.)
  • We need to ensure it’s organic and doesn’t clutter UI.
  • We need to figure out how to keep it viable/performant for React Native which doesn’t have a native performance implementation (but I’m adding a simple polyfill soon).
@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 6, 2017

Member

In that model, the timing may be aborted and resumed in ways that might be a bit misleading when viewed as absolute numbers. Think of it as if something really heavy happens to run in the background at the same time as a particular component runs.

In this particular case this should be okay because (in the current implementation) we stop the timers before pausing, and start after resuming. So absolute numbers shouldn't look bad. But there are probably other things to watch out for too (e.g. "wasted" low priority idle renders are probably less big of a deal).

Member

gaearon commented Jun 6, 2017

In that model, the timing may be aborted and resumed in ways that might be a bit misleading when viewed as absolute numbers. Think of it as if something really heavy happens to run in the background at the same time as a particular component runs.

In this particular case this should be okay because (in the current implementation) we stop the timers before pausing, and start after resuming. So absolute numbers shouldn't look bad. But there are probably other things to watch out for too (e.g. "wasted" low priority idle renders are probably less big of a deal).

@sebmarkbage

This comment has been minimized.

Show comment
Hide comment
@sebmarkbage

sebmarkbage Jun 7, 2017

Member

@gaearon There are two types of resumes. 1) Ones that are just paused to do other JS work. 2) Ones that are pause, do other React work, maybe higher priority and then rerenders.

The first one is fine. The second one can lead to more work than it otherwise would have. E.g. if it leads to a rerender.

Member

sebmarkbage commented Jun 7, 2017

@gaearon There are two types of resumes. 1) Ones that are just paused to do other JS work. 2) Ones that are pause, do other React work, maybe higher priority and then rerenders.

The first one is fine. The second one can lead to more work than it otherwise would have. E.g. if it leads to a rerender.

@wwwillchen

This comment has been minimized.

Show comment
Hide comment
@wwwillchen

wwwillchen Jun 7, 2017

Thanks for all the feedback!

Given that React Fiber async mode is the future, I think it makes sense to hold off on this specific work until the new perf API has been formed.

On my end, the DevTools team is interested in visualizing these types of User Timings within the Performance panel since this is something that'll be valuable across frameworks. I'm going to try prototype how we could visualize these user timings within the Performance panel itself (for example, it could be a data table like the Bottom-up view [0]). I'd be interested in getting your team's feedback once I have a working prototype.

[0]
image

wwwillchen commented Jun 7, 2017

Thanks for all the feedback!

Given that React Fiber async mode is the future, I think it makes sense to hold off on this specific work until the new perf API has been formed.

On my end, the DevTools team is interested in visualizing these types of User Timings within the Performance panel since this is something that'll be valuable across frameworks. I'm going to try prototype how we could visualize these user timings within the Performance panel itself (for example, it could be a data table like the Bottom-up view [0]). I'd be interested in getting your team's feedback once I have a working prototype.

[0]
image

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 7, 2017

Member

That would be amazing. We also have a feature request to surface suspicious timings which indicate problems. Not sure if there's any way to align it with User Timing API but it's pretty important to us. (The view for native JS already has this feature, e.g. code triggering layouts is highlighted differently.)

See w3c/user-timing#25.

Member

gaearon commented Jun 7, 2017

That would be amazing. We also have a feature request to surface suspicious timings which indicate problems. Not sure if there's any way to align it with User Timing API but it's pretty important to us. (The view for native JS already has this feature, e.g. code triggering layouts is highlighted differently.)

See w3c/user-timing#25.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 12, 2017

Member

@wwwillchen I was curious if you could take a look at overhead of User Timing API calls. We were going to keep them enabled in React 16, but in some cases (e.g. many components updating very often in DEV), performance.measure() and performance.mark() calls slow down the app several times.

For example, you can try https://github.com/ScottWeinstein/react-ng-perf-comparison/tree/master/react-16 (npm install and npm start in that folder). If you comment out all calls to performance.* in node_modules/react-dom/cjs/react-dom.development.js), the app will be 7x times faster.

Member

gaearon commented Jun 12, 2017

@wwwillchen I was curious if you could take a look at overhead of User Timing API calls. We were going to keep them enabled in React 16, but in some cases (e.g. many components updating very often in DEV), performance.measure() and performance.mark() calls slow down the app several times.

For example, you can try https://github.com/ScottWeinstein/react-ng-perf-comparison/tree/master/react-16 (npm install and npm start in that folder). If you comment out all calls to performance.* in node_modules/react-dom/cjs/react-dom.development.js), the app will be 7x times faster.

@wwwillchen

This comment has been minimized.

Show comment
Hide comment
@wwwillchen

wwwillchen Jun 12, 2017

@gaearon - I'm not familiar with the implementation of User Timing but I'll reach out to some colleagues who may know why.

wwwillchen commented Jun 12, 2017

@gaearon - I'm not familiar with the implementation of User Timing but I'll reach out to some colleagues who may know why.

@wwwillchen

This comment has been minimized.

Show comment
Hide comment
@wwwillchen

wwwillchen Jun 19, 2017

@gaearon - I talked with @ak239 who works on V8 debugging on my team and he said that the performance issue might be because it's going from Javascript to C++ to execute these methods (e.g. C++ implementation here: https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/core/v8/V8Performance.cpp?type=cs&l=279)

Will let you know if we have more details to share.

wwwillchen commented Jun 19, 2017

@gaearon - I talked with @ak239 who works on V8 debugging on my team and he said that the performance issue might be because it's going from Javascript to C++ to execute these methods (e.g. C++ implementation here: https://cs.chromium.org/chromium/src/out/Debug/gen/blink/bindings/core/v8/V8Performance.cpp?type=cs&l=279)

Will let you know if we have more details to share.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jun 19, 2017

Member

@wwwillchen Thanks for the update, it's appreciated!

Member

gaearon commented Jun 19, 2017

@wwwillchen Thanks for the update, it's appreciated!

@nitin42

This comment has been minimized.

Show comment
Hide comment
@nitin42

nitin42 Dec 10, 2017

I am working on this by extending @wwwillchen's work. I am building a chrome extension as well as an electron app similar to react-devtools.

nitin42 commented Dec 10, 2017

I am working on this by extending @wwwillchen's work. I am building a chrome extension as well as an electron app similar to react-devtools.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Dec 12, 2017

Member

It would be great if for a start it could be used just as an npm library (and print to console).

Member

gaearon commented Dec 12, 2017

It would be great if for a start it could be used just as an npm library (and print to console).

@nitin42

This comment has been minimized.

Show comment
Hide comment
@nitin42

nitin42 Dec 12, 2017

I am not sure how this would work with Node but I'll give it a try.

nitin42 commented Dec 12, 2017

I am not sure how this would work with Node but I'll give it a try.

@kelset

This comment has been minimized.

Show comment
Hide comment
@kelset

kelset Dec 18, 2017

👋 guys!
Sorry to bother y'all, was wondering if is there any update on this particular side of the perf monitoring:

We need to figure out how to keep it viable/performant for React Native which doesn’t have a native performance implementation (but I’m adding a simple polyfill soon).

Since @nitin42's take on the issue via https://github.com/nitin42/react-perf-devtool requires modifying one line on react-dom which is not viable for rn.

(possibly related: facebook/react-native#15371 )

kelset commented Dec 18, 2017

👋 guys!
Sorry to bother y'all, was wondering if is there any update on this particular side of the perf monitoring:

We need to figure out how to keep it viable/performant for React Native which doesn’t have a native performance implementation (but I’m adding a simple polyfill soon).

Since @nitin42's take on the issue via https://github.com/nitin42/react-perf-devtool requires modifying one line on react-dom which is not viable for rn.

(possibly related: facebook/react-native#15371 )

@nitin42

This comment has been minimized.

Show comment
Hide comment
@nitin42

nitin42 Dec 20, 2017

The extension is now available for Chrome and Firefox. Still needs some work though!

Chrome extension - https://chrome.google.com/webstore/detail/react-performance-devtool/fcombecpigkkfcbfaeikoeegkmkjfbfm

Firefox add-on - https://addons.mozilla.org/en-US/firefox/addon/nitin-tulswani/

nitin42 commented Dec 20, 2017

The extension is now available for Chrome and Firefox. Still needs some work though!

Chrome extension - https://chrome.google.com/webstore/detail/react-performance-devtool/fcombecpigkkfcbfaeikoeegkmkjfbfm

Firefox add-on - https://addons.mozilla.org/en-US/firefox/addon/nitin-tulswani/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment