Closed taneliang closed 4 years ago
This pull request is being automatically deployed with Vercel (learn more). To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/mlh-fellowship/scheduling-profiler-prototype/4k9fuz7p1 ✅ Preview: https://scheduling-profiler-prototype-git-eliang-flamechart-colors.mlh-fellowship.vercel.app
@bvaughn, what do you think about having flamegraph stack frames colored like in Chrome's performance tab? :D The colors are generated deterministically based on the script URL.
@taneliang I based the current coloring off of the new Mozilla profiler because I like its UI better, but I'm not opposed to trying different ones. Show me? :)
I guess you already did, in the screenshot. I like it fine! Go for it.
@bvaughn
I based the current coloring off of the new Mozilla profiler
Ah! I didn't realize that. This script URL-based coloring is something that I find really useful in Chrome's profiler, and sadly it's missing from Firefox's. Let's see what users think of this.
~Show me?~
Oops, should've mentioned the screenshots. There's also a Vercel deploy preview, which also includes flamechart hovering optimizations - we have 60FPS flamechart hovers now 🎉
I'll try to get this landed today so that all these improvements are deployed to prod.
As discussed earlier on Discord, I can't repro this on Chrome or Firefox on Windows. Can you check if you can repro this in #93's deploy preview? The changes in this PR don't seem to be able to cause the behavior you're describing.
Okay I realized you probably have your browser zoomed in. I can repro it now in #93 once I zoom in. Let's move the discussion there since it's unrelated to this PR.
Okay I realized you probably have your browser zoomed in. I can repro it now in #93 once I zoom in. Let's move the discussion there since it's unrelated to this PR.
It isn't zoomed in and yeah, I can repro it in #93 too. Let's discuss this there.
Ah! I didn't realize that. This script URL-based coloring is something that I find really useful in Chrome's profiler, and sadly it's missing from Firefox's. Let's see what users think of this.
Cool! I dig it.
Oops, should've mentioned the screenshots. There's also a Vercel deploy preview, which also includes flamechart hovering optimizations - we have 60FPS flamechart hovers now 🎉
Exciting! Great work :smile:
I am seeing some lag spikes while scrolling on Firefox but not when I open the same profile on Chrome, can you can repro it on your machine too?
I'm not sure what you're referring to. Is this a performance degradation that's this related to this PR? This PR should not have impacted scrolling performance. The PR stack that this PR is based on also did not improve scrolling performance noticably as it still required a full canvas render.
I'm not sure what you're referring to. Is this a performance degradation that's this related to this PR? This PR should not have impacted scrolling performance. The PR stack that this PR is based on also did not improve scrolling performance noticably as it still required a full canvas render.
It shouldn't be related to this but I only discovered it in this PR. Did you face it too on Firefox or is it related to my system only?
I profiled both the current production app and the deploy preview for this PR in Firefox, and I don't think there's any difference in scrolling performance. I also don't feel any difference between Firefox and Chrome.
Summary
Adapt Chromium performance tab's color generation code to our flamechart.
Hovers are implemented by decreasing the original colors' luminosity by 5%.
Depends on #93.
Test Plan
yarn test
: tests written for new codeyarn lint
,yarn flow
)yarn start
. Screenshots:Before:
After:
Comparison with performance tab: