atom / github

:octocat: Git and GitHub integration for Atom
https://github.atom.io
MIT License
1.12k stars 393 forks source link

measure performance of diff rendering #1781

Open annthurium opened 5 years ago

annthurium commented 5 years ago

We know that the changes in https://github.com/atom/github/pull/1512 improved performance, and we'd like to collect some data to prove it. Especially since we are building https://github.com/atom/github/pull/1767 which takes advantage of these improvements to show multiple diffs in the same pane.

Instrumenting the react lifecycle methods won't tell us much, because the real improvements are in painting and layout calculations, rather than DOM updates.

Let's:

annthurium commented 5 years ago

hi @maxbrunsfeld !

Since you are a performance expert, I'd love to get your perspective here.

I found some articles on how to use the flamegraph / devtools to diagnose and fix performance bottlenecks with painting and layout calculations. My questions for you are:

I didn't find much documentation on performance measurement for Electron apps, but if I'm missing something please do point me there.

Thanks!

maxbrunsfeld commented 5 years ago

Is it possible to instrument these APIs or observe events or something so we can collect performance data with our usage metrics and have a performance dashboard for Atom?

I think that for the purposes of a dashboard, you wouldn't necessarily want to use the dev-tools or flame graphs. Flame graphs contain a very large amount of fine-grained data, so they're great tools for understanding the source of performance problems once you're able to repro them, but there would probably be privacy and bandwidth concerns if you were going to send them over the wire in an automated way.

For the purposes of telemetry and dashboards, I would think you'd just want to collect a few specific time durations (e.g. how long does it take to call render on this component), which you could compute manually by calling performance.now() before and after. Does that make sense?

smashwilson commented 5 years ago

@maxbrunsfeld One problem I've had with recording durations around a React render() call is that it doesn't seem to include the layout and reflow time that happens as a result of the DOM manipulation - which can result in multi-second UI lags in the MultiFilePatchView. I was hoping you know of some way to hook into the Chrome performance tooling to capture that data as well, so we'd have a more accurate view of the user's experienced performance.

For example, here's a timeline from #1767 before we refactored the way we were using TextEditors:

screen shot 2018-11-13 at 11 15 47 am

And after:

screen shot 2018-11-13 at 11 18 08 am