Slow when having a large amount of signatures

[tangyaocn]

The program becomes very slow when there are many cells. Sometimes the program will take several minutes to respond after pressing the back bottom. I suspect the drop in performance is due to using url as a data storage (the url can become really long, 400k+ in my case). Also the url seems to be decoded and encoded for every operation, this might has direct effects on the performance.

[NickHu]

Yes, it is pretty clear to me that this is the major performance issue (see screenshot; it calculates the layout almost immediately, but then spends 7+ seconds serialising each time, which makes the program unusably slow).

@zrho is this something that is worth monkey-patching in the existing implementation or is it better to wait until the rewrite is done? I think it's time to retire the idea of putting data into the URL. The major implication of this is that we will actually have to implement undo properly.

[jamievicary]

Thanks for this issue and discussion. The serialization is essential not for the URL feature (a dispensable feature, I agree), but so we can push the state onto the browser history stack, so that the back/forward buttons work. If you look carefully, you can see that it's actually the compression step taking the most time, over 6 seconds in this case. But now we have a contradiction, because there's no way LZ4 takes 6 seconds to create 200kb of compressed output. So something else weird is going on here I think.

…

On Sat, Jun 6, 2020 at 1:15 AM Nick Hu ***@***.***> wrote: [image: image] <https://user-images.githubusercontent.com/450276/83931080-ae539b00-a792-11ea-8df5-a98d1c3c7827.png> Yes, it is pretty clear to me that this is the major performance issue (see screenshot; it calculates the layout almost immediately, but then spends 7+ seconds serialising each time, which makes the program unusably slow). @zrho <https://github.com/zrho> is this something that is worth monkey-patching in the existing implementation or is it better to wait until the rewrite is done? I think it's time to retire the idea of putting data into the URL. The major implication of this is that we will actually have to implement undo properly. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#121 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACQ4OHV7ZZVXQ5ZI7AVS66DRVGDBRANCNFSM4LDSHIOA> .

[NickHu]

@jamievicary ad5f553 seems like it's using Pako, which is deflate (zlib) and not LZ4. Why was this changed?

Addendum: lz-string library is not an implementation of LZ4, but an older (probably slower) algorithm called LZW.

[zrho]

There is a history API, which should enable the back button to work as we want it to without encoding the state into the URL. I haven't worked with that yet, but my impression is that we could intercept the navigation event and keep track of a history in non-serialised form. That'd also be more memory efficient since it can make use of sharing.

…

On Sun, 7 Jun 2020, 00:25 Nick Hu ***@***.***> wrote: @jamievicary <https://github.com/jamievicary> ad5f553 <ad5f553> seems like it's using Pako, which is deflate (zlib) and not LZ4. Why was this changed? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#121 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACXKJLWTR3XXY2DA4GZUGDRVK65VANCNFSM4LDSHIOA> .

[NickHu]

@zrho The problem is that the way undo works is by deserialising the entire state of the application. Seems like the latency arises from compression, but perhaps this isn't the best way to implement undo/redo anyway. There's a higher-order reducer called redux-undo which might just be able to save us lots of work.

[jamievicary]

Surely there are only 2 ways to have a fully-operative "undo" stack: (1) Serialize the entire applications state on every change (2) For every change, have an operation that reverses that change We are very far from having (2) available. For example, if the operation is "contract this part of the diagram", we would need to store some "uncontraction data" in the undo stack, that tells us in detail how to reverse that. Option (1) seems much simper to me, and I don't immediately see the fundamental problem, since it currently seems likely that the issue is that the compression is just taking too long, something that should be easy to fix.

…

On Sat, Jun 6, 2020 at 11:48 PM Nick Hu ***@***.***> wrote: @zrho <https://github.com/zrho> The problem is that the way undo works is by deserialising the entire state of the application. Seems like the latency arises from compression, but perhaps this isn't the best way to implement undo/redo anyway. There's a higher-order reducer called redux-undo which might just be able to save us lots of work. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#121 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACQ4OHROSVN3UNQTH3IC6PLRVLBSBANCNFSM4LDSHIOA> .

[NickHu]

There are two more options:

• Store the application state for every change

This is basically (1) except you don't have to serialise/deserialise all the time, which is probably preferable.

• Rather than having logical diffs like you suggest in (2), just store some appropriate form of JSON diff for the application state-over-time

I imagine in a modern web browser, the first probably does the second for you (probably redux-undo, which is based on the model of the first, is actually doing something like this).

The other thing is that tweaking the compression algorithm is conceptually simple, but it is a breaking change that we want to avoid doing too often (effectively, changing this changes our database schema). It's best to take the opportunity do to it properly.