From 3a776929074aa912ceb1fdb50488f3da32b38652 Mon Sep 17 00:00:00 2001 From: Ian Perkins Date: Fri, 18 Aug 2017 12:09:57 +0100 Subject: [PATCH 1/2] dns: fix issues in dns benchmark 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. The dns benchmark uses the core dns.lookup() function which does not access the network but instead uses "an operating system facility that can associate names with addresses, and vice versa" e.g. similar to ping; however, it is a synchronous call which runs on the libuv threadpool - the number of test calls was therefore reduced to 5e4 (from 5e6). --- benchmark/dns/lookup.js | 6 +++--- test/parallel/test-benchmark-dns.js | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-benchmark-dns.js diff --git a/benchmark/dns/lookup.js b/benchmark/dns/lookup.js index ebe9d05695ef23..90832b37ed4382 100644 --- a/benchmark/dns/lookup.js +++ b/benchmark/dns/lookup.js @@ -5,7 +5,7 @@ const lookup = require('dns').lookup; const bench = common.createBenchmark(main, { name: ['', '127.0.0.1', '::1'], - all: [true, false], + all: ['true', 'false'], n: [5e6] }); @@ -18,7 +18,7 @@ function main(conf) { if (all) { const opts = { all: true }; bench.start(); - (function cb(err, results) { + (function cb() { if (i++ === n) { bench.end(n); return; @@ -27,7 +27,7 @@ function main(conf) { })(); } else { bench.start(); - (function cb(err, result) { + (function cb() { if (i++ === n) { bench.end(n); return; diff --git a/test/parallel/test-benchmark-dns.js b/test/parallel/test-benchmark-dns.js new file mode 100644 index 00000000000000..afe4811f85c59d --- /dev/null +++ b/test/parallel/test-benchmark-dns.js @@ -0,0 +1,25 @@ +'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=5e4', + 'dns'], + { env }); + +child.on('exit', (code, signal) => { + assert.strictEqual(code, 0); + assert.strictEqual(signal, null); +}); From 83c6d6ba8b359ad189456eeb58686a9fc844dbcb Mon Sep 17 00:00:00 2001 From: Ian Perkins Date: Wed, 23 Aug 2017 20:53:57 +0200 Subject: [PATCH 2/2] benchmark: fix issues in dns 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. The dns benchmark uses the core dns.lookup() function which does not access the network. To improve performance the tests are limited to 1 invocation to a single endpoint. --- benchmark/dns/lookup.js | 2 +- test/parallel/test-benchmark-dns.js | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/benchmark/dns/lookup.js b/benchmark/dns/lookup.js index 90832b37ed4382..bb562d528c5b37 100644 --- a/benchmark/dns/lookup.js +++ b/benchmark/dns/lookup.js @@ -12,7 +12,7 @@ const bench = common.createBenchmark(main, { 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) { diff --git a/test/parallel/test-benchmark-dns.js b/test/parallel/test-benchmark-dns.js index afe4811f85c59d..ba15ad8c0d2fa9 100644 --- a/test/parallel/test-benchmark-dns.js +++ b/test/parallel/test-benchmark-dns.js @@ -15,7 +15,9 @@ const env = Object.assign({}, process.env, { NODEJS_BENCHMARK_ZERO_ALLOWED: 1 }); const child = fork(runjs, - ['--set', 'n=5e4', + ['--set', 'n=1', + '--set', 'all=false', + '--set', 'name=127.0.0.1', 'dns'], { env });