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

Improving speed by batching dom reads and writes #394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ambischof
Copy link

Background

My team has been using dagre-d3 for a while and we were noticing that the rendering time took forever on some of our larger graphs.

Issue and Solution

In investigating, I found it was due to constant layout reflows. For every node, it reads and writes to the dom several times, each time you go from reading DOM (mostly getting bounding box) to writing DOM, it forces the layout to redraw.

What I did was separate out the loops between reading DOM for all nodes and writing DOM for all nodes.

fiddle

I have made a fiddle here that compares the original version and my version. (toggle the comments in html portion to switch between my version and original)

It turns out in the fiddle, it only saves about 200ms (from about 800ms to 600ms) however in the environment we're using it in the app where the surrounding layout is quite complicated it went from like 30 seconds to less than a second. I had a hard time replicating that in the fiddle.

This is a screeenshot of the original code's performance analysis using chrome's dev tools. Notice all the forced reflows
Screen Shot 2020-02-19 at 12 41 38 PM

This a screenshot from my fork, of a similar period:
Screen Shot 2020-02-19 at 12 57 30 PM

Comments

This PR was put together somewhat hastily using an old version of dagre d3 (4.1.18) because that's what my team uses right now.
It was intended to be a fork that our team uses, but since this would help anyone, I decided to make a PR. I admittedly didn't have a lot of time to updated it to the latest version, but I did find that the code hasn't changed much between v4 and master.

Even if you don't use this PR exactly, I hope you at least consider refactoring to achieve the general concept, because this will help out in a lot of situations.

If you are interested in using this PR but need it in a better/different state, please let me know!

@kaelhem
Copy link

kaelhem commented Jun 3, 2020

Hi,
I was interested in this PR because I encountered some perf issues in my use case.
So, I've refactored it to be able to merge it in the master branch.
But, unfortunately, it seems there is no gain (for my own use case, once again).

If someone wants to give it a try, my branch is here :
https://github.com/kaelhem/dagre-d3/tree/improve_speed_uptodate

cheers !

@tbo47
Copy link

tbo47 commented Jan 6, 2022

I ported the project to es6 and I accept contributions!
https://github.com/tbo47/dagre-es

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

Successfully merging this pull request may close these issues.

3 participants