FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

benchmark: rest operator benchmark by jasnell · Pull Request #18442 · nodejs/node · GitHub

/ node Public

benchmark: rest operator benchmark - #18442

Closed
jasnell wants to merge 2 commits into
nodejs:masterfrom
jasnell:rest-assign-bench
ConversationCommits (2)ChecksFiles changed
Closed

benchmark: rest operator benchmark#18442
jasnell wants to merge 2 commits into
nodejs:masterfrom
jasnell:rest-assign-bench

Conversation

jasnell commented Jan 29, 2018

Copy link
Copy Markdown
Member

Benchmark comparing util._extend(), Object.assign(), and the rest operator for object assignment (e.g. { ...src }).

util._extend() still wins currently.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

benchmarks

nodejs-github-bot added benchmark Issues and PRs related to the benchmark subsystem. v8 engine Issues and PRs related to the V8 dependency. labels Jan 29, 2018

vsemozhetbyt commented Jan 29, 2018

Copy link
Copy Markdown
Contributor

Does util._extend() win including v8 6.6 canary?

Maybe @nodejs/v8 will be interested as per https://medium.com/the-node-js-collection/modernizing-node-js-with-idiomatic-javascript-f18d984dcf93 (util._extend() is deprecated but still faster)

vsemozhetbyt added the performance Issues and PRs related to the performance of Node.js. label Jan 29, 2018

jasnell commented Jan 29, 2018

Copy link
Copy Markdown
Member Author

Will try on Canary next

cjihrig commented Jan 29, 2018

Copy link
Copy Markdown
Contributor

Minor nit: shouldn't "rest" be "spread" here?

vsemozhetbyt reviewed Jan 30, 2018
View reviewed changes
Comment thread benchmark/es/rest-assign.js Outdated

vsemozhetbyt Jan 30, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

_extend -> extend, otherwise Error: Unexpected method

vsemozhetbyt commented Jan 30, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

FWIW, quick comparison for V8 6.1 – 6.6 (Windows 7 x64):

data: 
Node.js 8.9.4 (V8 6.1)

millions=1 count=5 method="rest":      752,699.0359181605
millions=1 count=10 method="rest":     372,941.1917168629
millions=1 count=20 method="rest":      84,057.2428263545
millions=1 count=5 method="assign":    793,823.9688696787
millions=1 count=10 method="assign":   375,358.4750854752
millions=1 count=20 method="assign":    67,693.94943596305
millions=1 count=5 method="extend":  2,584,490.362717147
millions=1 count=10 method="extend": 1,198,959.2352828898
millions=1 count=20 method="extend":   216,349.3978181487

Node.js 9.4.0 (V8 6.2)

millions=1 count=5 method="rest":      687,296.3816598814
millions=1 count=10 method="rest":     354,315.56483754155
millions=1 count=20 method="rest":      83,370.70472415994
millions=1 count=5 method="assign":    760,248.1311230726
millions=1 count=10 method="assign":   385,468.1048516094
millions=1 count=20 method="assign":    63,472.79442919072
millions=1 count=5 method="extend":  2,565,508.9168867497
millions=1 count=10 method="extend": 1,257,488.3162013523
millions=1 count=20 method="extend":   205,424.33819629342

Node.js 10.0.0 (V8 6.3) nightly 2018 01 24

millions=1 count=5 method="rest":      684,898.7527392372
millions=1 count=10 method="rest":     356,268.7172361571
millions=1 count=20 method="rest":      79,288.13879076047
millions=1 count=5 method="assign":    724,004.9034969702
millions=1 count=10 method="assign":   360,679.5228756991
millions=1 count=20 method="assign":    64,133.39508113714
millions=1 count=5 method="extend":  2,407,796.7753451667
millions=1 count=10 method="extend": 1,185,288.573254075
millions=1 count=20 method="extend":   209,083.51173147012

Node.js 10.0.0 (V8 6.4) nightly 2018 01 29

millions=1 count=5 method="rest":      685,563.4192962835
millions=1 count=10 method="rest":     334,340.1719023568
millions=1 count=20 method="rest":      75,269.57990286073
millions=1 count=5 method="assign":    715,290.1400279875
millions=1 count=10 method="assign":   333,842.15674219833
millions=1 count=20 method="assign":    61,556.97763110506
millions=1 count=5 method="extend":  2,085,493.3048177995
millions=1 count=10 method="extend":   946,739.1148928251
millions=1 count=20 method="extend":   167,513.3600694919

Node.js 10.0.0 (V8 6.5) v8-canary 2018 01 14

millions=1 count=5 method="rest":      623,919.9508906119
millions=1 count=10 method="rest":     311,921.2618755684
millions=1 count=20 method="rest":      73,699.93172484756
millions=1 count=5 method="assign":    678,319.6777814393
millions=1 count=10 method="assign":   321,501.81129796285
millions=1 count=20 method="assign":    53,248.98222549836
millions=1 count=5 method="extend":  1,879,489.0915637205
millions=1 count=10 method="extend":   912,540.6587167263
millions=1 count=20 method="extend":   162,888.70809418708

Node.js 10.0.0 (V8 6.6) v8-canary 2018 01 29

millions=1 count=5 method="rest":      599,835.5249790836
millions=1 count=10 method="rest":     303,927.47418558795
millions=1 count=20 method="rest":      77,074.23960553356
millions=1 count=5 method="assign":    673,456.9757643664
millions=1 count=10 method="assign":   325,568.64380933647
millions=1 count=20 method="assign":    60,131.06157883637
millions=1 count=5 method="extend":  2,048,995.3304625815
millions=1 count=10 method="extend":   994,131.322691864
millions=1 count=20 method="extend":   179,953.2114847908

apapirovski commented Jan 30, 2018

Copy link
Copy Markdown
Contributor

ping @bmeurer re: the decline in performance mentioned above for object spread.

starkwang approved these changes Jan 30, 2018
View reviewed changes

starkwang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Small nits: rest operator -> spread operator.
In fact there is no rest operator but rest parameter :-)

lpinca approved these changes Jan 30, 2018
View reviewed changes

lpinca left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM with @vsemozhetbyt's comment addressed.

BridgeAR added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. and removed author ready PRs that have at least one approval, no outstanding review comments, and a CI started. labels Feb 1, 2018

BridgeAR commented Feb 1, 2018

Copy link
Copy Markdown
Member

Ping @jasnell

benchmark: rest operator benchmark
8d947fa 
Benchmark comparing `util._extend()`, `Object.assign()`,
and the rest operator for object assignment.

`util._extend()` still wins currently.
[Squash] Nits
61b67bc

jasnell commented Feb 1, 2018

Copy link
Copy Markdown
Member Author

Lint CI: https://ci.nodejs.org/job/node-test-linter/15723/

jasnell added a commit that referenced this pull request Feb 1, 2018
benchmark: spread operator benchmark
84cc832 
Benchmark comparing `util._extend()`, `Object.assign()`,
and the spread operator for object assignment.

`util._extend()` still wins currently.

PR-URL: #18442
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
BridgeAR added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 1, 2018

jasnell commented Feb 1, 2018

Copy link
Copy Markdown
Member Author

Landed in 84cc832

jasnell closed this Feb 1, 2018
addaleax removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 4, 2018
vsemozhetbyt mentioned this pull request Feb 11, 2018
Closed
jvelezpo mentioned this pull request Feb 13, 2018
Closed
3 tasks
MylesBorins pushed a commit that referenced this pull request Feb 20, 2018
benchmark: spread operator benchmark
dfa467f 
Benchmark comparing `util._extend()`, `Object.assign()`,
and the spread operator for object assignment.

`util._extend()` still wins currently.

PR-URL: #18442
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
benchmark: spread operator benchmark
2456716 
Benchmark comparing `util._extend()`, `Object.assign()`,
and the spread operator for object assignment.

`util._extend()` still wins currently.

PR-URL: #18442
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
MylesBorins pushed a commit that referenced this pull request Feb 21, 2018
benchmark: spread operator benchmark
ffbad83 
Benchmark comparing `util._extend()`, `Object.assign()`,
and the spread operator for object assignment.

`util._extend()` still wins currently.

PR-URL: #18442
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
MylesBorins mentioned this pull request Feb 21, 2018
Merged
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
benchmark: spread operator benchmark
4b8bea8 
Benchmark comparing `util._extend()`, `Object.assign()`,
and the spread operator for object assignment.

`util._extend()` still wins currently.

PR-URL: #18442
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>

MylesBorins commented Mar 20, 2018

Copy link
Copy Markdown
Contributor

Should this be backported to v8.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
benchmark: spread operator benchmark
a4fda89 
Benchmark comparing `util._extend()`, `Object.assign()`,
and the spread operator for object assignment.

`util._extend()` still wins currently.

PR-URL: nodejs#18442
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lpinca lpinca approved these changes
cjihrig cjihrig approved these changes
BridgeAR BridgeAR approved these changes
+3 more reviewers
vsemozhetbyt vsemozhetbyt left review comments
hiroppy hiroppy approved these changes
starkwang starkwang approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

benchmark Issues and PRs related to the benchmark subsystem. performance Issues and PRs related to the performance of Node.js. v8 engine Issues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Back | FazBrowse Home | New Git URL