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

benchmarks: add microbenchmarks for new ES6 features #6222

Closed
wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 15, 2016

Checklist
  • tests and code linting passes
  • a test and/or benchmark is included
  • the commit message follows commit guidelines
Affected core subsystem(s)

benchmarks

Description of change

Adds new microbenchmarks for destructuring, rest params and default params.

Now that v8 v5 has landed, it makes sense to track the perf of various new language features. These are very simple raw micro-benchmarks.

Current results from master:

bash-3.2$ ./node benchmark/misc/restparams-bench.js 
misc/restparams-bench.js method=old millions=100: 11.42148
misc/restparams-bench.js method=new millions=100: 23.56694
misc/restparams-bench.js method=arguments millions=100: 14.49330

bash-3.2$ ./node benchmark/misc/defaultparams-bench.js 
misc/defaultparams-bench.js method=old millions=100: 26.88026
misc/defaultparams-bench.js method=new millions=100: 25.99777

bash-3.2$ ./node benchmark/misc/destructuring-bench.js 
misc/destructuring-bench.js method=old millions=100: 2230.47279
misc/destructuring-bench.js method=new millions=100: 15.22549
bash-3.2$ 

( Note how much slower the new destructuring assignment is for a simple var swap :-/ )

@jasnell jasnell added the benchmark Issues and PRs related to the benchmark subsystem. label Apr 15, 2016
@jasnell
Copy link
Member Author

jasnell commented Apr 15, 2016

@nodejs/benchmarking

@mscdex
Copy link
Contributor

mscdex commented Apr 15, 2016

I feel like these kinds of benchmarks should have their own, specific subdirectory rather than living in misc/.

var i = 0;
bench.start();
for (; i < n; i++)
b(1, 2, 'a', 'b');
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be calling c(), not b().

Copy link
Member Author

Choose a reason for hiding this comment

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

ha! stupid cut n paste errors

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think a(), b(), and c() should be renamed to something more descriptive.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

bench.end(n / 1e6);
}

function runSwapNew(n) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this should be named something like runSwapDestructure or something and the other one like runSwapManual.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@jasnell
Copy link
Member Author

jasnell commented Apr 15, 2016

@mscdex ... done! benchmarks moved to a separate dir

runUseArguments(n);
break;
default:
throw new Error('Unexpected');
Copy link
Contributor

@mscdex mscdex Apr 15, 2016

Choose a reason for hiding this comment

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

s/Unexpected/Unexpected method/

Same for the other benchmark files.

It's useful to be able to force optimization of a function.
Rather than duplicating the code everywhere for it, let's
make a utility available.
Adds new microbenchmarks for destructuring, rest params and
default params.
@jasnell
Copy link
Member Author

jasnell commented Apr 15, 2016

@mscdex ... done. Also, I moved the v8 force optimization code out to a utility function so it can be easily reused.

@mscdex
Copy link
Contributor

mscdex commented Apr 15, 2016

I'm not enough of a v8 expert to know if moving the forced optimization out to a separate function changes behavior at all or not (optimizing via a var/let/const variable vs a function parameter variable?).

@jasnell
Copy link
Member Author

jasnell commented Apr 15, 2016

running a few tests it did not appear to change the behavior at all.

@jasnell
Copy link
Member Author

jasnell commented Apr 15, 2016

@vkurchatkin ... any insights?

@mscdex
Copy link
Contributor

mscdex commented Apr 15, 2016

@jasnell
Copy link
Member Author

jasnell commented Apr 20, 2016

mscdex... LGTY?

@jasnell
Copy link
Member Author

jasnell commented Apr 20, 2016

running tests on the optimization code here and it does not appear that there is any difference in how it optimizes when called like this... at least no obvious difference

@mscdex
Copy link
Contributor

mscdex commented Apr 20, 2016

LGTM then

jasnell added a commit that referenced this pull request Apr 20, 2016
It's useful to be able to force optimization of a function.
Rather than duplicating the code everywhere for it, let's
make a utility available.

PR-URL: #6222
Reviewed-By: Brian White <mscdex@mscdex.net>
jasnell added a commit that referenced this pull request Apr 20, 2016
Adds new microbenchmarks for destructuring, rest params and
default params.

PR-URL: #6222
Reviewed-By: Brian White <mscdex@mscdex.net>
@jasnell
Copy link
Member Author

jasnell commented Apr 20, 2016

Landed in cb9eff2 and 7dc1a87

@jasnell jasnell closed this Apr 20, 2016
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
It's useful to be able to force optimization of a function.
Rather than duplicating the code everywhere for it, let's
make a utility available.

PR-URL: nodejs#6222
Reviewed-By: Brian White <mscdex@mscdex.net>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
Adds new microbenchmarks for destructuring, rest params and
default params.

PR-URL: nodejs#6222
Reviewed-By: Brian White <mscdex@mscdex.net>
jasnell added a commit that referenced this pull request Apr 26, 2016
It's useful to be able to force optimization of a function.
Rather than duplicating the code everywhere for it, let's
make a utility available.

PR-URL: #6222
Reviewed-By: Brian White <mscdex@mscdex.net>
jasnell added a commit that referenced this pull request Apr 26, 2016
Adds new microbenchmarks for destructuring, rest params and
default params.

PR-URL: #6222
Reviewed-By: Brian White <mscdex@mscdex.net>
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants