Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perf: replace spread with Object.assign #1900

Merged
merged 5 commits into from Oct 18, 2022

Conversation

ozonep
Copy link
Contributor

@ozonep ozonep commented Oct 4, 2022

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

Object.assign is at least 2x times faster than spread operator. This can be noticable when one needs to update state object quite often (for example - on every user keystroke). In this case performance is important.

📝 Changelog

Language Changelog
🇺🇸 English Internal change of spread operator to Object.assign for performance improvement
🇨🇳 Chinese

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

@CLAassistant
Copy link

CLAassistant commented Oct 4, 2022

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ ozonep
❌ crazylxr
You have signed the CLA already but the status is still pending? Let us recheck it.

@SignDawn
Copy link

SignDawn commented Oct 9, 2022

有参考资料说明 Object.assign 比 spread operator 更快吗?我查到的资料是 spread operator 更快
https://juejin.cn/post/6844903774620762120

@ozonep
Copy link
Contributor Author

ozonep commented Oct 9, 2022

@SignDawn Yes, if you look at that article, it IS correct.
But! Spread operator's performance is VERY dependant on number of "..." uses when copying object properties.
Example:

const obj = { foo: 1, bar: 2 };

const resultSpread = { baz: 3, ...obj };
const resultAssign = Object.assign({}, { baz: 3}, obj )

In this case performance will be almost identical, like in the article you mentioned. On my machine with Node.js v18 it's:

Object spread x 35,729,776 ops/sec ±0.45% (96 runs sampled)
Object.assign() x 37,869,629 ops/sec ±0.12% (99 runs sampled)

But once you do this (the way spread operator is used in ahooks):

const obj = { foo: 1, bar: 2 };
const obj2 = { baz: 3 };

const resultSpread = { ...obj, ...obj2 };
const resultAssign = Object.assign({}, obj, obj2 )

Performance of Spread drops significantly:

Object spread x 3,992,416 ops/sec ±0.16% (97 runs sampled)
Object.assign() x 24,466,196 ops/sec ±0.12% (99 runs sampled)

If you add yet another object:

const obj = { foo: 1, bar: 2 };
const obj2 = { baz: 3 };
const obj3 = { another: 4 }

const resultSpread = { ...obj, ...obj2, ...obj3 };
const resultAssign = Object.assign({}, obj, obj2, obj3 )

You get even worse results:

Object spread x 1,895,265 ops/sec ±0.31% (100 runs sampled)
Object.assign() x 18,974,961 ops/sec ±0.11% (97 runs sampled)

You can test it yourself with "benchmark" package on Node.js.

Summary: I haven't found a single csse when Object.spread is actually faster that Object.assign.

@SignDawn
Copy link

简单说,'Object spread' 使用的越多,性能越差
In short, the more 'Object spread' is used, the worse the performance is

@crazylxr crazylxr merged commit 223852b into alibaba:master Oct 18, 2022
@miracles1919
Copy link
Collaborator

miracles1919 commented Oct 18, 2022

In Fact, there is no need to change... Babel will transfrom when build

// source
{ ...prevState, ...newState }

// es
var __assign = this && this.__assign || function () {
  __assign = Object.assign || function (t) {
    for (var s, i = 1, n = arguments.length; i < n; i++) {
      s = arguments[i];
      for (var p in s) {
        if (Object.prototype.hasOwnProperty.call(s, p)) t[p] = s[p];
      }
    }
    return t;
  };
  return __assign.apply(this, arguments);
};
__assign(__assign({}, prevState), newState)

But if you use Object.assign, you also need to add @babel/plugin-transform-object-assign

// source
Object.assign({}, prevState, newState)

// without @babel/plugin-transform-object-assign
Object.assign({}, prevState, newState)

// with @babel/plugin-transform-object-assign
function _extends() { ... }
_extends({}, prevState, newState)

So I think this pr should be revert, but thanks for your contribution!

Alt-er pushed a commit to Alt-er/hooks that referenced this pull request Oct 18, 2022
* perf: replace spread with Object.assign

* chore: update version

* chore: update pnpm-lock

Co-authored-by: lxr <1076629390@qq.com>
Co-authored-by: 潇见 <xiaojian.lj@antgroup.com>
Alt-er pushed a commit to Alt-er/hooks that referenced this pull request Oct 18, 2022
* perf: replace spread with Object.assign

* chore: update version

* chore: update pnpm-lock

Co-authored-by: lxr <1076629390@qq.com>
Co-authored-by: 潇见 <xiaojian.lj@antgroup.com>
crazylxr added a commit that referenced this pull request Oct 18, 2022
crazylxr added a commit that referenced this pull request Oct 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants