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

benchmark: rest operator benchmark #18442

Closed
wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Jan 29, 2018

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 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
Copy link
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 vsemozhetbyt added the performance Issues and PRs related to the performance of Node.js. label Jan 29, 2018
@jasnell
Copy link
Member Author

jasnell commented Jan 29, 2018

Will try on Canary next

@cjihrig
Copy link
Contributor

cjihrig commented Jan 29, 2018

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

const util = require('util');

const bench = common.createBenchmark(main, {
method: ['rest', 'assign', '_extend'],
Copy link
Contributor

Choose a reason for hiding this comment

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

_extend -> extend, otherwise Error: Unexpected method

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Jan 30, 2018

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
Copy link
Member

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

Copy link
Contributor

@starkwang starkwang left a comment

Choose a reason for hiding this comment

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

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

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

LGTM with @vsemozhetbyt's comment addressed.

@BridgeAR BridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Feb 1, 2018
@BridgeAR
Copy link
Member

BridgeAR commented Feb 1, 2018

Ping @jasnell

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

`util._extend()` still wins currently.
@jasnell
Copy link
Member Author

jasnell commented Feb 1, 2018

jasnell added a commit that referenced this pull request Feb 1, 2018
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 BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 1, 2018
@jasnell
Copy link
Member Author

jasnell commented Feb 1, 2018

Landed in 84cc832

@jasnell jasnell closed this Feb 1, 2018
@addaleax addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 4, 2018
MylesBorins pushed a commit that referenced this pull request Feb 20, 2018
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 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 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 MylesBorins mentioned this pull request Feb 21, 2018
MylesBorins pushed a commit that referenced this pull request Mar 20, 2018
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
Copy link
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 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.