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.

Sign up for GitHub

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

test: reduce run time for misc benchmark tests #16120

Closed
wants to merge 1 commit into from
Closed

test: reduce run time for misc benchmark tests #16120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions benchmark/misc/console.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ function runUsingArgumentsAndApply(n, concat) {
function main(conf) {
const n = +conf.n;
switch (conf.method) {
// '' is a default case for tests
Copy link
Member

@fhinkel fhinkel Oct 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Why are we always running the first case? Are the default cases missing a break? I'd rather have explicit calls then the first one as default. (Maybe I shouldn't comment if I'm missing context)

Copy link
Member Author

@Trott Trott Oct 12, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@fhinkel We are limiting to the first case in tests only. conf.method is assigned a default value earlier in the code (line 16). Tests override it with a blank string by using --set method= as a command line option.

The reason is so that we don't have to change parameter names in all the benchmarks.

Let's say test-benchmark-misc runs a benchmark called foo.js. It takes an configuration option called method. The method option may be readFile or readFileSync. If it's not one of those, the benchmark throws.

But test-benchmark-misc also runs a benchmark called bar.js. It also takes a method option but throws if they are anything other than chdir or chmod.

It is now impossible to specify a configuration option (so that each benchmark file only runs a single benchmark) for method without causing one or the other to throw.

Rather than try to figure out unique names for each benchmark file (which seems like an anti-pattern--I don't want to have to know that foo uses method but bar uses function or something like that), we assign a default for the case where the configuration option is explicitly set to an empty value.

Copy link
Member

Choose a reason for hiding this comment

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

That makes a lot of sense Thanks for the explanation!

case '':
case 'restAndSpread':
runUsingRestAndSpread(n, conf.concat);
break;
Expand Down
2 changes: 2 additions & 0 deletions benchmark/misc/object-property-bench.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ function main(conf) {
const n = +conf.millions * 1e6;

switch (conf.method) {
// '' is a default case for tests
case '':
case 'property':
runProperty(n);
break;
Expand Down
2 changes: 2 additions & 0 deletions benchmark/misc/punycode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ function main(conf) {
const n = +conf.n;
const val = conf.val;
switch (conf.method) {
// '' is a default case for tests
case '':
case 'punycode':
runPunycode(n, val);
break;
Expand Down
7 changes: 4 additions & 3 deletions test/parallel/test-benchmark-misc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ require('../common');
const runBenchmark = require('../common/benchmark');

runBenchmark('misc', [
'n=1',
'val=magyarország.icom.museum',
'concat=0',
'method=',
'millions=.000001',
'n=1',
'type=extend',
'concat=0'
'val=magyarország.icom.museum'
]);