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

Re-rendering abysmally slow on big graphs (if "before graph" and "after graph" are both large) #232

Open
Domiii opened this issue Jun 13, 2022 · 4 comments

Comments

@Domiii
Copy link

Domiii commented Jun 13, 2022

We have been using d3-graphviz for a few weeks now, and it is just amazing!

However, there seems to be a really bad bug and it only happens when re-rendering, and there is already a somewhat large graph on screen. It is really fast most of the time, but if I re-render a big graph and the new graph is very similar to the old graph, things get super slow.

In one of my experiments, I got the following performance measurements:

  1. original render time: 1s
  2. Re-rendering time: 8s
  3. Re-rendering time with workaround (see below): 1s (just like the initial render)
  4. dot string is about 170k in length

It happens, regardless of whether or not I added transitions.

Workaround

It seems to have to do with the internally stored graphviz state on the element. The only workaround I found so far is to simply re-create the graph element every time, something like this:

      if (this.graphEl) {
        this.graphEl.remove();
      }
      const graphEl = this.graphEl = compileHtmlElement('<div id="timeline-graph"></div>');
      this.els.graphcont.appendChild(graphEl);
      this.graphviz = d3Graphviz.graphviz(graphEl, { ...GraphVizCfg });
      console.debug('re-initializing graph');
@Domiii Domiii changed the title Re-rendering abysmally slow on big graphs Re-rendering abysmally slow on big graphs (if "before graph" and "after graph" are very similar) Jun 14, 2022
@magjac
Copy link
Owner

magjac commented Jun 15, 2022

Thank you for the kind words 🙂.

Can you share the code for me to debug?

Have you tried if any of the tips under https://github.com/magjac/d3-graphviz#performance makes any difference?

@magjac magjac added the bug label Jun 15, 2022
@Domiii
Copy link
Author

Domiii commented Jun 16, 2022

hi @magjac

Thank you for the swift reply!

As it looks right now, adding those performance settings has helped a lot.

However, IIRC, this was an issue even without transition added to the graph.
Could it be that it does all that work even if its not being used?

For reference, this is what I added to my graphviz config:

const GraphVizCfg = {
  /**
   * Performance tweaks.
   * @see https://github.com/magjac/d3-graphviz/issues/232#issuecomment-1156834744
   * @see https://github.com/magjac/d3-graphviz#performance
   * @see https://github.com/magjac/d3-graphviz#graphviz_tweenShapes
   */
  tweenShapes: false,
  tweenPaths: false,
  // tweenPrecision: 100, // NOTE: not necessary when tweening is disabled
  // convertEqualSidedPolygons: false // NOTE: not necessary when `tweenShapes` is disabled
};

@magjac
Copy link
Owner

magjac commented Jun 20, 2022

Sorry for the delay.

Could it be that it does all that work even if its not being used?

Yes. It has to. Transitions can be applied after the data processing phase.

I actually suffered from this problem myself a few months ago. I had turned off transitions, but forgotten to disable tweening. My browser choked and I was looking for the problem in the application's processing of data from it's backend for several days before realizing that it was the d3-graphviz tweening calculations that were the problem.

tweenPrecision: 100,

This shouldn't be necessary when tweening is not enabled.

convertEqualSidedPolygons: false

This shouldn't be necessary when shape tweening is not enabled.

@Domiii
Copy link
Author

Domiii commented Jun 21, 2022

@magjac, thanks again for the great help!

My 2 cents:

Considering that you yourself got caught in this "performance trap", it is reasonable to assume that many others will, too? 🤷

Going below some acceptable performance threshold is a high crime, and sometimes a deadly sin for real-time applications 😭

I thus strongly recommend changing the default configuration, or maybe adding a warning that appears in development mode to help developers quickly get this fixed when necessary? What do you think?

@magjac magjac added enhancement and removed bug labels Aug 13, 2022
@Domiii Domiii changed the title Re-rendering abysmally slow on big graphs (if "before graph" and "after graph" are very similar) Re-rendering abysmally slow on big graphs (if "before graph" and "after graph" are both large) Mar 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants