vasturiano / force-graph

Force-directed graph rendered on HTML5 canvas
https://vasturiano.github.io/force-graph/example/directional-links-particles/
MIT License
1.54k stars 246 forks source link

for very large graph; saw issue where __indexColor=null on node causing error when updating graph #118

Open heatherandthewho opened 4 years ago

heatherandthewho commented 4 years ago

Describe the bug Was using 5000+ node graph; started seeing an error regarding when performing updates, __indexColor is created on the node; but null causing error logic in the code.

To Reproduce Steps to reproduce the behavior:

  1. Not easy--I have a websocket that is running to get the graph nodes--it chokes at about 5000k nodes. To get it to not do this; when updating the graph I recreate the node without the __indexColor property so that it falls through and reregisters.

The error happens here in the force-graph module, the node has a ___indexColor property, but it is null, so then the code attents to do the state.colorTracker.lookup and then you get an error generated by " _nonIterableSpread" in canvas-color-tracker.

Here is the area in force-graph, either it needs to be checking more than hasOwnProperty or check if that property is null return true; which would be good; I'm not getting an indication of why for the node it sometimes doesn't return a registered color:

function hexIndex({ type, objs }) { objs .filter(d => { if (!d.hasOwnProperty('indexColor')) return true; const cur = state.colorTracker.lookup(d.indexColor); return (!cur || !cur.hasOwnProperty('d') || cur.d !== d); }) .forEach(d => { // store object lookup color d.__indexColor = state.colorTracker.register({ type, d }); }); }

Desktop (please complete the following information):

vasturiano commented 4 years ago

@heatherandthewho thanks for reaching out.

Normally you would only reach such a state when registering more than a quarter million nodes/links, as documented in canvas-color-tracker.

Are you using more than 262,000 objects? Or alternatively, are you doing repeated calls to graphData, at every update from your web socket? If so, you may wish to keep a copy of your data before passing it to the library, and at every update "merge" the data into your structure and keep the existing object references. This would prevent it from creating new objects every time.

heatherandthewho commented 4 years ago

No; not using more than 262,000 objects. I am actually keeping the data in indexeddb; I do the merge of the data nodes with old graph to graph update using lodash, then put the changed version back in the react components state. It doesn't happen on small graphs. Just on a large one after many updates. But; it sounds like keeping the graph just in state would be better.

vasturiano commented 4 years ago

I see. Is it possible that when merging the nodes/links objects you are cloning the nodes/links (effectively generating new references) instead of just modifying the contents of the existing object? This would explain why you hit that limit after many updates, because eventually you reach 262k new nodes, even though at any given point you only have a much smaller number. Not cloning the objects would solve that.

heatherandthewho commented 4 years ago

I'll do some experimentation and do the merge differently... thanks! Great library!

heatherandthewho commented 4 years ago

I was also not recreating the React component between displays of different graphs and I think that may be part of the issue.