From 667471ee8b75bda9c9bb5834bd0590dab270e9f6 Mon Sep 17 00:00:00 2001 From: Matheus Marchini Date: Tue, 15 Oct 2019 15:14:20 -0700 Subject: [PATCH] benchmark: improve `--filter` pattern matching * Let users provide more than one pattern by repeating the flag * Add new flag --exclude to exclude patterns * Add tests for --filter * Document --filter This commit also fixes a bug where things like `compare.js --new --old binary --new binary` was acceptable (now the script will exit and print the usage message). PR-URL: https://github.com/nodejs/node/pull/29987 Reviewed-By: Matteo Collina Reviewed-By: Denys Otrishko Reviewed-By: Rich Trott --- benchmark/_cli.js | 25 ++++++- benchmark/compare.js | 7 +- benchmark/run.js | 7 +- benchmark/writing-and-running-benchmarks.md | 82 +++++++++++++++++++++ test/parallel/test-benchmark-cli.js | 38 ++++++++++ 5 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-benchmark-cli.js diff --git a/benchmark/_cli.js b/benchmark/_cli.js index 8b9f22d0fb4b57..771cc72bff1964 100644 --- a/benchmark/_cli.js +++ b/benchmark/_cli.js @@ -34,7 +34,7 @@ function CLI(usage, settings) { if (arg === '--') { // Only items can follow -- mode = 'item'; - } else if (['both', 'option'].includes(mode) && arg[0] === '-') { + } else if ('both' === mode && arg[0] === '-') { // Optional arguments declaration if (arg[1] === '-') { @@ -82,13 +82,12 @@ CLI.prototype.abort = function(msg) { CLI.prototype.benchmarks = function() { const paths = []; - const filter = this.optional.filter || false; for (const category of this.items) { if (benchmarks[category] === undefined) continue; for (const scripts of benchmarks[category]) { - if (filter && scripts.lastIndexOf(filter) === -1) continue; + if (this.shouldSkip(scripts)) continue; paths.push(path.join(category, scripts)); } @@ -96,3 +95,23 @@ CLI.prototype.benchmarks = function() { return paths; }; + +CLI.prototype.shouldSkip = function(scripts) { + const filters = this.optional.filter || []; + const excludes = this.optional.exclude || []; + let skip = filters.length > 0; + + for (const filter of filters) { + if (scripts.lastIndexOf(filter) !== -1) { + skip = false; + } + } + + for (const exclude of excludes) { + if (scripts.lastIndexOf(exclude) !== -1) { + skip = true; + } + } + + return skip; +}; diff --git a/benchmark/compare.js b/benchmark/compare.js index bd7c4a95cbb617..53f82bb4b9f1b9 100644 --- a/benchmark/compare.js +++ b/benchmark/compare.js @@ -18,10 +18,13 @@ const cli = CLI(`usage: ./node compare.js [options] [--] ... --new ./new-node-binary new node binary (required) --old ./old-node-binary old node binary (required) --runs 30 number of samples - --filter pattern string to filter benchmark scripts + --filter pattern includes only benchmark scripts matching + (can be repeated) + --exclude pattern excludes scripts matching (can be + repeated) --set variable=value set benchmark variable (can be repeated) --no-progress don't show benchmark progress indicator -`, { arrayArgs: ['set'], boolArgs: ['no-progress'] }); +`, { arrayArgs: ['set', 'filter', 'exclude'], boolArgs: ['no-progress'] }); if (!cli.optional.new || !cli.optional.old) { cli.abort(cli.usage); diff --git a/benchmark/run.js b/benchmark/run.js index 2eb1ab1a4b0905..8e81a2c5e16ab7 100644 --- a/benchmark/run.js +++ b/benchmark/run.js @@ -8,10 +8,13 @@ const cli = CLI(`usage: ./node run.js [options] [--] ... Run each benchmark in the directory a single time, more than one directory can be specified. - --filter pattern string to filter benchmark scripts + --filter pattern includes only benchmark scripts matching + (can be repeated) + --exclude pattern excludes scripts matching (can be + repeated) --set variable=value set benchmark variable (can be repeated) --format [simple|csv] optional value that specifies the output format -`, { arrayArgs: ['set'] }); +`, { arrayArgs: ['set', 'filter', 'exclude'] }); const benchmarks = cli.benchmarks(); if (benchmarks.length === 0) { diff --git a/benchmark/writing-and-running-benchmarks.md b/benchmark/writing-and-running-benchmarks.md index 6097830aeeba1f..1db72d22de5324 100644 --- a/benchmark/writing-and-running-benchmarks.md +++ b/benchmark/writing-and-running-benchmarks.md @@ -8,6 +8,7 @@ * [Running benchmarks](#running-benchmarks) * [Running individual benchmarks](#running-individual-benchmarks) * [Running all benchmarks](#running-all-benchmarks) + * [Filtering benchmarks](#filtering-benchmarks) * [Comparing Node.js versions](#comparing-nodejs-versions) * [Comparing parameters](#comparing-parameters) * [Running Benchmarks on the CI](#running-benchmarks-on-the-ci) @@ -149,6 +150,87 @@ It is possible to execute more groups by adding extra process arguments. $ node benchmark/run.js assert async_hooks ``` +#### Filtering benchmarks + +`benchmark/run.js` and `benchmark/compare.js` have `--filter pattern` and +`--exclude pattern` options, which can be used to run a subset of benchmarks or +to exclude specific benchmarks from the execution, respectively. + +```console +$ node benchmark/run.js --filter "deepequal-b" assert + +assert/deepequal-buffer.js +assert/deepequal-buffer.js method="deepEqual" strict=0 len=100 n=20000: 773,200.4995493788 +assert/deepequal-buffer.js method="notDeepEqual" strict=0 len=100 n=20000: 964,411.712953848 + +$ node benchmark/run.js --exclude "deepequal-b" assert + +assert/deepequal-map.js +assert/deepequal-map.js method="deepEqual_primitiveOnly" strict=0 len=500 n=500: 20,445.06368453332 +assert/deepequal-map.js method="deepEqual_objectOnly" strict=0 len=500 n=500: 1,393.3481642240833 +... + +assert/deepequal-object.js +assert/deepequal-object.js method="deepEqual" strict=0 size=100 n=5000: 1,053.1950937538475 +assert/deepequal-object.js method="notDeepEqual" strict=0 size=100 n=5000: 9,734.193251965213 +... +``` + +`--filter` and `--exclude` can be repeated to provide multiple patterns. + +```console +$ node benchmark/run.js --filter "deepequal-b" --filter "deepequal-m" assert + +assert/deepequal-buffer.js +assert/deepequal-buffer.js method="deepEqual" strict=0 len=100 n=20000: 773,200.4995493788 +assert/deepequal-buffer.js method="notDeepEqual" strict=0 len=100 n=20000: 964,411.712953848 + +assert/deepequal-map.js +assert/deepequal-map.js method="deepEqual_primitiveOnly" strict=0 len=500 n=500: 20,445.06368453332 +assert/deepequal-map.js method="deepEqual_objectOnly" strict=0 len=500 n=500: 1,393.3481642240833 + +$ node benchmark/run.js --exclude "deepequal-b" --exclude "deepequal-m" assert + +assert/deepequal-object.js +assert/deepequal-object.js method="deepEqual" strict=0 size=100 n=5000: 1,053.1950937538475 +assert/deepequal-object.js method="notDeepEqual" strict=0 size=100 n=5000: 9,734.193251965213 +... + +assert/deepequal-prims-and-objs-big-array-set.js +assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual_Array" strict=0 len=20000 n=25 primitive="string": 865.2977195251661 +assert/deepequal-prims-and-objs-big-array-set.js method="notDeepEqual_Array" strict=0 len=20000 n=25 primitive="string": 827.8297281403861 +assert/deepequal-prims-and-objs-big-array-set.js method="deepEqual_Set" strict=0 len=20000 n=25 primitive="string": 28,826.618268696366 +... +``` + +If `--filter` and `--exclude` are used together, `--filter` is applied first, +and `--exclude` is applied on the result of `--filter`: + +```console +$ node benchmark/run.js --filter "bench-" process + +process/bench-env.js +process/bench-env.js operation="get" n=1000000: 2,356,946.0770617095 +process/bench-env.js operation="set" n=1000000: 1,295,176.3266261867 +process/bench-env.js operation="enumerate" n=1000000: 24,592.32231990992 +process/bench-env.js operation="query" n=1000000: 3,625,787.2150573144 +process/bench-env.js operation="delete" n=1000000: 1,521,131.5742806569 + +process/bench-hrtime.js +process/bench-hrtime.js type="raw" n=1000000: 13,178,002.113936031 +process/bench-hrtime.js type="diff" n=1000000: 11,585,435.712423025 +process/bench-hrtime.js type="bigint" n=1000000: 13,342,884.703919787 + +$ node benchmark/run.js --filter "bench-" --exclude "hrtime" process + +process/bench-env.js +process/bench-env.js operation="get" n=1000000: 2,356,946.0770617095 +process/bench-env.js operation="set" n=1000000: 1,295,176.3266261867 +process/bench-env.js operation="enumerate" n=1000000: 24,592.32231990992 +process/bench-env.js operation="query" n=1000000: 3,625,787.2150573144 +process/bench-env.js operation="delete" n=1000000: 1,521,131.5742806569 +``` + ### Comparing Node.js versions To compare the effect of a new Node.js version use the `compare.js` tool. This diff --git a/test/parallel/test-benchmark-cli.js b/test/parallel/test-benchmark-cli.js new file mode 100644 index 00000000000000..49069fecac43ce --- /dev/null +++ b/test/parallel/test-benchmark-cli.js @@ -0,0 +1,38 @@ +'use strict'; + +require('../common'); + +// This tests the CLI parser for our benchmark suite. + +const assert = require('assert'); + +const CLI = require('../../benchmark/_cli.js'); + +const originalArgv = process.argv; + +function testFilterPattern(filters, excludes, filename, expectedResult) { + process.argv = process.argv.concat(...filters.map((p) => ['--filter', p])); + process.argv = process.argv.concat(...excludes.map((p) => ['--exclude', p])); + process.argv = process.argv.concat(['bench']); + + const cli = new CLI('', { 'arrayArgs': ['filter', 'exclude'] }); + assert.deepStrictEqual(cli.shouldSkip(filename), expectedResult); + + process.argv = originalArgv; +} + + +testFilterPattern([], [], 'foo', false); + +testFilterPattern(['foo'], [], 'foo', false); +testFilterPattern(['foo'], [], 'bar', true); +testFilterPattern(['foo', 'bar'], [], 'foo', false); +testFilterPattern(['foo', 'bar'], [], 'bar', false); + +testFilterPattern([], ['foo'], 'foo', true); +testFilterPattern([], ['foo'], 'bar', false); +testFilterPattern([], ['foo', 'bar'], 'foo', true); +testFilterPattern([], ['foo', 'bar'], 'bar', true); + +testFilterPattern(['foo'], ['bar'], 'foo', false); +testFilterPattern(['foo'], ['bar'], 'foo-bar', true);