bartbutenaers / node-red-contrib-ui-svg

A Node-RED widget node to show interactive SVG (vector graphics) in the dashboard
Apache License 2.0
93 stars 27 forks source link

Too slow editing/saving node with huge SVG #50

Closed andreypopov closed 4 years ago

andreypopov commented 4 years ago

my svg node is 100kb+ try to import, edit and save. We need some optimization here.. I use the latest macbook pro 2019/ 64gb ram

andreypopov commented 4 years ago

svg_huge.json.zip

2020-03-05_09-59-23 Thank you for this amazing node.

andreypopov commented 4 years ago

Just noticed ver2.0 description, I have to try it how to install it?

bartbutenaers commented 4 years ago

Hi Andrey, Damn yes that is slow. At first sight I have no idea what could cause the problem... Will need to investigate that, but I'm a bit overloaded at the moment.

The 2.0.0 version is not released yet on NPM due to the lack of time. You need to install it directly from my Github account:

npm install bartbutenaers/node-red-contrib-ui-svg#panzoom

A question about your SVG:

<image width="1704px" height="695" id="background" xlink:href="/SVG.png" />

Could you explain where your png image is located, and how you expose it to the dashboard this way? Was looking myself for a secure way to use private images inside my svg...

Bart

bartbutenaers commented 4 years ago

P.S. The 2.0 version contains a breaking change unfortunately. Please have a look at the start of the readme page for more information.

andreypopov commented 4 years ago

Hi, I checked code and found super slow place. Speed improved from 15 seconds to 2-3 sec and there are no render lags.

TypedInput is slow. You can try with these changes. 2020-03-11_10-01-22

Do you have an idea how to change typedInput field and save payloadType functionality?

andreypopov commented 4 years ago

The second slow place is - autocompletes. I suggest to add new option "disable autocompletes" for performance.

2020-03-11_14-04-20

bartbutenaers commented 4 years ago

Hi Andrey, I REALLY appreciate the fact that you have analysed it by yourself!!! This is a tremendous help... But at the moment I'm recovering from a surgery, so will try to review your solution as soon as possible! Will keep you updated, Bart

bartbutenaers commented 4 years ago

Morning @andreypopov,

Sorry for keep you waiting! It is very helpful that you already spotted the two performance bottlenecks!!!

But before I merge your pull request, I would first like to try whether we can make it faster. Just to be able to keep the same functionality for large SVG's. If that fails I will have no other choice then using your checkboxes...

P.S. would you be so kind to respond to another issue please. Perhaps you can explain there how you use your local image path. Think you might help lot of users with that. Thanks !!

Autocomplete

Could you explain shortly how you have measured that the autocomplete was one of the bottlenecks? I have added a fix for the autocomplete on the "panzoom" branch. Now the autocomplete is only added once the user clicks on an element, instead of adding autocomplete to all elements at conf screen startup:

targetIdField.click(function() { // Lazy load autocomplete for performance reasons
   targetIdField.autocomplete({
      source: function(req, add){
         add($.ui.autocomplete.filter(node.nonAnimationIds || [] , req.term));
      }
   });
});

I have done this for all autocomplete fields, so hopefully that helps. Would be nice if you could install that branch and do your performance test, and let me know if I should keep this fix.

TypedInput's

When I use the profiler of my Chrome developer tools, it seems there are an awful lot of resizing calculations for the TypedInput's:

image

Which is being triggered as soon as we do an addItem of a typedInput:

image

I think it would be better if the resizing was executed once at the end... But I have no clue at all whether that is possible. Did you have the same conclusion, or did you see something else going wrong perhaps? Seems this is a known problem , but they haven't solved the root cause since they have added workarounds to their nodes (change node...). Don't think these workarounds could be introduced here, because they only apply now TypedInputs to visible fields. But all our TypedInput fields are already visible in our table ...

I will discuss this on the forum as soon as I have time. So please share me your analysis of the problem, so we can explain correctly to the folks what is going wrong...

Thanks !!!

Bart

andreypopov commented 4 years ago

Could you explain shortly how you have measured that the autocomplete was one of the bottlenecks?

That was my suggestion, I disabled it and found performance boost. I don't know how to accurate measure it. Your solution could help, I thought about it. But sometimes autocompletes really interfere with filling inputs. Checkbox to disable would be good

Did you have the same conclusion, or did you see something else going wrong perhaps?

Yes, the same conclusion. It is typedInput's issue. Can we init it onclick like autocompletes?

btw. node's saving is also so slooow.. my svg node is still growing :)

bartbutenaers commented 4 years ago

Hey Andrey,

Sorry but I'm really short of free time lately. I have now created a discussion on the Node-RED forum, and hopefully I get the golden tip to solve this.

P.S. There is another workaround, since you can replace a series of event handlers by a single one, when you should use CSS selectors. See an example on our wiki page. Then your config page becomes more simple and readable.

Will keep you updated if I should get an answer from anybody.

Bart

bartbutenaers commented 4 years ago

Good news, Nick has responded on the Node-RED discussion. He has redesigned the typedinput widget completely, to solve the performance issues. It will be released in Node-RED version 1.0.5 in the near future

bartbutenaers commented 4 years ago

Evening Andrey (@andreypopov), Node-RED 1.0.5 has been published today. I have installed it and now your SVG node config screen opens in 2 à 3 seconds, which is very acceptable for me. Especially since we don't have to disable the typedinputs, which would most probably result in other issues for other use cases afterwards.

But there is still a problem with the layouts of the typedinputs. I have reported this problem and Nick has solved it already in Node-RED 1.0.6 (which will most probably released next week).

P.S. I did test this on the "panzoom" branch, which contains also my fix to lazy initialize the autocomplete fields.

bartbutenaers commented 4 years ago

Hi @andreypopov, I have tested it tonight with Node-RED 1.0.6. Your SVG node config screen still opens in 2 à 3 seconds, and the layouts of the typedinputs seem to be fixed correctly now. Can you check whether the issue is solved for you with that Node-RED version? Thanks!!

bartbutenaers commented 4 years ago

Hi @andreypopov, Would be nice if you could let me know if I can close this issue for the upcoming 2.0 release. Thanks and have a nice weekend!

bartbutenaers commented 4 years ago

I assume it is solved ...