Optimize the performance of scatter trace sorting comparator with map #1555
Conversation
Construct a map rather than an array to improve the performance of index search. Jasmine tests passed.
hy9be
changed the title
|
Looks good to me. @rreusser can you think of any side effects (e.g. for animations)? |
| @@ -51,13 +51,13 @@ module.exports = function plot(gd, plotinfo, cdscatter, transitionOpts, makeOnCo | |||
| // Sort the traces, once created, so that the ordering is preserved even when traces | |||
| // are shown and hidden. This is needed since we're not just wiping everything out | |||
| // and recreating on every update. | |||
| for(i = 0, uids = []; i < cdscatter.length; i++) { | |||
| uids[i] = cdscatter[i][0].trace.uid; | |||
| for(i = 0, uids = {}; i < cdscatter.length; i++) { | |||
There was a problem hiding this comment.
Seems entirely equivalent to me 👍
I didn't anticipate this would be more than like 10-15 or so, which would make the overhead of indexOf negligible. I'll keep this in mind when contemplating orders of magnitude!
There was a problem hiding this comment.
Yeah, actually there is almost no difference on map and array here if the amount of trace is small. My cases might be a bit crazy (with 5000+ traces). But it seems worthwhile to use map instead here since there is almost no side effect.
|
Suggested benchmark strategy for this case (though end-to-end is relevant too): var t0 = performance.now() // if your browser supports it, otherwise Date.now()
... the above lines of code before, then after
var t1 = performance.now()
console.log('Time taken to sort:', t0 - t1)The fix seems appropriate either way (I should have known better in the first place 🙄 ), it's just very useful to have a sense for the cost and gains of optimizations like this. |
|
Here is my script. And some data points collected on my computer (time in ms): |
|
@hy9be thanks for that! |
|
By the way, @hy9be thanks for all your contributions this week! You get my vote for contributor of the week 🏆 |
|
Wow...feel flattered. Thanks! I had some exposure to the idea of grammar of graphics. Plotly is not quite an implementation of that but the design is much more orthogonal and abstract than most other charting libraries. I like the project. Will try to help more. |
|
Thanks for the benchmark! Honestly I didn't think that single call would make a big enough difference to have that obvious of an effect in an end-to-end test. 🐢 |
Construct a map rather than an array to improve the performance of index search. Jasmine tests passed.