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

benchmark: reduce string concatenations #12455

Closed
wants to merge 4 commits into from
Closed

benchmark: reduce string concatenations #12455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cbfa141
benchmark: reduce string concatenations
vsemozhetbyt Apr 17, 2017
3c800ef
benchmark: add benchmark for string concatenations
vsemozhetbyt Apr 17, 2017
c61bdd7
benchmark: fix URL in _http-benchmarkers.js
vsemozhetbyt Apr 17, 2017
620a87c
benchmark: fix typo in _http-benchmarkers.js
vsemozhetbyt Apr 17, 2017
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
18 changes: 10 additions & 8 deletions benchmark/_benchmark_progress.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@
const readline = require('readline');

function pad(input, minLength, fill) {
var result = input + '';
return fill.repeat(Math.max(0, minLength - result.length)) + result;
var result = String(input);
var padding = fill.repeat(Math.max(0, minLength - result.length));
return `${padding}${result}`;
}

function fraction(numerator, denominator) {
const fdenominator = denominator + '';
const fdenominator = String(denominator);
const fnumerator = pad(numerator, fdenominator.length, ' ');
return `${fnumerator}/${fdenominator}`;
}
Expand Down Expand Up @@ -100,11 +101,12 @@ class BenchmarkProgress {
const percent = pad(Math.floor(completedRate * 100), 3, ' ');

const caption = finished ? 'Done\n' : this.currentFile;
return `[${getTime(diff)}|% ${percent}` +
`| ${fraction(completedFiles, scheduledFiles)} files ` +
`| ${fraction(completedRunsForFile, runsPerFile)} runs ` +
`| ${fraction(completedConfig, scheduledConfig)} configs]` +
`: ${caption} `;
return `[${getTime(diff)}|% ${
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW I'm not particularly a fan of this style to avoid the extra spacing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate?

Copy link
Contributor

Choose a reason for hiding this comment

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

Using ${ followed by a newline to avoid including spaces due to indentation.

percent}| ${
fraction(completedFiles, scheduledFiles)} files | ${
fraction(completedRunsForFile, runsPerFile)} runs | ${
fraction(completedConfig, scheduledConfig)} configs]: ${
caption} `;
}

updateProgress(finished) {
Expand Down
20 changes: 11 additions & 9 deletions benchmark/_http-benchmarkers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ const child_process = require('child_process');
const path = require('path');
const fs = require('fs');

const requirementsURL =
'https://github.com/nodejs/node/blob/master/doc/guides/writing-and-running-benchmarks.md#http-benchmark-requirements';

// The port used by servers and wrk
exports.PORT = process.env.PORT || 12346;

Expand Down Expand Up @@ -133,20 +136,19 @@ exports.run = function(options, callback) {
benchmarker: exports.default_http_benchmarker
}, options);
if (!options.benchmarker) {
callback(new Error('Could not locate required http benchmarker. See ' +
'https://github.com/nodejs/node/blob/master/doc/guides/writing-and-running-benchmarks.md##http-benchmark-requirements ' +
'for further instructions.'));
callback(new Error(`Could not locate required http benchmarker. See ${
requirementsURL} for further instructions.`));
return;
}
const benchmarker = benchmarkers[options.benchmarker];
if (!benchmarker) {
callback(new Error(`Requested benchmarker '${options.benchmarker}' is ` +
'not supported'));
callback(new Error(`Requested benchmarker '${
options.benchmarker}' is not supported`));
return;
}
if (!benchmarker.present) {
callback(new Error(`Requested benchmarker '${options.benchmarker}' is ` +
'not installed'));
callback(new Error(`Requested benchmarker '${
options.benchmarker}' is not installed`));
return;
}

Expand All @@ -172,8 +174,8 @@ exports.run = function(options, callback) {

const result = benchmarker.processResults(stdout);
if (result === undefined) {
callback(new Error(`${options.benchmarker} produced strange output: ` +
stdout, code));
callback(new Error(
`${options.benchmarker} produced strange output: ${stdout}`), code);
return;
}

Expand Down
13 changes: 11 additions & 2 deletions benchmark/assert/deepequal-typedarrays.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,17 @@
const common = require('../common.js');
const assert = require('assert');
const bench = common.createBenchmark(main, {
type: ('Int8Array Uint8Array Int16Array Uint16Array Int32Array Uint32Array ' +
'Float32Array Float64Array Uint8ClampedArray').split(' '),
type: [
'Int8Array',
'Uint8Array',
'Int16Array',
'Uint16Array',
'Int32Array',
'Uint32Array',
'Float32Array',
'Float64Array',
'Uint8ClampedArray',
],
n: [1],
method: ['strict', 'nonstrict'],
len: [1e6]
Expand Down
2 changes: 1 addition & 1 deletion benchmark/buffers/buffer-base64-decode-wrapped.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function main(conf) {
const linesCount = 8 << 16;
const bytesCount = charsPerLine * linesCount / 4 * 3;

const line = 'abcd'.repeat(charsPerLine / 4) + '\n';
const line = `${'abcd'.repeat(charsPerLine / 4)}\n`;
const data = line.repeat(linesCount);
// eslint-disable-next-line no-unescaped-regexp-dot
data.match(/./); // Flatten the string
Expand Down
8 changes: 1 addition & 7 deletions benchmark/buffers/buffer-bytelength.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function main(conf) {
} else {
for (var string of chars) {
// Strings must be built differently, depending on encoding
var data = buildString(string, len);
var data = string.repeat(len);
if (encoding === 'utf8') {
strings.push(data);
} else if (encoding === 'base64') {
Expand All @@ -54,9 +54,3 @@ function main(conf) {
}
bench.end(n);
}

function buildString(str, times) {
Copy link
Member

Choose a reason for hiding this comment

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

It's funny we even had a recursive implementation :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I am even sorry to retire it)

if (times === 1) return str;

return str + buildString(str, times - 1);
}
12 changes: 6 additions & 6 deletions benchmark/buffers/buffer-read.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ function main(conf) {
var len = +conf.millions * 1e6;
var clazz = conf.buf === 'fast' ? Buffer : require('buffer').SlowBuffer;
var buff = new clazz(8);
var fn = 'read' + conf.type;
var fn = `read${conf.type}`;
Copy link
Member

Choose a reason for hiding this comment

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

just a reminder that this will make it impossible to run these benchmarks to compare against anything older than 4.0.0. That's not an objection, by any means.

Copy link
Contributor Author

@vsemozhetbyt vsemozhetbyt Apr 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, but there were many other template literals in benchmarks before this edition, including common.js.

Copy link
Contributor

@mscdex mscdex Apr 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

At this point I don't think it really matters. I can't see anyone wanting to run the benchmarks for v0.10 or v0.12 anymore, especially since it wouldn't really do any good (it's not as if the V8 team can/will revert to v0.10/v0.12-era V8 code if there is a performance issue in node v4.x+).

Copy link
Member

Choose a reason for hiding this comment

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

Yep, as I said, it's not an objection. Just noting the change. We may want to let folks know just so that they're aware.

Copy link
Member

Choose a reason for hiding this comment

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

I think a lot of benchmarks don't run on v4.x or older anyways...I've seen people on IRC
asking why some benchmark doesn't run and turns out they are using v4.x to run it


buff.writeDoubleLE(0, 0, noAssert);
var testFunction = new Function('buff', [
'for (var i = 0; i !== ' + len + '; i++) {',
' buff.' + fn + '(0, ' + JSON.stringify(noAssert) + ');',
'}'
].join('\n'));
var testFunction = new Function('buff', `
for (var i = 0; i !== ${len}; i++) {
buff.${fn}(0, ${JSON.stringify(noAssert)});
}
`);
bench.start();
testFunction(buff);
bench.end(len / 1e6);
Expand Down
10 changes: 5 additions & 5 deletions benchmark/buffers/buffer-swap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ function createBuffer(len, aligned) {
}

function genMethod(method) {
const fnString =
'return function ' + method + '(n, buf) {' +
' for (var i = 0; i <= n; i++)' +
' buf.' + method + '();' +
'}';
const fnString = `
Copy link
Member

Choose a reason for hiding this comment

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

Definitely not a fan of multiline literals.

return function ${method}(n, buf) {
for (var i = 0; i <= n; i++)
buf.${method}();
}`;
return (new Function(fnString))();
}

Expand Down
22 changes: 11 additions & 11 deletions benchmark/buffers/buffer-write.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function main(conf) {
var len = +conf.millions * 1e6;
var clazz = conf.buf === 'fast' ? Buffer : require('buffer').SlowBuffer;
var buff = new clazz(8);
var fn = 'write' + conf.type;
var fn = `write${conf.type}`;

if (fn.match(/Int/))
benchInt(buff, fn, len, noAssert);
Expand All @@ -60,22 +60,22 @@ function main(conf) {

function benchInt(buff, fn, len, noAssert) {
var m = mod[fn];
var testFunction = new Function('buff', [
'for (var i = 0; i !== ' + len + '; i++) {',
' buff.' + fn + '(i & ' + m + ', 0, ' + JSON.stringify(noAssert) + ');',
'}'
].join('\n'));
var testFunction = new Function('buff', `
for (var i = 0; i !== ${len}; i++) {
buff.${fn}(i & ${m}, 0, ${JSON.stringify(noAssert)});
}
`);
bench.start();
testFunction(buff);
bench.end(len / 1e6);
}

function benchFloat(buff, fn, len, noAssert) {
var testFunction = new Function('buff', [
'for (var i = 0; i !== ' + len + '; i++) {',
' buff.' + fn + '(i, 0, ' + JSON.stringify(noAssert) + ');',
'}'
].join('\n'));
var testFunction = new Function('buff', `
Copy link
Member

Choose a reason for hiding this comment

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

much nicer

for (var i = 0; i !== ${len}; i++) {
buff.${fn}(i, 0, ${JSON.stringify(noAssert)});
}
`);
bench.start();
testFunction(buff);
bench.end(len / 1e6);
Expand Down
2 changes: 1 addition & 1 deletion benchmark/buffers/dataview-set.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function main(conf) {
var ab = new ArrayBuffer(8);
var dv = new DataView(ab, 0, 8);
var le = /LE$/.test(conf.type);
var fn = 'set' + conf.type.replace(/[LB]E$/, '');
var fn = `set${conf.type.replace(/[LB]E$/, '')}`;

if (/int/i.test(fn))
benchInt(dv, fn, len, le);
Expand Down
4 changes: 2 additions & 2 deletions benchmark/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ Benchmark.prototype._parseArgs = function(argv, configs) {
for (const arg of argv) {
const match = arg.match(/^(.+?)=([\s\S]*)$/);
if (!match) {
console.error('bad argument: ' + arg);
console.error(`bad argument: ${arg}`);
process.exit(1);
}
const config = match[1];
Expand Down Expand Up @@ -206,7 +206,7 @@ function formatResult(data) {
// Construct configuration string, " A=a, B=b, ..."
let conf = '';
for (const key of Object.keys(data.conf)) {
conf += ' ' + key + '=' + JSON.stringify(data.conf[key]);
conf += ` ${key}=${JSON.stringify(data.conf[key])}`;
}

var rate = data.rate.toString().split('.');
Expand Down
6 changes: 3 additions & 3 deletions benchmark/compare.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,14 @@ if (showProgress) {
// Construct configuration string, " A=a, B=b, ..."
let conf = '';
for (const key of Object.keys(data.conf)) {
conf += ' ' + key + '=' + JSON.stringify(data.conf[key]);
conf += ` ${key}=${JSON.stringify(data.conf[key])}`;
}
conf = conf.slice(1);
// Escape quotes (") for correct csv formatting
conf = conf.replace(/"/g, '""');

console.log(`"${job.binary}", "${job.filename}", "${conf}", ` +
`${data.rate}, ${data.time}`);
console.log(`"${job.binary}", "${job.filename}", "${conf}", ${
data.rate}, ${data.time}`);
if (showProgress) {
// One item in the subqueue has been completed.
progress.completeConfig(data);
Expand Down
2 changes: 1 addition & 1 deletion benchmark/crypto/cipher-stream.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function main(conf) {
message = Buffer.alloc(conf.len, 'b');
break;
default:
throw new Error('unknown message type: ' + conf.type);
throw new Error(`unknown message type: ${conf.type}`);
}

var fn = api === 'stream' ? streamWrite : legacyWrite;
Expand Down
2 changes: 1 addition & 1 deletion benchmark/crypto/hash-stream-creation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function main(conf) {
message = Buffer.alloc(conf.len, 'b');
break;
default:
throw new Error('unknown message type: ' + conf.type);
throw new Error(`unknown message type: ${conf.type}`);
}

var fn = api === 'stream' ? streamWrite : legacyWrite;
Expand Down
2 changes: 1 addition & 1 deletion benchmark/crypto/hash-stream-throughput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function main(conf) {
message = Buffer.alloc(conf.len, 'b');
break;
default:
throw new Error('unknown message type: ' + conf.type);
throw new Error(`unknown message type: ${conf.type}`);
}

var fn = api === 'stream' ? streamWrite : legacyWrite;
Expand Down
8 changes: 4 additions & 4 deletions benchmark/crypto/rsa-encrypt-decrypt-throughput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ var RSA_PublicPem = {};
var RSA_PrivatePem = {};

keylen_list.forEach(function(key) {
RSA_PublicPem[key] = fs.readFileSync(fixtures_keydir +
'/rsa_public_' + key + '.pem');
RSA_PrivatePem[key] = fs.readFileSync(fixtures_keydir +
'/rsa_private_' + key + '.pem');
RSA_PublicPem[key] =
fs.readFileSync(`${fixtures_keydir}/rsa_public_${key}.pem`);
RSA_PrivatePem[key] =
fs.readFileSync(`${fixtures_keydir}/rsa_private_${key}.pem`);
});

var bench = common.createBenchmark(main, {
Expand Down
8 changes: 4 additions & 4 deletions benchmark/crypto/rsa-sign-verify-throughput.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ var RSA_PublicPem = {};
var RSA_PrivatePem = {};

keylen_list.forEach(function(key) {
RSA_PublicPem[key] = fs.readFileSync(fixtures_keydir +
'/rsa_public_' + key + '.pem');
RSA_PrivatePem[key] = fs.readFileSync(fixtures_keydir +
'/rsa_private_' + key + '.pem');
RSA_PublicPem[key] =
fs.readFileSync(`${fixtures_keydir}/rsa_public_${key}.pem`);
RSA_PrivatePem[key] =
fs.readFileSync(`${fixtures_keydir}/rsa_private_${key}.pem`);
});

var bench = common.createBenchmark(main, {
Expand Down
Loading