Skip to content
This repository has been archived by the owner on Aug 31, 2018. It is now read-only.

Commit

Permalink
benchmark: fix issues in dns benchmark
Browse files Browse the repository at this point in the history
The benchmark script for dns contained functions with args declared
but never used. This fix removes those arguments from the function
signatures.

No test existed for the dns benchmark so one was added to the
parallel suite.
To improve performance the tests are limited to 1 invocation to a
single endpoint.

PR-URL: nodejs/node#14936
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
ian-perkins authored and Olivia Hugger committed Aug 30, 2017
1 parent c841263 commit e042c76
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
8 changes: 4 additions & 4 deletions benchmark/dns/lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,20 @@ const lookup = require('dns').lookup;

const bench = common.createBenchmark(main, {
name: ['', '127.0.0.1', '::1'],
all: [true, false],
all: ['true', 'false'],
n: [5e6]
});

function main(conf) {
const name = conf.name;
const n = +conf.n;
const all = !!conf.all;
const all = conf.all === 'true' ? true : false;
var i = 0;

if (all) {
const opts = { all: true };
bench.start();
(function cb(err, results) {
(function cb() {
if (i++ === n) {
bench.end(n);
return;
Expand All @@ -27,7 +27,7 @@ function main(conf) {
})();
} else {
bench.start();
(function cb(err, result) {
(function cb() {
if (i++ === n) {
bench.end(n);
return;
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-benchmark-dns.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';

require('../common');

// Minimal test for dns benchmarks. This makes sure the benchmarks aren't
// horribly broken but nothing more than that.

const assert = require('assert');
const fork = require('child_process').fork;
const path = require('path');

const runjs = path.join(__dirname, '..', '..', 'benchmark', 'run.js');

const env = Object.assign({}, process.env,
{ NODEJS_BENCHMARK_ZERO_ALLOWED: 1 });

const child = fork(runjs,
['--set', 'n=1',
'--set', 'all=false',
'--set', 'name=127.0.0.1',
'dns'],
{ env });

child.on('exit', (code, signal) => {
assert.strictEqual(code, 0);
assert.strictEqual(signal, null);
});

0 comments on commit e042c76

Please sign in to comment.