Stress test performance

[alexkuz]

Hi! I've noticed that stress test performance in the examples is quite poor (10x10 is really not a very big graph). I think it could be fixed with these improvements:

• batch updateNodeDimensions calls on initial render (this seems to be a bottleneck)
• render only nodes in view (given that they already use ResizeObserver, it shouldn't be too hard)

I've already done the same performance patch for https://github.com/MrBlenny/react-flow-chart a while ago (https://github.com/MrBlenny/react-flow-chart/pull/7), if you are interested, I can try to make a similar PR for this project (I hope this one will have a better maintenance in future)

[moklick]

Hey @alexkuz

thanks for your input!

We are already only rendering elements that are inside the viewport but the idea to batch updateNodeDimensions calls sounds very interesting! We would be happy if you open a PR or point us into a direction so that we can implement the improvements on our own.

[alexkuz]

I would try to do something like this:

• call queueNodeDimensionsUpdate that would collect all updates in a local array instead of immediately updating state.elements
• call commitNodeDimensionsUpdates somewhere in parent node (in useEffect) that would process these updates at once.

We are already only rendering elements that are inside the viewport

Great! Although when I tried 100x100 graph with dimention updates turned off I've got Maximum call stack size exceeded, so there still must be some redundant components rendered (or just some code executed, not sure).

[alexkuz]

Also I would check if using state.elements.forEach (instead of for) is affecting performance for 10k-sized element arrays (probably not, it shouldn't really, but still worth trying)

[moklick]

I like the idea of queueing updates but I am not aware of this pattern in a react context. When would we commit the updates? In a debounced function? Every x milliseconds?

[alexkuz]

I think committing in useEffect in parent component should work, I believe it is called after children's useEffect. I never tried it this way though tbh, not sure if that's reliable. (one caveat is to make sure parent's useEffect is called when any of the children's useEffect is triggered) But yeah, if everything else failed, debounced function should do the trick.

[alexkuz]

I've played around it a bit more and found out that:

• since node width and height are unknown initially, getNodesInside always return all nodes on the first run. Maybe use some sensible default values here for width/height (or have some prop like maxNodeSize) to filter out at least the most of the nodes that shouldn't be rendered?
• call stack error I've mentioned was caused by MiniMap (React Dev Tools couldn't handle it). Since it is basically a static image, I would consider using canvas instead of SVG there (or render SVG without using React)

[moklick]

Hey @alexkuz

could you check out the next branch or npm i react-flow-renderer@next? We are now using one Resize Observer for all nodes and batch update the changes that come from the observer.

[alexkuz]

@moklick seems like there's still a problem on initial render - update is called for each node: https://github.com/wbkd/react-flow/blob/1afa00a428cc90fe5176d53cf7d1387c42a5e124/src/components/Nodes/wrapNode.tsx#L161-L165 (to reproduce, I'm using "Stress" example with 30x30 graph and fitView disabled)

const onLoad = (reactFlowInstance) => {
  // reactFlowInstance.fitView();
  console.log(reactFlowInstance.getElements());
};

const initialElements = getElements(30, 30);

[moklick]

The update is called for each node, but React should do the batch update here since it's inside a useEffect. As you can see the NodeRenderer only gets re-rendered once.

[alexkuz]

You're probably right here, but there's still a problem, since 20x20 grid takes like 5 seconds to render (in dev mode). The only sensible way I can think of to fix this is to introduce getNodeMaxSize prop (as I mentioned earlier), where user could provide the limits for node size, so there's no need to render nodes to figure out which of them shouldn't be rendered. Here's a crude example with 1000x1000 nodes: https://wt6f1.csb.app/

[moklick]

The codesandbox seems to be the default codesandbox.

Since react flow is not canvas/webgl based it's meant to be used for smaller/midsize flows. Everything above a few hundred nodes (depending how complex the nodes are) will be hard to handle I guess.

[alexkuz]

Yeah, sorry about that, I'm not used to csb, for some reason it didn't save it. It was just a demonstration that without relying on DOM to calculate which nodes should be rendered it works pretty smooth even for 1M nodes (given that we're not trying to fit them all in one screen of course).

I think it would be possible to handle relatively big flows with some constraints, but I get it if that's not a priority for this project. The thing is, though, that even with few hundred nodes (which is the actual order of magnitude that is required for me) this library is hardly usable right now, unfortunately.

[moklick]

And this would be better if we didn't have to render the nodes outside the viewport at the beginning?

[alexkuz]

I believe so - it would help with the initial render at least. Also, the graph updates (like dragging a node) are probably affected by using of Immer, which is not very effective comparing with native updates, but that's my speculation.

[moklick]

I would like to work on the dragging performance but we would need a good way to measure the performance of specific user actions automatically. Do you know if this is possible in some way?

[alexkuz]

Hm, that's an interesting question. Maybe something like this will do https://github.com/keiya01/react-performance-testing - but I just googled that, I'm not really familiar with performance testing.

[moklick]

thanks! I will have a look.

[dsebastian]

+1 on this issue. 100 nodes is just too few unfortunately. Otherwise, it looks like a great library when a small number of diagrams is needed.

Is there any plan/priority for the performance improvements mentioned above? Thanks.

[moklick]

Hey @dsebastian

we are working on a next version without easy-peasy and immer. With that version we have more control and can improve the performance more easily. If you or @alexkuz could help us with this TypeScript issue it would be a big step towards the next version.

[alexkuz]

I'll take a look. BTW have you considered using recoil or just react context? (I have nothing against redux, just wondering, although I'm moving away from redux in my projects)

[moklick]

We wanted to do a next version without or at least with few as possible breaking changes. Since easy-peasy uses redux we only have to make a few adjustments and the api can stay the same. Maybe for v10.0.0 we could think about using recoil or something similar.

[moklick]

Hey @alexkuz

the new v9.0.0 doesn't use easy-peasy (and immer) anymore. The initial rendering is much faster now. Especially when you have a lot of nodes you can see the difference.

[alexkuz]

Thanks @moklick, much faster indeed!

Now, what would be even better is to add maxNodeSize prop or function as I mentioned previously :) Here's a crude implementation (I didn't fix MiniMap there):

https://github.com/alexkuz/react-flow/commit/91ba0f2218acbbc80742d1dfe70ce0b2f5bc3b6d

This would make react-flow much more scalable I believe

[moklick]

We could also give the user to specify the width and height upfront. If that would be the case we could use these values for initialisation. What do you think about that?

[alexkuz]

This could work too, but I think setting a limit instead of exact size would be more flexible. In my case, nodes have variable height that depends on its content and which I don't know beforehand. What I do know is their max height. But if these values would be updated after the actual node render instead of been fixed forever, that would be good enough I think.

[moklick]

The width and height get updated when the resize observer detects a change. What do you think about the node options initWidht/initHeight or defaultWidth/defaultHeight?

[mlguy123]

@moklick @alexkuz Sorry for the interruption. I'd like to ask, are there any chances that the elements state be implemented in type of Object? Wouldn't it be a lot faster updating elements when there are thousands of nodes and edges?

const elements = {
    [nodeId]: {
        ...
    },
    ...
    [edgeId]: {
        ...
    },
    ...
}

[alexkuz]

@mlguy123 indeed, react-flow-chart uses this format, but to be honest, array feels more appropriate to me (it's a bit easier to deal with when feeding your data to lib) and it's not obvious if that would in fact affect performance noticeably. Worth investigating though. (I did a simple vanilla example with a million of draggable nodes, without this lib, and it worked quite good with an array, though I made sure it wasn't actually updated until node dragging stopped)

[mlguy123]

@alexkuz In my current project I have to update the elements very frequently. That's why I think making the elements state in Object would make a huge difference. I've looked into react-flow-chart couple of minutes ago, and that's exactly what I meant, I thought it has to be in a strictly order from 1,2,3 initially. But that library hasn't been updated for a while, I hope to stick to this one which is actively maintained.

[moklick]

@mlguy123 let's discuss it in this issue: #743

[apoutchika]

Hi, Still no solution to this performance problem? Since then, have you found any alternatives?

[chrtze]

@alexkuz, did you try upgrading to v10 already? We have rewritten the state management as well. Does this problem persist?

[alexkuz]

@chrtze yes, seems like it is much faster now, thanks!

btw I've experimented with using canvas instead of svg for minimap some time ago which can also improve performance a bit (not much though)

[Okdesk]

@alexkuz, have you had an opportunity to do another stress test on the upgraded version?

[Pavan-Chavan]

what's the max node render limit for "11.9.4" ??? @alexkuz @moklick

[moklick]

It depends on the machine you are using to render the flow. Normally rendering dozens is always fine, hundreds is OK and thousands is too much. React Flow wasn't built for visualizations but for editors. If you want to render visualizations, I would use a canvas based library. We created a list here: https://github.com/xyflow/awesome-node-based-uis#renderers

[Pavan-Chavan]

Thank you so much for kind response @moklick

[Pavan-Chavan]

on a viewport we have N amout of nodes, now if user zooms in on that viewport, can we render more nodes on zoom dynamically ?? @alexkuz @moklick