Draw a single rectangle for 2 markers in bar charts only if both the …#4734
Conversation
…start and end horizontal pixels match.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4734 +/- ##
==========================================
+ Coverage 88.32% 88.34% +0.01%
==========================================
Files 300 299 -1
Lines 26733 26727 -6
Branches 7213 7214 +1
==========================================
- Hits 23612 23611 -1
+ Misses 2908 2903 -5
Partials 213 213
☔ View full report in Codecov by Sentry. |
|
I'm a bit confused: I see spikes with the deploy preview, the spikes are always thin no matter how much I zoom in around them, and I can't hover them. Is that expected? Screencast_20230901_155141.webmAnother source of confusion that is unrelated to this patch: where does the label "CPU Use" come from? I don't see it in the schema, I only see "CPU". |
|
Thanks for looking at this PR :).
It's not expected, in that it's a bug of the python code generating the profile (I've now added a workaround to remove these spikes from the generated profiles), but the profiler front-end is behaving reasonably when showing these tiny spikes (if you keep zooming, you may be able to hover them at some point, they last about 1ms when the other markers are 1 or 2s long). Without the PR, the larger markers are shown drawn at the height of these spikes, and that's what actually made me realize there was a regression in how these charts are drawn.
It's the marker name. |
julienw
left a comment
julienw
left a comment
There was a problem hiding this comment.
looks good to me, thanks
* main: (37 commits) Show the hovered time in the ruler at the top of the timeline. (PR firefox-devtools#4748) Hardcode 'project' to 'firefox-profiler' in .taskcluster.yml (PR firefox-devtools#4759) Update all Yarn dependencies (2023-09-27) (PR firefox-devtools#4758) Update the uploading command in the developer documentation (PR firefox-devtools#4752) Make prettier ignore the taskcluster files Add taskcluster Update all Yarn dependencies (2023-09-20) (PR firefox-devtools#4756) Add a robots.txt file to disallow indexing our profile links (PR firefox-devtools#4753) Avoid showing the calltree panel in profiles without samples. (PR firefox-devtools#4744) Display the vertical line in the timeline when hovering the marker chart or stack chart (PR firefox-devtools#4742) Draw a single rectangle for 2 markers in bar charts only if both the start and end horizontal pixels match. (PR firefox-devtools#4734) Update all Yarn dependencies (2023-09-13) (firefox-devtools#4746) Update all Yarn dependencies (2023-09-06) (PR firefox-devtools#4741) Remove prefer-wait-for rule in our config because it's been removed from the plugin ⬆️ Update eslint-plugin-testing-library to version 6.0.1 Use a cache for prettier (PR firefox-devtools#4731) Remove leftover eslint error Run prettier on the codebase Upgrade prettier to latest version Ignore the coverage directory when present ...
2 participants
…start and end horizontal pixels match.
This fixes a regression from #4711.
https://share.firefox.dev/3sB46FE is an example of a profile where the marker track shape is very different with/without the fix.
I also verified using console.log debugging that the big profiles in #4711 still merge some markers for drawing (ie. that the optimization still works).