Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance #2

Open
alex-wilmer opened this issue Dec 20, 2016 · 12 comments
Open

Improve performance #2

alex-wilmer opened this issue Dec 20, 2016 · 12 comments

Comments

@alex-wilmer
Copy link
Contributor

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
Copy link
Contributor

cheapsteak commented Dec 21, 2016

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:

  • using d3's animations
  • cache selectors
  • don't update things that aren't on screen (maybe you're already doing this? can't tell for sure in animator.js I see a data.proteins.forEach)

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

@cheapsteak
Copy link
Contributor

cheapsteak commented Dec 21, 2016

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor

cheapsteak commented Dec 29, 2016

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
Copy link
Contributor Author

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants