Performance issues with implicit returns in reduce functions

[HendrikThePendric]

Writing a reduce function like this looks really concise and clean:

arr.reduce(
    (acc, { id, value }) => ({
        ...acc,
        [id]: value,
    }),
    {}
)

The alternative, looks a little bit less pretty:

arr.reduce(
    (acc, { id, value }) => {
        acc[id]: value
        return acc
    },
    {}
)

However, the first version is making a shallow copy of the accumulator object on every iteration, and because of this I expected it to be a bit slower.

I was interested in finding out how much slower this would actually be, so I made a jsperf yesterday to find out. I was pretty shocked about the results: it's not just a little bit slower, it's a HUGE difference...

Because the differences were so extreme, I initially didn't trust the results. So I also created a simple HTML page with a button that tests the same thing, and confirms the results. And I also discussed things with @amcgee @Birkbjo and @Mohammer5, and they couldn't find anything wrong with my setup.

@amcgee also prepared a jsperf to compare object-rest-spread to parameter assignment in isolation.

Basically, everything points to the object spread operator and object assign being way slower than parameter assignment. So my advice would be: don't use this pattern in iterations. Especially in a reduce function there is no need to do this, if you just make sure that the initialValue of the accumulator object is "a fresh object", you really don't have to worry about doing mutations.

Let me know if you agree.

[amcgee]

Great work @HendrikThePendric!

It would be really interesting to analyze our code and see how often we’re using this slow pattern (it was WAY slower than I thought it would be in the perf tests) in iterable reduce functions.

In my performance tests I found that a function performing explicit assignment performs comparably to direct assignment (1-10% overhead depending on the browser) and avoids the requirement to include a function body in the reduce handler:

// fastest, somewhat clunky
list.reduce((out, item) => {
   out[item.key] = item.value
   return out
}, {})

// clean, second fastest (assuming setProperty is a common utility function)
const setProperty(obj, key, value) => {
  obj[key] = value
  return obj
}
list.reduce((out, item) => setProperty(out, item.key, item.value), {})

// cleanish, 10-20x slower!
list.reduce((out, item) => Object.assign(out, { [item.key]: item.value }), {})

// cleanish, 10-20x slower!
list.reduce((out, item) => ({
  ...out,
  [item.key]: item.value
}), {})

I agree 100% that this could be problematic, let's not do it!

[varl]

Good job taking the time to produce some numbers on common patterns! :100: It takes presence of mind to go from :thinking: to :straight_ruler:.

Object.assign and the Spread syntax copies all the enumerable properties, so it is not a surprise that these are slower than not copying over all properties on each iteration. The sheer amount they are slower by is surprising, indeed.

The Spread syntax and Object.assign are very nice to use where immutability is required (e.g. reducers) but this is a very nice reminder that just because they make sense in one place, doesn't mean the pattern is applicable everywhere.

If you want to go even faster :tm:, remember that TurboFan optimizes by creating a type based on the object properties. So if you define { foo: 'bar' }, V8 will create a class definition for that object which it will pass to TurboFan to use in loops. If that object gets a new property { foo: 'bar', fuz: 'buz' } it fails the type check and gets passed back to Ignition. It in turn creates a new class def and passes it to TurboFan.

Adding new properties to an object after instantiation breaks TurboFan optimizations. This means that if it is possible to create a single object with all the properties defined (even if they are set to undefined), it will be faster to use that object. The more hints the engine gets, the better it can optimize.

https://jsperf.com/implicit-vs-explicit-return-in-reducer-fn/7

(Creating a type this way is not the way to do it in production code, this is a synthetic example to highlight the differences in how the ES engine is able to optimize code at runtime.)

Firefox:

Chromium:

All-in-all this is a fine gotcha to write down and to have in mind! :+1:

N.B. I'm referring to the V8's TurboFan and Ignition, but Firefox has similar JIT optimization techniques in place.

Edit: updated second paragraph for precision, some of the examples create objects and copies properties, and some just copy properties, and some create and assign properties directly. Text is updated to reflect Object.assign and Spread specifically.

[amcgee]

Wow, super deep dive nice @varl!

I was also surprised by the amount of slowdown when using object spread, but am thoroughly puzzled by the slowdown using Object.assign, since Object.assign shouldn't be creating new object clones each iteration (it just assigns to the first argument passed). My guess is that the construction of an object just to pass as the second argument to Object.assign (Object.assign(obj, { a: 'b' })) is the bottleneck, not the cloning of all the properties. Interesting stuff.

[varl]

I see that I should have been more precise and stated that some of the listed functions create new Objects and copies the properties. Some of them only copies the properties. From what I can gather, all the slow ones copy and all the fast ones assign.

You are right; Object.assign does not create a new copy on each run, but it does get all enumerable properties and copies them to the target.

Creating a new Object is quick (using DOMHighResTimeStamp I cannot even measure the speed), as is assigning a property to an Object, so I don't expect that to be the bottleneck, however copying properties from one object to another where the engines start getting into problems (which I can measure with DOMHighResTimeStamp), which both Spread and Object.assign does, under the hood.

Object.assign also runs any setters and getters, so on Objects that use those functions it can be very slow indeed. I don't think native Spread calls those, but, most of the polyfills for Spread uses Object.assign so YMMV.

Object.assign(obj, { a: 'b' })

So based on the above, this creates a new object for { a: 'b' } (fast), and then it copies the prop a to obj.a (slow). Not sure if this is situational, but it aligns with my understanding (for now at least). :)

[amcgee]

Huh, the enumeration of the source object must be what's slow then. Fascinating.

[HendrikThePendric]

@varl thanks for sharing that example that demonstrates optimisation techniques. Seems we all agree: use assignment in reduce functions. Now currently this is just a GitHub issue, but you mention it should be a gotcha. Shall I write a PR that adds a summary of this discussion to the gotchas section?

[HendrikThePendric]

Alternatively, we could add it as a decisions section. You could argue the case for both....

[janhenrikoverland]

I noticed the same thing some time ago and have added to and returned the same object in reduce iterations since then.

However, a guy called Sigurd Schneider from the v8 team had a presentation at last year's Web Rebels in Oslo (Edoardo, Jennifer, Viktor and I were there) and he was pretty clear that: "you write simple code, we fix the performance".

I still think we should go with the clunky one though. The perf difference is just too huge to ignore right now.

[varl]

Now currently this is just a GitHub issue, but you mention it should be a gotcha. Shall I write a PR that adds a summary of this discussion to the gotchas section?

Alternatively, we could add it as a decisions section. You could argue the case for both....

Absolutely. I consider this more of a gotcha than something we want to enforce through a decision in large part based on the quote @janhenrikoverland provided:

"you write simple code, we fix the performance"

As a general rule I think it is heavy-handed to say never use object spread in a reduce function, because the problem is bound by the data set.

If we have 10'000 elements to reduce over and we run it on a field validation on every keystroke, it is important to maximise performance.

Yet if we have 10 elements, and we are able to cache the result? The more readable version might be the superiour choice, in that case.

So I think this is a "gotcha" we should have in mind, rather than a decision ("law") we can unilaterally apply to our entire code base in every situation.

cc: @HendrikThePendric