Performance Issues

[TomYeoman]

Hi!

First off I'd like to give my thanks for creating this library, we're enjoying using it on our project.

I would like to discuss some performance issues we're having, potential fixes, and whether these are something you may consider being part of the core library offering.

Initially, I'd like to share a screenshot of our graph, so you can get an idea of what we're working with.

Now, we're facing several issues:

• Load time takes 1min+ in Firefox, and even longer in chrome.
• Connecting nodes on the edges, can take 15-20s+ on chrome (although stay within a couple of seconds in Firefox strangely).

I've boiled this down to the fact that CalculateOrder function, which runs every time we CheckConnection, is run:

• Every time we load a graph, the library attempts to re-add every node's connection and this function gets called for every node X>Y
• Every time we try to connect 2 node's (after load).

Here's what happens every time a node is connected:

• We run a DFS, with cycle check for every node with no outputs (E.G for this particular graph that means we're running a DFS 37 times across the graph once nearly every connection is added, for every output node we have)
• Once we have the full path for each output node via the DFS, run a BFS across the node's, so the baklava engine can calculate the order to run node's.

---

Suggested changes?

1. Allow the CalculateOrder check for each node to be optional when calling load, it doesn't seem like there's always a need to do that when we're already running the checks every connection we add. This change alone make's load run in <1s for FF/Chrome

2. Decouple the generation of baklava node execution order, from the cycle checking.

Myself (and perhaps others) are actually just using Baklava as a graph rendering Vue library (then using the save/load feature when done, and running the graph on the server using our own execution logic).

If we were to remove the BFS check / make it optional via some sort of opt in engine config parameter, it speeds up node connection (worse case) from ~20s to ~2s in chrome. It must be noted I am noticing weird behaviour in the fact Firefox seems to be able to handle the stack generated here of ~5mil items on some connections extremely fast, compared to chrome which completely chokes.

---

Would appreciate any thoughts you have regarding the above!

Thanks

[newcat]

I have to admit, I have never tested this lib on graphs this large. So, unsurprisingly, the very handcrafted and probably far from optimal cycle detection does cause these performance issues (although I guess there are a lot of other things that aren't really optimized for large graphs).

If you only need the cycle checking, it's probably best to not use the engine plugin and instead provide your own plugin that does the cycle check on the checkConnection event (basically this).

It's probably too late for Baklava v1, but for v2 I'll try and also consider performance on larger graphs when implementing logic like this.

[TomYeoman]

Haha yes, we are pushing it to the limit.

Thanks for the response, I agree it looks like we should just remove the engine plugin (I thought we were using it for other things, but upon second inspection it looks like it was legacy perhaps on our side).

One issue is, even if we do write our own cycle check detection plugin (which would make adding new connections much quicker), it wouldn't solve the graph load issue whereby checkConnection is called hundreds of times (on a graph we already know has no cycles).

So I still wonder whether a parameter to load would be considered, which could let consumers of checkConnection know whether it was part of a load (do nothing), or part of actually adding a new connection (do cycle check).

[TigerHix]

Myself (and perhaps others) are actually just using Baklava as a graph rendering Vue library (then using the save/load feature when done, and running the graph on the server using our own execution logic).

Second this!

[newcat]

I think I will just add a editor.loading flag, which indicates whether the editor is currently in the process of loading nodes and connections. So in your checkConnection event you can then directly return if editor.loading === true.

This way I don't have breaking API changes and it also usable in other areas.

[TomYeoman]

That would be fantastic thanks so much. I will try and contribute back a plugin which strictly does a cyclic check on connection add.

Curiously which release is this likely to be shipped under (current, or V2)?

[newcat]

This is for v1 (which I guess you are using). I will try to port all the features from v1 to v2 though.

[newcat]

Implemented in v1.10.0