retejs / rete

JavaScript framework for visual programming
https://retejs.org
MIT License
10.17k stars 653 forks source link

`node.update()` is very slow in larger nets (~50 nodes) and causes Chrome to freeze for a long time (> 1 second) #589

Closed ghost closed 1 year ago

ghost commented 2 years ago

Initially I thought this was a problem in the react render, but it also turned out to be a problem in the angular renderer; so my assumption is that the rete core is responsible.

My machine is an MacBook Pro (16-inch, 2021) with M1 Max running MacOS 12.6 (21G115). Problematic browser is Google Chrome Version 107.0.5304.87 (Official Build) (arm64).

In either of the following 2 samples clicking a node with left-mousebutton in Chrome will freeze the browser for upwards of 1 second.

In my application, I also render the profiling information (how long each node ran) in each node, and that also takes more than a second to update; I found node.update() (which is supposed to trigger the re-render) to be the culprit.

Performance seems to be much better in Firefox 106.0.2 (64-bit), however, if you increase the number of nodes from 50 to 200, then it also starts to become a problem (although not as severely as in Chrome, where it makes rete.js unusable).

Click here to unfold the reproduction cases, based on original rete samples --- ### Code patch for https://codesandbox.io/s/rete-js-react-render-forked-rqbb67?file=/src/rete.jsx:0-4316 [(Forked code is here; you can also just copy the code manually)](https://codesandbox.io/s/rete-js-react-render-forked-k1wh11?file=/src/rete.jsx) ```jsx import React, { useState, useEffect, useCallback, useRef } from "react"; import Rete from "rete"; import { createRoot } from "react-dom/client"; import ReactRenderPlugin from "rete-react-render-plugin"; import ConnectionPlugin from "rete-connection-plugin"; import AreaPlugin from "rete-area-plugin"; import Context from "efficy-rete-context-menu-plugin"; import { MyNode } from "./MyNode"; var numSocket = new Rete.Socket("Number value"); class NumControl extends Rete.Control { static component = ({ value, onChange }) => ( { ref && ref.addEventListener("pointerdown", (e) => e.stopPropagation()); }} onChange={(e) => onChange(+e.target.value)} /> ); constructor(emitter, key, node, readonly = false) { super(key); this.emitter = emitter; this.key = key; this.component = NumControl.component; const initial = node.data[key] || 0; node.data[key] = initial; this.props = { readonly, value: initial, onChange: (v) => { this.setValue(v); this.emitter.trigger("process"); } }; } setValue(val) { this.props.value = val; this.putData(this.key, val); this.update(); } } class NumComponent extends Rete.Component { constructor() { super("Number"); } builder(node) { var out1 = new Rete.Output("num", "Number", numSocket); var ctrl = new NumControl(this.editor, "num", node); return node.addControl(ctrl).addOutput(out1); } worker(node, inputs, outputs) { outputs["num"] = node.data.num; } } class AddComponent extends Rete.Component { constructor() { super("Add"); this.data.component = MyNode; // optional } builder(node) { var inp1 = new Rete.Input("num1", "Number", numSocket); var inp2 = new Rete.Input("num2", "Number2", numSocket); var out = new Rete.Output("num", "Number", numSocket); inp1.addControl(new NumControl(this.editor, "num1", node)); inp2.addControl(new NumControl(this.editor, "num2", node)); return node .addInput(inp1) .addInput(inp2) .addControl(new NumControl(this.editor, "preview", node, true)) .addOutput(out); } worker(node, inputs, outputs) { var n1 = inputs["num1"].length ? inputs["num1"][0] : node.data.num1; var n2 = inputs["num2"].length ? inputs["num2"][0] : node.data.num2; var sum = n1 + n2; this.editor.nodes .find((n) => n.id == node.id) .controls.get("preview") .setValue(sum); outputs["num"] = sum; } } export async function createEditor(container) { var components = [new NumComponent(), new AddComponent()]; var editor = new Rete.NodeEditor("demo@0.1.0", container); editor.use(ConnectionPlugin); editor.use(ReactRenderPlugin, { createRoot }); editor.use(Context); var engine = new Rete.Engine("demo@0.1.0"); components.map((c) => { editor.register(c); engine.register(c); }); var n1 = await components[0].createNode({ num: 2 }); var n2 = await components[0].createNode({ num: 3 }); n1.position = [80, 200]; n2.position = [80, 400]; editor.addNode(n1); editor.addNode(n2); for (let i = 0; i < 50; i++) { var add = await components[1].createNode(); add.position = [500 + i * 4, 240]; editor.addNode(add); editor.connect(n1.outputs.get("num"), add.inputs.get("num1")); editor.connect(n2.outputs.get("num"), add.inputs.get("num2")); } editor.on( "process nodecreated noderemoved connectioncreated connectionremoved", async () => { console.log("process"); await engine.abort(); await engine.process(editor.toJSON()); } ); editor.view.resize(); editor.trigger("process"); AreaPlugin.zoomAt(editor, editor.nodes); return editor; } export function useRete() { const [container, setContainer] = useState(null); const editorRef = useRef(); useEffect(() => { if (container) { createEditor(container).then((value) => { console.log("created"); editorRef.current = value; }); } }, [container]); useEffect(() => { return () => { if (editorRef.current) { console.log("destroy"); editorRef.current.destroy(); } }; }, []); return [setContainer]; } ``` ### Code patch for https://codepen.io/Ni55aN/pen/xzgQYq (Forking requires an account, so you'll have to replace the code manually) ```jsx var numSocket = new Rete.Socket('Number value'); var VueNumControl = { props: ['readonly', 'emitter', 'ikey', 'getData', 'putData'], template: '', data() { return { value: 0, } }, methods: { change(e){ this.value = +e.target.value; this.update(); }, update() { if (this.ikey) this.putData(this.ikey, this.value) this.emitter.trigger('process'); } }, mounted() { this.value = this.getData(this.ikey); } } class NumControl extends Rete.Control { constructor(emitter, key, readonly) { super(key); this.component = VueNumControl; this.props = { emitter, ikey: key, readonly }; } setValue(val) { this.vueContext.value = val; } } class NumComponent extends Rete.Component { constructor(){ super("Number"); } builder(node) { var out1 = new Rete.Output('num', "Number", numSocket); return node.addControl(new NumControl(this.editor, 'num')).addOutput(out1); } worker(node, inputs, outputs) { outputs['num'] = node.data.num; } } class AddComponent extends Rete.Component { constructor(){ super("Add"); } builder(node) { var inp1 = new Rete.Input('num',"Number", numSocket); var inp2 = new Rete.Input('num2', "Number2", numSocket); var out = new Rete.Output('num', "Number", numSocket); inp1.addControl(new NumControl(this.editor, 'num')) inp2.addControl(new NumControl(this.editor, 'num2')) return node .addInput(inp1) .addInput(inp2) .addControl(new NumControl(this.editor, 'preview', true)) .addOutput(out); } worker(node, inputs, outputs) { var n1 = inputs['num'].length?inputs['num'][0]:node.data.num1; var n2 = inputs['num2'].length?inputs['num2'][0]:node.data.num2; var sum = n1 + n2; this.editor.nodes.find(n => n.id == node.id).controls.get('preview').setValue(sum); outputs['num'] = sum; } } (async () => { var container = document.querySelector('#rete'); var components = [new NumComponent(), new AddComponent()]; var editor = new Rete.NodeEditor('demo@0.1.0', container); editor.use(ConnectionPlugin.default); editor.use(VueRenderPlugin.default); editor.use(ContextMenuPlugin.default); editor.use(AreaPlugin); editor.use(CommentPlugin.default); editor.use(HistoryPlugin); editor.use(ConnectionMasteryPlugin.default); var engine = new Rete.Engine('demo@0.1.0'); components.map(c => { editor.register(c); engine.register(c); }); var n1 = await components[0].createNode({num: 2}); var n2 = await components[0].createNode({num: 0}); n1.position = [80, 200]; n2.position = [80, 400]; editor.addNode(n1); editor.addNode(n2); for (let i = 0; i < 50; i++) { var add = await components[1].createNode(); add.position = [500 + i * 4, 240]; editor.addNode(add); editor.connect(n1.outputs.get('num'), add.inputs.get('num')); editor.connect(n2.outputs.get('num'), add.inputs.get('num2')); } editor.on('process nodecreated noderemoved connectioncreated connectionremoved', async () => { console.log('process'); await engine.abort(); await engine.process(editor.toJSON()); }); editor.view.resize(); AreaPlugin.zoomAt(editor); editor.trigger('process'); })(); ```

Random Notes

uto-j commented 1 year ago

In chrome, I'm having a silmiler problem too. I agree with the assumption that rete core's problem.

My tree is built with 18 nodes, 93 sockets and 55 connections, all sockets type are same.

cap362 cap364 cap365

cap366

Is the filter of relatedConnections working correctly? Can I suppress unnecessary 'rendersocket' events?

uto-j commented 1 year ago

This might be related to #586. As in #586, this issue did not occur with rete v1.4.8 (in my case).

Ni55aN commented 1 year ago

can you reproduce it on 1.5.0-rc2?

Ni55aN commented 1 year ago

Is the filter of relatedConnections working correctly?

@uto-j btw, no. It was fixed in 1.5.0-rc2

uto-j commented 1 year ago

@uto-j btw, no. It was fixed in 1.5.0-rc2

@Ni55aN Thanks for the fix. The problem has mostly improved.

  • When I click (MouseDown) a node, constructor of SocketView is called 93 times (number of sockets), then trigger 'rendersocket' event.

Do you have any plans to improve this?

Now ConnectionView.update() is called 55 (connections) * 2 (input/output sockets) = 110 times. Processing time reduced from 2+ seconds to 55ms.

cap367

(Sorry for my bad English.)

Ni55aN commented 1 year ago

@uto-j fixed in rete@1.5.0 and rete-vue-render-plugin@0.5.2 Example: https://codepen.io/Ni55aN/pen/poZWROg

Vue render plugin affected all the nodes on nodeselected event that caused socket updates. Let me know if somebody else faced a similar problem, but use angular or react render plugin

uto-j commented 1 year ago

@Ni55aN Thank you for your reply. Sorry for my lack of explanation. I've been using rete-react-render-plugin@0.3.0. I updated rete from 1.5.0-rc2 to 1.5.0. It's still same as previous comment.

Ni55aN commented 1 year ago

@uto-j fixed in v0.3.1 https://codepen.io/Ni55aN/pen/NWBwqmz?editors=0010

uto-j commented 1 year ago

@Ni55aN Amazing! Everything works as expected. Thanks a lot.

Hatead1 commented 1 year ago

@uto-j fixed in rete@1.5.0 and rete-vue-render-plugin@0.5.2 Example: https://codepen.io/Ni55aN/pen/poZWROg

I modified this example a little and everything began to slow down when removing/adding connection. https://codepen.io/hatead1/pen/rNrbvPO

Ni55aN commented 1 year ago

@Hatead1 the graph is too large for being quickly processed by Engine. I think that because of parallel output connections, bidirectional processing (forwardProcess) traverses the nodes too many times.

In general, this is an interesting case, I will check how it will work in v2

Hatead1 commented 1 year ago

the graph is too large

20 nodes?

Ni55aN commented 1 year ago

@Hatead1 as you can see, recursion detection is harmful in this case (or at least it doesn't work optimally) image

In general, this is an interesting case, I will check how it will work in v2

In fact, this is the answer, cause v2 doesn't have recursion detection

Hatead1 commented 1 year ago

Looking forward to v2 :)

ghost commented 1 year ago

Can this fix/fixes be backported to rete 1.4.x?

I'd like to deploy a product which is affected by this, but using rete 1.5.x requires changes to our CI and package configs, because the plugins aren't available for it yet.

Ni55aN commented 1 year ago

@JannikGM published v1.4.10 with changes from the branch v1.4.x

rete-js[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.