Closed 0x0ade closed 6 years ago
I just saw on Twitter that @neauoire is taking this weekend off. It'd be nice if someone else could at least test the changes :)
I'm dead in the water with no sail. Last Beaker update has 32bit build failing still; no solution yet. ☹️
Oh, sad to hear that :( Are you compiling Beaker yourself?
Yes, and issue is opened. Further assistance pending.
Fixed almost all bugs / regressions I could find for now. The performance gain doesn't seem to be as big for some as it is for me, though :/
Edit: One bug is still left in the networks tab - portals you're following are being added. Possibly a regression in has_hash
?
@0x0ade
The performance gain doesn't seem to be as big for some as it is for me, though :/
are you referring to my answer on rotonde? I should have added that my prior performance wasn't as awful as yours. maybe I have a faster processor or something. OR maybe I'm full of shit and don't know how to read the performance reports
before:
after:
@eelfroth Oh 😅 that may be the reason why.
By the way, the slower set x iterator
/ iterator x set
comparisons in has_hash
now use the result of .hashes()
(array) as the iterator. Iterating through the set caused some issues - the performance difference should be negligible.
This fixes the last last bug I could find.
@eelfroth Seems like the stutter I was experiencing was caused by Feed.register
and all its related functions taking too long. All other portal registrations "piled up" and left no room for the browser to repaint. Combine that with how aggressively the DOM was manipulated before, and I get this:
Early in the graph, we can still see the browser behaving normally. Portal.connect
and its Feed.register
/ Feed.refresh
don't take too long, the browser can resume rendering immediately afterwards.
Without the portal & entry - related changes, Portal.entries
keeps "updating" all entries for each portal, even if they are up to date. Additionally, Entry.to_element
keeps re-adding every visible entry's element to ensure correct order.
Once a critical point's reached, Beaker doesn't even try to repaint until all DOM manipulations are complete. Towards the end, a single Portal.entries
call takes up to around 160ms (10 frames at 60 FPS!) and Beaker doesn't even repaint after every Feed.refresh
anymore.
Seems like I just had really bad luck ¯\_(ツ)_/¯
By the way: The feed's currently spawning connection "loops."
In Feed.start
:
https://github.com/Rotonde/rotonde-client/blob/277dd9be7bbdef7b64213d1f0b42b307ec48459c/scripts/feed.js#L74
In Portal.connect
:
https://github.com/Rotonde/rotonde-client/blob/277dd9be7bbdef7b64213d1f0b42b307ec48459c/scripts/portal.js#L66
Removing the interval causes the feed to load painfully slowly: # of portals * 750ms + connection overhead (or timeout time) for each portal.
On the other hand, keeping the interval with Portal.connect()
in its current form causes issues:
Every 500ms, r.home.feed.next()
-> Portal.connect()
-> Run r.home.feed.next()
again in 750ms
Thus every 500ms, we kick off a new async r.home.feed.next()
loop.
I propose doing one of the following:
.connect()
won't call r.home.feed.next()
. Portals timing out won't affect connecting to other portals, but this can take up more networking resources. While a slow feed's loading, another faster feed can load at the same time.Personally, I'd just prefer if there was just one loop with no delay at all - I prefer to get connected as fast as I can, not to slow down the connection arbitrarily. I'm blinded by having a consistent internet connection and understand the limitations of slower connections, but I can't see how arbitrary delays help with connecting to portals.
(Posting as new comment because I heavily edited it for clarity, sorry for the possible spam.)
I went with a combination of the 3 points outlined above:
Portal.connect
and Feed.next
are async and a timeout of 0 allows the browser to "breathe" (perform other microtasks, update the style and layout).This gif wasn't sped up in any way.
Please check the commit message for more info :)
😮
Oops, this became a little more than just optimizations ¯\_(ツ)_/¯
I'd be interested in seeing how this code runs on a high-latency network (e.g. 700-800ms)...
Sorry about that, I took saturday off and away from the computer.
No need to be sorry for taking the weekend off :)
I'm wondering, @neauoire, you increased the delay between connections. Did increasing the delay actually help with high latency connections? Because I don't really understand how letting the loop sleep for 750ms could help with that :/
It was mostly for stability, I did this right before 0.78. It allowed me to work more easily and with fewer crashes.
Ooh, that makes more sense. Thanks for explaining! :)
move_element
to move elements, thus manipulating the DOM much less aggressively. This is whatEntry.to_element
/Portal.add_badge
built towards. Before, all elements were blindly re-added, causing all elements to get invalidated with every "refresh."format_links
with character index based manipulation instead of split & join. Still made it simple to use.sort
direct delta (faster sorting in my testing).to_hash
.Portal.hashes_set()
, allowhas_hash
to compare againstSet
.Set.has
instead ofArray.indexOf
My portal (following 114 portals, 1998 entries at the moment) doesn't stutter while loading anymore.