improve performance of updateElement

[zheksoon]

Hello!

I've noticed that updateElement function uses object spreads in order to iterate by joined old and new properties. Spreading (which is transformed to Object.assign later after build) is quite expensive, and the function is quite performance-critical, so I've made an attempt to make it faster.

With these changes, it's consistently 25-38% faster than your implementation, see this microbenchmark: https://jsben.ch/YhZh4.

In the benchmark I've made an attempt to recreate some properties changes on independent DOM elements, using some common DOM attributes and handlers. updateElement1 is your implementation, updateElement2 is my improved one.

The cost of the improvement is about +100 bytes of bundle size, but it might worth it if you are heading to be the fastest VDOM framework or optimize for rendering speed.

Anyway, will be glad to hear any feedback on this topic. Thanks!

[yisar]

Can this be name in aProps? Maybe can save bytes.

[zheksoon]

Modern JS engines do some optimizations for the for ... in loops, followed by hasOwnProperty check, so this might be needed for getting maximum performance

[yisar]

I tried Object.keys. It will be faster and have fewer bytes.

const jointIter = <P extends Attributes>(
  aProps = defaultObj as P,
  bProps = defaultObj as P,
  callback: (name: string, a: any, b: any) => void
) => {
  Object.keys(aProps).forEach(k => callback(k, aProps[k], bProps[k]))
  Object.keys(bProps).forEach(k => callback(k, null, bProps[k]))
}

Do you agree with me to make some changes on your code?

[zheksoon]

Hah, I've also discovered that Object.keys and forEach performs almost the same as for ... in loop. But your version has an issue: it will iterate twice by properties that are both in aProps and bProps. Correct version is this one:

const jointIter = <P extends Attributes>(
  aProps = defaultObj as P,
  bProps = defaultObj as P,
  callback: (name: string, a: any, b: any) => void
) => {
  Object.keys(aProps).forEach((k) => {
    callback(k, aProps[k], bProps[k]);
  });
  Object.keys(bProps).forEach((k) => {
    if (!hasOwnProperty.call(aProps, k)) {
      callback(k, undefined, bProps[k]);
    }
  });
}

I'd also rather prefer returning undefined to the callback as this is consistent with your previous implementation

[yisar]

You're right. Thank you. I'll revise it again.

I'd also rather prefer returning undefined to the callback as this is consistent with your previous implementation.

What does this mean? I like your implementation better than mine.

[zheksoon]

What does this mean? I like your implementation better than mine.

It's just for compatibility with your previous implementation.

https://github.com/yisar/fre/blob/afea19f5b3d7ca91025f2f1c2d812f27c70397ee/src/dom.ts#L11
There is a problem with this check - it won't iterate any values at all if it will be called as jointIter(null, { a: 1 }, callback) or jointIter({ a: 1 }, null, callback) - this case usually can't happen with usual props, but can happen with style prop.

And just realized that my implementation has the flaw with null argument too - it will throw with any null argument. The right one will be like this:

const jointIter = <P extends Attributes>(
  aProps: P,
  bProps: P,
  callback: (name: string, a: any, b: any) => void
) => {
  aProps = aProps || defaultObj;
  bProps = bProps || defaultObj;
  Object.keys(aProps).forEach((k) => {
    callback(k, aProps[k], bProps[k]);
  });
  Object.keys(bProps).forEach((k) => {
    if (!hasOwnProperty.call(aProps, k)) {
      callback(k, undefined, bProps[k]);
    }
  });
}

[yisar]

done! Thank u!

[yisar]

Great implementation, I'm willing to merge it.