tree-view: simplify reducer function

[just-boris]

Spread operator is really not necessary here. It can be replaced with a simple filter function. Fortunately .filter() returns new array instance as it required by Redux and here it has a perfect fit.

Even more, filter works here faster: because doesn't create temporary arrays.

[mxstbr]

I like this, it's a much cleaner solution – @gaearon do we want to merge this?

[gaearon]

If this is a better solution we should search for other uses of slice and update other examples accordingly.

Have you verified it runs better in something other than a microbenchmark, and in different browser engines? For example, dispatching a 1000 actions with random indices and measuring the difference with performance.now() would be a better benchmark.

[just-boris]

1. This is the only place with that. There is a similar thing in the reducer from TodoMVC example. It already uses .filter()

2. Do we really need more benchmarks? I think the performance difference is obvious. Just think about that every slice call produces an extra array instance that will be immediately destroyed after.

[johnsoftek]

Performance is not obvious. It must be measured - and not in a
micro-benchmark; rather it must be done using a more real-world benchmark.

On 4 March 2016 at 00:08, Boris Serdiuk notifications@github.com wrote:

1. This is the only place with that. There is a similar thing in the
   reducer
   https://github.com/reactjs/redux/blob/master/examples/todomvc/reducers/todos.js#L49
   from TodoMVC example. It already uses .filter()

2. Do we really need more benchmarks? I think the performance difference
   is obvious. Just think about that every slice call produces an extra
   array instance that will be immediately destroyed after.

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

[gaearon]

I think the performance difference is obvious.

Performance is never obvious. 😉

[just-boris]

Can this be considered as a real-world benchmark: https://gist.github.com/just-boris/6375c63bd8f334170511?

[gaearon]

Does it also work better with smaller data sets (e.g. < 100)?

[just-boris]

@gaearon Added results on various sizes in the gist. The advantage is not big, but filter is still better.

[gaearon]

Can you also check SpiderMonkey (Firefox) please?

[just-boris]

Done. Still faster.

ArraySize: 100
spread x 624,463 ops/sec ±2.34% (59 runs sampled)
filter x 1,307,458 ops/sec ±3.89% (58 runs sampled)

All results are still availabe in the gist

[gaearon]

👍

Thanks for researching this thoroughly!

[gaearon]

Would you please look for slice usage in the docs?
I definitely see it in other places, e.g. https://github.com/reactjs/redux/blob/master/docs/basics/Reducers.md

[johnsoftek]

@just-boris Well done! The results are fairly conclusive.

Not so much of an advantage for small arrays. Even so, the filter version is actually easier to read.

[rajdee]

Ha-ha, optimization of browsers moves on =)
Slice is much faster than the filter in the latest Chrome.