8-10x slower than React at 10k item list reversal

[brianmfranklin]

I was very interested in seeing how fast Riot was compared to React based on the intro docs.

So I put together these simple demos to help me learn and compare.

10k item list with reverse button. After you click the button, you'll see the time it took to reverse and finish rendering. (Actually, in a separate issue, I don't understand why you have to click the button twice on the Riot version before the timing text appears.)

React: http://jsfiddle.net/brianmfranklin/w674Lv7p/
Riot2: http://jsfiddle.net/brianmfranklin/j05ukz2r/

On my MacBook Pro, I consistently get about 2500 ms for Riot, and only 250-350ms for React.

I thought maybe this was a quirk of how I was having to measure the timing. But it DOES seem to actually take longer to do the re-render.

Is this expected? Or are there more optimizations to do?

[GianlucaGuarini]

@brianmfranklin Thanks for your issue! Now we have a new test that will be used to check the speed of the riot rendering engine. We'll work on this

[rsbondi]

I get about 450 for react on my machine and about 1600 for riot. A little less dramatic. If I cut it down 5000 items it is about 450 for riot. This seems acceptable but performance enhancements are always nice..

[GianlucaGuarini]

I think we can still optimize a lot the riot rendering speed and fix the memory leak re-introduced with the new releases #480

[brianmfranklin]

I agree that losing the DOM element references is less than ideal. I am glad to have contributed in some way to any improvements that come of it.

I am not sure if 10k is a realistic real world case, at least not commonly, however I think it is reasonable to think someone might have a grid view with some x thousands of rows that get re-sorted when a heading is clicked. Somewhere there is a balance between the speed of the computer and the browser, the size of the list, and Riot's optimizations.

For anyone who is wondering, I realized why the timing text didn't get updated until after the second button click. It's because the automatic update() that gets invoked after the clicked() handler happens before the setTimeout() sets the timeTaken property. So you are actually seeing the timeTaken on the PREVIOUS reversal, not the one that just happened. Here is an adjusted version:
http://jsfiddle.net/brianmfranklin/j05ukz2r/10/

[pakastin]

Here's native test:
http://jsfiddle.net/xue5b5eg/2

appendChild method is a lot faster than insertBefore (at least in Chrome) and a bit surprise is that documentFragment method is not faster but slower.

I will work these results out to Riot.

[GianlucaGuarini]

@pakastin Awesome test, I think a combination of RAF + documentFragment would be awesome

[pakastin]

Here's requestAnimationFrame + documentFragment:
http://jsfiddle.net/xue5b5eg/3/

appendChild seems to be the fastest (Mac Chrome 41).

Edit: Actually in Safari requestAnimationFrame + documentFragment is the fastest and in Firefox all of them are almost equally slow.. And after some cycles Chrome slows down appendChild also. requestAnimationFrame + documentFragment seems to maintain performance better.

[pakastin]

I think we have to use documentFragment anyway, because there can be elements after iterated nodes (appendChild is out of question):

<p each="{items}">{item.title}</p>
<p>Element after iterated nodes..</p>

[GianlucaGuarini]

I think so too

[pakastin]

Here's a Riot-test with documentFragment updates:
http://jsfiddle.net/j05ukz2r/11/

There's something else wrong, because I still get delays over one second (compared to 30 ms on my native version). It's a bit faster than insertBefore though..

Here's what I did:
https://github.com/pakastin/riotjs/blob/dev/lib/tag/each.js

[pakastin]

documentFragment + requestAnimationFrame is not any faster:
http://jsfiddle.net/j05ukz2r/12/

[GianlucaGuarini]

Hmm.. it seems the problem is somewhere else..

[cognitom]

I think another possible bottleneck is Array.prototype.indexOf. Actually it takes more than 500ms just for this call in my setup. for loop would be 5 times faster.
http://jsperf.com/js-for-loop-vs-array-indexof/8

[cognitom]

Oh, really? Hmm...

[paldepind]

It looks like the algorithm inside _each is O(n^2). No amount of replacing x with y based on micro benchmarks is going to fix that.

Maybe supplying unique keys could be optional? Then a more efficient algorithm is possible.

[brianmfranklin]

I don’t know the internals of Riot at this point, but if appendChild is faster, then would it be possible to basically clear out the parent’s children, then rebuild the parent element out of 3 segments of children:

1. children before the iterated nodes
2. the reordered iterated nodes
3. children after the iterated nodes

Or would this cause other problems, e.g. with event handlers firing on these temporary node removals?

On Mar 9, 2015, at 1:36 AM, Juha Lindstedt <notifications@github.com mailto:notifications@github.com> wrote:

I think we have to use documentFragment anyway, because there can be elements after iterated nodes (appendChild is out of question):

{item.title}

Element after iterated nodes..

—
Reply to this email directly or view it on GitHub #484 (comment).

[paldepind]

Ok. One of the reasons the React example is fast is because it doesn't preserve the elements between renders. We can tell React to reuse elements. This makes it about twice as slow in my tests. See for yourself: http://jsfiddle.net/paldepind/w674Lv7p/4/

[brianmfranklin]

A 2x difference from the normal React behavior (resulting in 1 sec to reverse) would still be quite an improvement and probably quite acceptable.

Notably, that 1 sec becomes around 4 seconds when doubling the list size to 20k with the keyed React demo. So there's certainly some dropoff point on the list size/performance curve, even with React.

[GianlucaGuarini]

I will be back on this as soon as other important bugs will be fixed #480 #475

[tipiirai]

Definitely need to address this one. My guess is that this is caused by cascading updates, since every time a parent is updated all the children are updated as well. I'm sure this can be fixed.

[pakastin]

Here is quite fast way to reorder:
http://jsfiddle.net/rju8rzmn/1/

1. request animation frame for all the following:
2. detach container from parent (add placeholder to keep place)
3. create documentFragment
4. move items to documentFragment
5. add documentFragment to container
6. attach container to parent (replace placeholder)

That all should happen during one event loop to prevent flashing to white.

[brianmfranklin]

Wow. That’s quite an improvement. And only takes 33% the time that the keyed React demo does with a 20k list.

Well done.

On Mar 10, 2015, at 3:56 AM, Juha Lindstedt <notifications@github.com mailto:notifications@github.com> wrote:

Here is quite fast way to reorder:
http://jsfiddle.net/rju8rzmn/1/ http://jsfiddle.net/rju8rzmn/1/

1. request animation frame for all the following:
2. detach container from parent (add placeholder to keep place)
3. create documentFragment
4. move items to documentFragment
5. add documentFragment to container
6. attach container to parent (replace placeholder)

That all should happen during one event loop to prevent flashing to white.

—
Reply to this email directly or view it on GitHub #484 (comment).

[paldepind]

@pakastin, You're not only timing the actual reordering but also the idle time between the frames? I get a more accurate smaller timing by not doing that: http://jsfiddle.net/paldepind/rju8rzmn/4/

The performance is very impressive. For me it's quite a bit faster in Firefox though.

[GianlucaGuarini]

Tnx guys!! Would you like to make a pull request? I am really excited about the results in riot

[pakastin]

@paldepind, requestAnimationFrame triggers before next DOM reflow. You have to wait for the next reflow to get correct results. Idle time between reflows is normally around 1/60th second, so it doesn't add that much extra time.

[paldepind]

I don't think this can be pulled. The problem is that @pakastin program rebuilts the entire list. This is faster in the current case when the entire list of elements is to be reversed. But, if only one item was moved, the current Riot implementation would only move one element but this code would still rebuilt the entire list.

@pakastin, The update happens inside updateElements(). You need to track the time spent inside that function to get accurate results. You're also tracking the time the browser waits until the next frame. That is not relevant with regards to the speed of the DOM manipulations. For me in FF there is a quite significant difference between the two results.

[pakastin]

I created a test project for DOM updates and it happened to become a new framework called frzr!

It's heavily inspired by Riot.js 2.0, but doesn't have all the features like templating/binding/routing:
http://pakastin.github.io/frzr
http://github.com/pakastin/frzr

Be free to copy the ideas from frzr. I'm really busy right now with my own client projects, so I don't have that much time to contribute to Riot.js. If you have any questions, I'm happy to answer – there's not that much comments yet.

[GianlucaGuarini]

Thanks @pakastin we will update riot using also your contribute

[GianlucaGuarini]

After many tests I have decided to keep the current riot each method ( without DOM reordering ).
The fastest libraries here are the ones updating the templates not the children ( without any DOM reordering ) ( see vuejs, react, and backbone).
I think the next riot release will use the current each method (the one in the dev branch) that is small, fast and clear.
Anyway I will keep this issue open for a while.
@tipiirai I hope you could agree on this with me

[tipiirai]

Totally fine with that. No reason to block the development because of the lack of reordering.

btw: last time I checked the child elements lacked the parent reference on dev- branch.

[GianlucaGuarini]

@tipiirai could you check please the dev branch? Let me know if you still have the problem

[pakastin]

I'd argue the reasons for slowdowns are somewhere else. Here's 10 000 items reordering:
http://jsfiddle.net/ohaf2jxn/

Here's documentFragment -version, again 10 000 items reordering:
http://jsfiddle.net/ohaf2jxn/1/

I'm getting ~30-40 ms rendering time (Chrome/Safari) – I don't think that's slow..

[GianlucaGuarini]

Thanks for your input @pakastin but I think in this post we have a bit lost the point.
Here some thoughts about the topic:

1. in your demos you just reorder a list without any data lookup, and even in that case the rendering is slower than the current riot in the dev branch
2. I have tested several solutions and you are right, the rendering it's not slow but the items position lookup by using indexOf or even track slows down the loops
3. Other libraries such as vuejs, backbone, rivetsjs, react, do not really reorder the DOM nodes but they simply update them using a template engine (sometimes just strings) making the whole rendering fast ( try for example the iframe example also with other libraries and you will see what I mean)
4. The old riot each is x3 bigger, slower and more complicated to mantain (~110 lines vs 40 ) than the current version I consider this a win not a con
5. If anyone is able to bring back the DOM reordering in riot without bloating the source code and with the same performances of the current version I will be more than happy merging it

Right now I consider this topic closed and I am moving forward improving other riot features. Discussing about this without any new pull request is pointless to me
Thanks anyway for your help without you guys the riot loops probably wouldn't run at speed of light

peace ✌️

[tipiirai]

~110 lines vs 40

That is just fantastic job @GianlucaGuarini !

[gut4]

Just a question. I need to make drag-n-drop sortable list. Is it possible with new version of each.js without dom reordering?

[GianlucaGuarini]

@gut4 of course it will.. It all depends from your code base. You should always keep your html list in sync with your javascript data collection.

[GianlucaGuarini]

Riot 2.2.0 is finally out! I will close this issue hoping our users will not complain the old each method 😄

[geekingreen]

Awesome!

[MauriSalazar]

great!

[diegochavez]

I just got mesmerized with all the hard work, this is great!. I learn a lot of stuff!

[brauliobo]

there is clealy a regression in 2.3.x (tried 2.3.15), it requires more than 10s for the same jsfiddle. In best cases it takes 2s. Riot 2.4.0 is taking 1,5s here. Riot 2.2.4 takes less than 300ms.

that is probably related to the huge slowdown I'm having to render a big list (more than 40s to render)

I'll probably revert the app back to riot 2.2.0 if there is no easy fix for this?

[brauliobo]

@GianlucaGuarini the version you posted at http://jsfiddle.net/gianlucaguarini/cbjuek58/ still take less than 100ms, is it possible to use that same code in new riot versions?

[GianlucaGuarini]

Just use the no-reorder option an you should be fine

[brauliobo]

With no-reorder the rendering took +500ms on riot 2.3.15 and +1000ms on riot 2.4.0. Still not satisfatory IMHO. Shouldn't the most performant version be the default?

PS: Edited the original fiddle at http://jsfiddle.net/brianmfranklin/j05ukz2r/

[brauliobo]

Also it is important to note that the slowdown also applies to the initial rendering, which makes a really bad user experience in the page load.

[GianlucaGuarini]

@brauliobo it's already on our todo list #1694 . This does not mean that rendering huge lists on the clientside with any framework is not insane IMHO

[brauliobo]

nice @GianlucaGuarini. one thing to note is that the improvement proposed at http://jsfiddle.net/paldepind/rju8rzmn/4/ is taking +400ms to render, so still http://jsfiddle.net/gianlucaguarini/cbjuek58/ is much better (and also riot 2.2)

[brauliobo]

btw, in my usecase the list has almost 6k items, the each loop is using virtual and has 5 elements inside, and that is taking almost 60s for riot.mount to finish

[brauliobo]

sorry for buzz here, the issue I have is actually another problem, reported at #1819