2

I have a fairly time-critical piece of code in a react app that gets run on all user keystrokes in a form. It has no performance issues in general, but I was looking into optimising it a bit and was a little surprised at the performance differences between for (let k in obj) and Object.keys(obj).reduce.

I would think that setting up function call stacks, etc., would be expensive in JS, but the functional version of the following routine blows the procedural one out of the water (by a full order of magnitude!).

Here are the different versions:

Procedural

const generateProps = (fields, source, start) => {
  if (!fields) return start
  let finish = {...start}
  for (let k of Object.keys(fields)) {
    const fld = fields[k]
    if (fld instanceof Array) {
      if (fld.length === 0) continue
      // Handle an array of scalars (e.g. phoneNumbers)
      if (fld[0].hasOwnProperty('value')) {
        let sf = {}
        for (let i = 0; i < fld.length; i++) {
          sf[i] = fld[i]
        }
        finish = generateProps(sf, source[k], finish)
      // Handle an array of fields (e.g. addresses)
      } else {
        for (let i = 0; i < fld.length; i++) {
          finish = generateProps(fld[i], source[k][i], finish)
        }
      }
    } else {
      finish = {
        hasError: fields[k].hasError || fields[k].value === '' || finish.hasError,
        isEditing: fields[k].editing || finish.isEditing,
        unchanged: (!fields[k].isNew && fields[k].value === source[k]) && finish.unchanged,
        hasNew: fields[k].isNew || finish.hasNew
      }
    }
  }
  return finish
}

Functional

const generateProps = (fields, source, start) => {
  if (!fields) return start
  const keys = Object.keys(fields)
  return keys.reduce((props, k) => {
    const fld = fields[k]
    if (fld instanceof Array) {
      if (fld.length === 0) return props
      // Handle an array of scalars (e.g. phoneNumbers)
      if (fld[0].hasOwnProperty('value')) return generateProps(fld.reduce((sf, f, i) => {sf[i] = f; return sf}, {}), source[k], props)
      // Handle an array of fields (e.g. addresses)
      return fld.reduce((subp, f, i) => generateProps(f, source[k][i], subp), props)
    }
    return {
      hasError: fields[k].hasError || fields[k].value === '' || props.hasError,
      isEditing: fields[k].editing || props.isEditing,
      unchanged: (!fields[k].isNew && fields[k].value === source[k]) && props.unchanged,
      hasNew: fields[k].isNew || props.hasNew
    }
  }, start)
}

And here are the jperf results

As you can see when you run the test, the procedural version is nearly 50% slower than the functional. I would be interested to hear a breakdown as to why there is such a stark difference.

Dallas
  • 888
  • 3
  • 10
  • 24
  • 1
    As so very often, [that benchmark](https://jsperf.com/for-in-vs-for-of-keys-vs-keys-reduce) you linked is totally flawed. – Bergi Oct 13 '17 at 04:48
  • Besides being flawed, `for(let ... in ...)` runs twice as fast as the last two in firefox .... and `Object.keys(obj).reduce` runs twice as fast as the first two in Chrome - so benchmarks on one browser mean nothing anyway :p – Jaromanda X Oct 13 '17 at 04:56
  • What browser are you using? `{...start}` is a syntax error in the "procedural" jsperf. – Bergi Oct 13 '17 at 04:59
  • 1
    Chrome Version 63.0.3230.0 on Ubuntu – Dallas Oct 13 '17 at 05:09
  • I think something is wrong with your algorithm (or data format) as well. Why are you copying from the `fld` array to the `sf` object? And both in case of the scalar descriptor and the case of the object of fields, you're trying to iterate `fld` - which (at least in your example data) is *always* an array of length `1`. Shouldn't you rather iterate the respective array in the `source`? – Bergi Oct 13 '17 at 05:13
  • The copy to `sf` is a shortcut to allow recursion over the array of scalars by calling `generateProps` on it. It is present in both versions, so this is not an issue. The example data only has one element in the array, but actual data may have 0 to n. `source` is a different format, in certain cases may not contain an entry that is in `fields` (thus the check for `isNew` in the last expression). – Dallas Oct 13 '17 at 05:22

1 Answers1

0

Ok, I found a critical difference! The functional version is mutating start (I had assumed reduce would create a copy, coming from a ramdajs background), whereas the procedural is creating a copy. If I change the last argument of the top reduce call to {...start}, they are about even.

Which makes me wonder... Why is object spread this slow?

Dallas
  • 888
  • 3
  • 10
  • 24