-
Notifications
You must be signed in to change notification settings - Fork 30.1k
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
Conversation
cc2bc39
to
91bd204
Compare
@nodejs/benchmarking |
I feel like these kinds of benchmarks should have their own, specific subdirectory rather than living in |
var i = 0; | ||
bench.start(); | ||
for (; i < n; i++) | ||
b(1, 2, 'a', 'b'); |
There was a problem hiding this comment.
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()
.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
91bd204
to
55e546a
Compare
bench.end(n / 1e6); | ||
} | ||
|
||
function runSwapNew(n) { |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
55e546a
to
6d5058e
Compare
@mscdex ... done! benchmarks moved to a separate dir |
runUseArguments(n); | ||
break; | ||
default: | ||
throw new Error('Unexpected'); |
There was a problem hiding this comment.
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.
f5b3d1e
to
5071349
Compare
@mscdex ... done. Also, I moved the v8 force optimization code out to a utility function so it can be easily reused. |
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 |
running a few tests it did not appear to change the behavior at all. |
@vkurchatkin ... any insights? |
mscdex... LGTY? |
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 |
LGTM then |
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>
Adds new microbenchmarks for destructuring, rest params and default params. PR-URL: #6222 Reviewed-By: Brian White <mscdex@mscdex.net>
Landed in cb9eff2 and 7dc1a87 |
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>
Adds new microbenchmarks for destructuring, rest params and default params. PR-URL: nodejs#6222 Reviewed-By: Brian White <mscdex@mscdex.net>
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>
Adds new microbenchmarks for destructuring, rest params and default params. PR-URL: #6222 Reviewed-By: Brian White <mscdex@mscdex.net>
Checklist
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:
( Note how much slower the new destructuring assignment is for a simple var swap :-/ )