More optimization for cartesian subplots #2487
Conversation
- this can speed up Axes.doTicks by 50ms on 50x50 subplots grids
- so that drawTitle() can reuse it - this can speed up doTicks by 200ms at 50x50 subplots
|
I was hoping to improve a few more things in this PR, namely:
but I'll have to leave that for later. @alexcjohnson would mind taking a 👀 at this? |
| // draw ticks, titles, and calculate axis scaling (._b, ._m) | ||
| function drawAxes() { | ||
| return Axes.doTicks(gd, 'redraw'); | ||
| return Axes.doTicks(gd, graphWasEmpty ? '' : 'redraw'); |
There was a problem hiding this comment.
OK... this is the only caller using doTicks(gd, '') right? From the logic below it looks like there may have been one previously (or someone just calling doTicks(gd)) but I don't see it now. I'm a little reticent to add new branches inside doTicks when what we really need to do is unpack it into more digestible pieces - but that's probably a project for another time refactoring all of axes.js.
There was a problem hiding this comment.
OK... this is the only caller using doTicks(gd, '') right? From the logic below it looks like there may have been one previously (or someone just calling doTicks(gd)) but I don't see it now.
That's correct.
src/plot_api/subroutines.js
Outdated
| plotgroupBg = plotinfo.bg = Lib.ensureSingle(plotgroup, 'rect', 'bg'); | ||
| pgNode.insertBefore(plotgroupBg.node(), pgNode.childNodes[0]); | ||
| } else { | ||
| plotgroupBg = plotgroup.select('rect,bg'); |
There was a problem hiding this comment.
Nice catch. I guess that means we have some untested code branches in there 😡
There was a problem hiding this comment.
we have some untested code branches in there
maybe, maybe not. rect,bg would delete all rect elements (are there any others? if not, then what you have there will work, it just won't be as efficient) and all bg elements (which don't exist)
There was a problem hiding this comment.
😌 yeah you're right,
this thing
src/plot_api/subroutines.js
Outdated
| pgNode.insertBefore(plotgroupBg.node(), pgNode.childNodes[0]); | ||
| } else { | ||
| plotgroupBg = plotgroup.select('rect,bg'); | ||
| if(plotgroupBg.size()) plotgroupBg.remove(); |
There was a problem hiding this comment.
does this if make a difference? Does it take a non-negligible time to remove an empty selection?
There was a problem hiding this comment.
that if did not improve things -> 🔪 in 5307125
| var avoidSelection = d3.select(gd).selectAll('g.' + axid + 'tick'); | ||
| var avoid = { | ||
| selection: avoidSelection, | ||
| selection: tickLabels, |
|
Fantastic- really whittling down the overhead! 💃 🐎 |
to be more merged in #2474
In progress for now, as I'll try to speed up a few more things in the coming days.