DWilliames / paddy-sketch-plugin

Automated padding, spacing and alignment for your Sketch layers
MIT License
2.17k stars 61 forks source link

[Performance] Paddy causes major performance lags when editing symbols #36

Open sajidsan opened 6 years ago

sajidsan commented 6 years ago

Noticed performance issues when copying and editing symbols. Whenever I would copy and edit symbols, Sketch would freeze for 10 - 15s. Turned off my plugins one by one and noticed that Paddy was causing the issue.

frederickandersen commented 6 years ago

I've experienced this too. I'm running Sketch 48.2. I've restarted Sketch, disabled plug-ins, re-installed Paddy. Nothing did the trick.

Martijn-Schoenmaker-Webbio commented 6 years ago

Me too, but not for all files actually. I have no idea what makes paddy do it, but for now I had to disable it.

frankko commented 6 years ago

Downgrading to 1.0.2 solved the problem for me, for now, FYI for anybody who comes across this thread and hasn’t tried it.

seansmyth commented 6 years ago

Sadly, I've had to uninstall Paddy. I'm working on a very large library and it would literally make my Sketch hang for about 5 minutes every time I tried to interact with a symbol on the symbols page.

DWilliames commented 6 years ago

Hey all! Thanks for pointing this out!

At the moment, it can take a little while for Paddy to recalculate the size of a symbol instance, taking into account the padding within the Symbol Master, and its overrides.

However, this is made even worse when you edit the Symbol master directly... because at the moment, whenever you edit the Symbol master, it will recalculate its padding/spacing... and then also tell all of its instances to update their padding too, based on its changes.

Unfortunately this is incredibly non-performant. I have experienced this too.

I am thinking that a possible solution is that after you alter a Symbol Master that has padding in it, an alert would appear saying something like "Do you want to also resize the 10 instances of this Symbol now?" in which case it would be optional to resize all the instances or not.

Thoughts on that approach?

Indyandie commented 6 years ago

Hey @DWilliames thanks for building Paddy! I freaking love this plugin 😍.

When would you trigger the alert? It could be tricky for someone like me who is constantly tweaking the same symbol.

DWilliames commented 6 years ago

@Indyandie Yeah, that's a really good point. That's why I'm kinda a little stuck with this one 🤔

Got any suggestions? :P

DWilliames commented 6 years ago

Alright, until I can come up with a better approach, I've implemented the alert for the user to 'opt in' to resize all the instances.

The alert will only appear if there are 10 or more instances — since in most cases, less than 10 instances don't seem to take too long to update.

Indyandie commented 6 years ago

Hmm..

And that's all I got for now.

frankko commented 6 years ago

Hm, 1.0.4 didn’t fix the issue for me. I’ve yet to see the alert, which makes me think my issue isn’t related to 9+ instances. I think I’m seeing the beachball less in 1.0.4 than I was in 1.0.3, but nowhere near as much as 1.0.2 (which I downgraded to after 1.0.3 was released).

Also, I used to be able to select a layer intended for Paddy and hit ESC repeatedly, all the way up to the Symbol root. With 1.0.4, after Paddy does its thing on one layer, the selection drops out, and my layer list collapses almost all the way for that Symbol.

I might be kind of an edge case here because I rarely directly edit any of the layers/groups that are intended for Paddy. I have a custom plugin that updates content to many groups at once, using our site’s JSON API. In 1.0.2, I’d first run my plugin, then select an object that needed to be re-spaced, and then ESC all the way up the tree, and by the time I got to the Symbol root, Paddy would have gotten everything. I can’t seem do that with 1.0.4. An “update Paddy symbols” command mentioned above would solve that problem for me, at least. That command would also be good for people who did get the alert, said no to updating now, and wanted to update later. Rather than having to hunt down all the layers/groups that need attention, one command could get them all.

vic-tian commented 6 years ago

How about defer the update until you have the elements visible in the viewport? Can you detect which element is currently visible in Sketch vs. the hidden instances? If possible, you could just run the calculations on the elements that are important, and do the other instance re-render in batches to not kill the performance. Not sure if this helps at all.

dorian-grey commented 6 years ago

I'm not sure, if this is directly related to the updating issue, but there seems to be another problem with performance: Had around 8 buttons (groups, not Symbols) on an Artboard, which I told Paddy to take care of.

From that moment on Sketch permanently used 50-100% of my CPU and Sketch was beachballing all the time. I then removed the padding information from the layer names, which made Paddy not update them anymore - but CPU usage/Sketch performance was still the same. Only after disabling Paddy everything is back to normal.

So even if Paddy should not be doing anything, it seems to cause some problems in the background.

ziyafenn commented 6 years ago

There is indeed a performance issue. When you deselect the group/layer on the artboard, it deselects it with some delay. And i'm talking about 1 rectangle in a document. When i disable paddy, it works okay and deselects it immediately as it should.

tbnv commented 6 years ago

I have the same issue. Should it be with a small file or a big one (300+ symbols library, 150+ artboards).

DWilliames commented 6 years ago

Thanks for the suggestions all. @vic-topcoder that is a great idea. It may take a bit of changing of how Paddy is set up in order to be able to do that; but I think would be worth it.

@dorian-grey that sounds quite strange 🤔 I hope that's not Paddy causing it. It shouldn't be causing that much strain on the computer with such a small amount of elements.