Draw bar charts in custom marker tracks faster.#4711
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #4711 +/- ##
=======================================
Coverage 88.33% 88.34%
=======================================
Files 299 299
Lines 26714 26720 +6
Branches 7208 7210 +2
=======================================
+ Hits 23599 23605 +6
Misses 2902 2902
Partials 213 213
☔ View full report in Codecov by Sentry. |
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
It's a very big difference in the performance, thanks!
Looks good to me with the Math.max/min comment I mentioned below.
| ) | ||
| : x + 1; | ||
|
|
||
| ctx.fillRect(x, y, x2 - x, deviceHeight - y); |
There was a problem hiding this comment.
Nice improvement, thanks for investigating this!
| } | ||
|
|
||
| marker = nextMarker; | ||
| y = Math.min(y, getY(++i)); |
There was a problem hiding this comment.
Don't we want to get the max value with the Math.max here instead of min?
There was a problem hiding this comment.
We want the max value, which (unless I'm confused) is the min y (as y = 0 would mean top of the canvas).
There was a problem hiding this comment.
Oh welp right, I was the one who's confused not you. Sorry about the confusion!
fqueze
force-pushed
the
marker-track-speed
branch
from
462828b to
31fdfe7
Compare
2 participants
Short example profile (about 3k samples) : https://deploy-preview-4711--perf-html.netlify.app/public/kgqe66dn8k0r2k89kbsrxxp7tv1fbahfcgp722r/ (compare with https://main--perf-html.netlify.app/public/kgqe66dn8k0r2k89kbsrxxp7tv1fbahfcgp722r)
Longer example profile (22.5k samples) : https://deploy-preview-4711--perf-html.netlify.app/public/mgtzdc7myr2j56hs2jjnrg68fmsm36g1az8cgx0 (hangs the current main--perf-html.netlify.app; try at your own risk :-)).
The first commit has the most impact here: drawing 1px rectangles is much faster than drawing a line and calling stroke. The second commit saves some redundant work by reducing the number of rectangles when more than one would be starting on the same horizontal pixel.
I only fixed bar charts because it's an easier subset of the problem, and their performance issues will become very noticeable if we land CPU frequency recording and people try playing with that new experimental feature.
It should be possible to also improve the performance of drawing line charts with an approach similar to #4636, but I would prefer to do that in a separate PR to ensure the bar chart perf improvement becomes available sooner.