Bookmarks Performance Issues

[Aradiv]

The Bookmarks Plugin is pretty imperformant.

Issues: The biggest Performance issue comes from the jquery.sortable. But also other parts aren't well implemented.

eg:

• multiple read/writes to localStorage for the same function.
• sorting of bookmarks is done by rewriting the JSON. (to display bookmarks in order we reparse the JSON every time because we don't store the sorting information.)
• the bookmarks ID has no real use (but also isn't that big of an issue)

Solutions:

• replace jquery.sortable with sortableJs/Sortable
• store sorting to reuse without having to reparse the localStorageJson everytime the box gets redrawn.
• don't reparse localStorage JSON after inital parsing.

[Aradiv]

Some old posts where other people hat issues with jquery.sortable performance https://stackoverflow.com/questions/20398400/jquery-ui-sortable-call-is-slow-when-applies-to-thousands-of-elements

But the workarround mentioned doens't really work for us because then the user first have to scroll over every entry that they want to interact with otherwise the user can't drop the entry.

Also that the storing of the sorted list works is a gamble because we rely on js to return attributes of an object in the same order they where added. sortBookmarks and JSON.parse to not alter the order when parsing to an object.

[MysticJay]

localstorage handling is pretty complex since BMrks is enabled for SYNC. There are two additional QUEUE storages assigned to BMs each time a BM is changed the first queue get updated using extend function every 3000 ms that queue gets matched with the 2nd queue which is the queue that actually is handled by SYNC. I have not analyzed how exactly SYNC handles the rest. -> @modos189

This two step SYNC-procedure is created to avoid conflicts between incoming and outgoing portal changes (adding and deleting). (A user might have done changes on PC and on his phone and on a third device... at the same time)

[McBen]

Also that the storing of the sorted list works is a gamble because we rely on js to return attributes of an object in the same order they where added

It's an erroneous assumption that fooled me too. But property order is fixed in JavaScript objects since ES2015. A nice to read on this: https://www.stefanjudis.com/today-i-learned/property-order-is-predictable-in-javascript-objects-since-es2015/ If you hardly keep fighting the "only ES5" rule it's a thing to think about. But you won't hit the problem on real systems.

multiple read/writes to localStorage for the same function.

localStorage isn't that bad. I wouldn't spend much time on optimizing it without reason.

[Aradiv]

localStorage isn't that bad. I wouldn't spend much time on optimizing it without reason.

while access to localStorage isn't that bad it is still IO and the repeated JSON.parse and JSON.stringfy calls quickly add up. And optimizing it actually isn't even that complex since we have to write the bkmrksObj anyway.

There is no need to ever reload from localStorage after the initial load. even the import has to write the bkmrksObj and localStorage so we don't need to load from localStorage.

Another Problem with the sorting via object-attribute is that moving 1 element requires to rewrite the entire object to get the attributes added int the correct order.

If it would be an array we could just move 1 element instead of having to rewrite the entire object.

[johnd0e]

@Aradiv When you say 'problem', do you mean that code is not perfect (or even bad)? Or you mean that we have real measurable problem?

[Aradiv]

@johnd0e it is measureable slower exspecially since the method we are using to apply changes runs in O(n^2) on an array storage we could do the same in O(n)

[johnd0e]

@Aradiv Then please add samples to reproduce measurable delays.

[Aradiv]

@johnd0e You need to have a lot Bookmarks in the idOthers folder

then run Object.keys(plugin.bookmarks.bkmrksObj.portals.idOthers.bkmrk).map((id) => { console.log(id); bkmrksArray.push(plugin.bookmarks.bkmrksObj.portals.idOthers.bkmrk[id]); }); to have the directory converted to an array

now you can bkmrksArray.splice in and out elements

almost the same as 2 calls to splice would be to call 1 time window.plugin.bookmarks.sortBookmark("bkmrk_portals") this does iterate over the dom so it is not enterly compareable but the speed difference should still be obvious.

at the point where sortBookmarks gets called we only need to handle the 1 element we just moved

[johnd0e]

So you are proposing to adopt sortableJs. How big is it?

[mralext20]

@johnd0e Looks like it's 14K of minified JS