magjac / d3-graphviz

Graphviz DOT rendering and animated transitions using D3
BSD 3-Clause "New" or "Revised" License
1.69k stars 103 forks source link

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

Open Domiii opened 2 years ago

Domiii commented 2 years ago

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');
magjac commented 2 years ago

Thank you for the kind words :slightly_smiling_face:.

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?

Domiii commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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?