Skip to content

Conversation

@suryagh
Copy link
Contributor

Checklist
  • make -j4 test (UNIX) or vcbuild test nosign (Windows) passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

benchmark

Description of change

To copy the values of all enumerable properties from-
a source object to a target object, node still use-
util._extend, though newer standard Object.assign
is available. This is because util._extend is found to
be faster than Object.assign. This benchmark test is
to keep track of how performance compare.

To copy the values of all enumerable own properties from- a source object to a target object, node still use- `util._extend`, though newer standard `Object.assign` is available. This is because `util._extend` is found to be faster than `Object.assign`. This benchmark test is to keep track of how performance compare.
@nodejs-github-botnodejs-github-bot added the benchmark Issues and PRs related to the benchmark subsystem. label Jun 10, 2016
@mscdexmscdex added util Issues and PRs related to the built-in util module. v8 engine Issues and PRs related to the V8 dependency. labels Jun 10, 2016

const bench = common.createBenchmark(main,{
type: ['util._extend', 'Object.assign',
'util._extend', 'Object.assign'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are there duplicates?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Removed.

if (conf.type === 'extend'){
fn = util._extend;
v8command = '%OptimizeFunctionOnNextCall(util._extend)'
} else if (conf.type === 'assign'){
Copy link
Member

Choose a reason for hiding this comment

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

FYI: https://github.com/nodejs/node/blob/master/benchmark/common.js#L255

the benchmark/common.js now has a method for handling the details of v8 optimization for you. e.g.

functionmyMethod(a,b){/** ... **/}common.v8ForceOptimization(myMethod,'a','b');myMethod('a','b');

@jasnell
Copy link
Member

Couple of minor nits but LGTM if @mscdex is happy with it.

v8command = '%OptimizeFunctionOnNextCall(util._extend)'
} else if (conf.type === 'assign'){
fn = Object.assign;
//Object.assign is built-in, cannot be optimized
Copy link
Contributor

Choose a reason for hiding this comment

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

Space needed after //.

@mscdex
Copy link
Contributor

One minor style nit, but otherwise LGTM

@suryagh
Copy link
ContributorAuthor

Fixed the minor style.

jasnell pushed a commit that referenced this pull request Jun 27, 2016
To copy the values of all enumerable own properties from- a source object to a target object, node still use- `util._extend`, though newer standard `Object.assign` is available. This is because `util._extend` is found to be faster than `Object.assign`. This benchmark test is to keep track of how performance compare. PR-URL: #7255 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Brian White <[email protected]>
@jasnell
Copy link
Member

Landed in 6abb06f

@jasnelljasnell closed this Jun 27, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
To copy the values of all enumerable own properties from- a source object to a target object, node still use- `util._extend`, though newer standard `Object.assign` is available. This is because `util._extend` is found to be faster than `Object.assign`. This benchmark test is to keep track of how performance compare. PR-URL: #7255 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Brian White <[email protected]>
@Fishrock123Fishrock123 mentioned this pull request Jul 5, 2016
@suryaghsuryagh deleted the bmark1 branch September 4, 2016 03:02
@cjihrigcjihrig mentioned this pull request Jan 17, 2017
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmarkIssues and PRs related to the benchmark subsystem.utilIssues and PRs related to the built-in util module.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@suryagh@jasnell@mscdex@Fishrock123@MylesBorins@nodejs-github-bot