oncojs / lolliplot

Chart to aid in understanding mutations and their location on a gene.
Apache License 2.0
22 stars 5 forks source link

Improve performance #2

Open alex-wilmer opened 7 years ago

alex-wilmer commented 7 years ago

When there are thousands of nodes the animation gets very choppy. Two things to try:

1) canvas everything

2) fiddle with requestIdleCallback to possibly let the browser try to catch up if an animation frame is taking too long

cheapsteak commented 7 years ago

Would perhaps recommend moving in stages and prioritize low-hanging gains

e.g. for dcc, it's really choking on ENSG00000011083 (single digit FPS for what seems like a minute after any user interaction)

image

Main culprit seems to be document.querySelector

Would recommend:

Update: for some reason, things slow down after a window resize

cheapsteak commented 7 years ago

Update:

Replaced all instances of d3.select and d3.selectAll with memoized function calls to

const d3Select = _.memoize(d3.select);
const d3SelectAll = _.memoize(d3.selectAll);

Significant performance boost. Still sluggish but useable

image

Went from rendering 1 frame every 1-2 seconds, to rendering 2-4 times per second

edit:

There is a major caveat with this method though, in that it assumes that the results from a selection is always the same. That means a fresh memoized function must be created for each new instance of lolliplot, and no DOM nodes can be added or removed from the graph.

May not be the best way to go then. Perhaps should still do explicit caching of elements before animation loops (would recommend passing the animation buck off to d3)

alex-wilmer commented 7 years ago

Caching the mutations was the first big performance boost I gained, went from being able to animate 400 or so mutations to about 800 without tooo much lag.

https://github.com/oncojs/lolliplot/blob/master/modules/node_modules/%40oncojs/lolliplot/animator.js#L147

but I suspect not calling select at all during the animation phase would do well to boost the performance. windowing could work too.

thanks for taking a look

alex-wilmer commented 7 years ago

Tried out canvas https://github.com/oncojs/lolliplot/tree/canvas

performance is significantly boosted. In the meantime, I realized I had a major memory leak on master.

Basically canvas, as suspected, wlll improve performance but bloat the DOM interaction logic.

shanewilson commented 7 years ago

It might be worth looking into ways to limit what you have to render - merging hits in the same location or even bucketing hits within a range depending on the zoom level.

alex-wilmer commented 7 years ago

@shanewilson I did just that over the last few days, basically implementing various clustering algorithms. using react fiber style architecture too which works pretty well (code is crazy looking though)

alex-wilmer commented 7 years ago

clustering + canvas + fiber === great performance. I think now it's just a matter of making a good design decision about how to cluster stuff.

cheapsteak commented 7 years ago

May it be prudent to investigate using D3's animations as well? Perhaps a simpler solution would yield similar performance boosts without adding to code complexity

alex-wilmer commented 7 years ago

Sure I think it's worth looking into, though I'm having a hard time imagining what d3 could do to improve the performance of animating 1000+ nodes. The simplest solution would be to lose the animation. The user would lose a little bit of context, but the minimap would help a lot for that.

I think a really cool solution that might be complex / multi-faceted is worth continuing to investigate, even if just for academic purposes.. clustering / down sampling isn't just for performance, but would give the user a better high level view of what they're looking at.

For the sake of time though, a simple solution (such as turning off animations once there are a large number of nodes) + fixing the other important issues should be prioritized.

alex-wilmer commented 7 years ago

react fiber style architecture

and by that I mean this thing I learned about when reading about fiber. there's probably an actual name for this technique (low priority mock-concurrency?)

cheapsteak commented 7 years ago

I agree that clustering might be a worthwhile feature to add :)

The reason I suggest using d3's built in animations is that, based on profiling, the current library's performance bottleneck doesn't seem to be rendering, but rather uncached selectors trying to scan the DOM however many times a second, and second to that it is the custom RAF loop somehow causing GC. I think removing our own RAF loop and using D3's transitions would bypass both problems

Perhaps moving this to canvas and using a react fiber style architecture would further enhance performance, but I worry about what internalizing both canvas interaction and an architecture that Andrew Clark found difficult to follow would do to the maintainability of the code. Perhaps it may still need to be done, but I'd recommend reevaluating the trade-offs after the low hanging bottlenecks have been fixed?

alex-wilmer commented 7 years ago

but I'd recommend reevaluating the trade-offs after the low hanging bottlenecks have been fixed?

agreed 100%. As the vacation ends as well, this becomes more important :)