Skip to content

Commit

Permalink
test: don't inspect values if not necessary
Browse files Browse the repository at this point in the history
The inspection triggered on each assert call eagerly even tough the
assertion was never triggered. That caused significant CPU overhead.

PR-URL: #22903
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
BridgeAR authored and targos committed Sep 18, 2018
1 parent f163c4a commit 8d5ab42
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 12 deletions.
25 changes: 19 additions & 6 deletions test/common/heap.js
Expand Up @@ -33,10 +33,17 @@ class State {
(node) => [expectedChild.name, 'Node / ' + expectedChild.name]
.includes(node.name);

assert(snapshot.some((node) => {
const hasChild = snapshot.some((node) => {
return node.outgoingEdges.map((edge) => edge.toNode).some(check);
}), `expected to find child ${util.inspect(expectedChild)} ` +
`in ${util.inspect(snapshot)}`);
});
// Don't use assert with a custom message here. Otherwise the
// inspection in the message is done eagerly and wastes a lot of CPU
// time.
if (!hasChild) {
throw new Error(
'expected to find child ' +
`${util.inspect(expectedChild)} in ${util.inspect(snapshot)}`);
}
}
}
}
Expand All @@ -57,9 +64,15 @@ class State {
node.value.constructor.name === expectedChild.name);
};

assert(graph.some((node) => node.edges.some(check)),
`expected to find child ${util.inspect(expectedChild)} ` +
`in ${util.inspect(snapshot)}`);
// Don't use assert with a custom message here. Otherwise the
// inspection in the message is done eagerly and wastes a lot of CPU
// time.
const hasChild = graph.some((node) => node.edges.some(check));
if (!hasChild) {
throw new Error(
'expected to find child ' +
`${util.inspect(expectedChild)} in ${util.inspect(snapshot)}`);
}
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion test/internet/test-trace-events-dns.js
Expand Up @@ -49,7 +49,12 @@ for (const tr in tests) {
{ encoding: 'utf8' });

// Make sure the operation is successful.
assert.strictEqual(proc.status, 0, `${tr}:\n${util.inspect(proc)}`);
// Don't use assert with a custom message here. Otherwise the
// inspection in the message is done eagerly and wastes a lot of CPU
// time.
if (proc.status !== 0) {
throw new Error(`${tr}:\n${util.inspect(proc)}`);
}

const file = path.join(tmpdir.path, traceFile);

Expand Down
7 changes: 6 additions & 1 deletion test/parallel/test-trace-events-fs-sync.js
Expand Up @@ -136,7 +136,12 @@ for (const tr in tests) {
}

// Make sure the operation is successful.
assert.strictEqual(proc.status, 0, `${tr}:\n${util.inspect(proc)}`);
// Don't use assert with a custom message here. Otherwise the
// inspection in the message is done eagerly and wastes a lot of CPU
// time.
if (proc.status !== 0) {
throw new Error(`${tr}:\n${util.inspect(proc)}`);
}

// Confirm that trace log file is created.
assert(fs.existsSync(traceFile));
Expand Down
12 changes: 8 additions & 4 deletions test/parallel/test-worker-message-port-transfer-self.js
Expand Up @@ -27,14 +27,18 @@ assert.throws(common.mustCall(() => {
port2.onmessage = common.mustCall((message) => {
assert.strictEqual(message, 2);

assert(util.inspect(port1).includes('active: true'), util.inspect(port1));
assert(util.inspect(port2).includes('active: true'), util.inspect(port2));
const inspectedPort1 = util.inspect(port1);
const inspectedPort2 = util.inspect(port2);
assert(inspectedPort1.includes('active: true'), inspectedPort1);
assert(inspectedPort2.includes('active: true'), inspectedPort2);

port1.close();

tick(10, () => {
assert(util.inspect(port1).includes('active: false'), util.inspect(port1));
assert(util.inspect(port2).includes('active: false'), util.inspect(port2));
const inspectedPort1 = util.inspect(port1);
const inspectedPort2 = util.inspect(port2);
assert(inspectedPort1.includes('active: false'), inspectedPort1);
assert(inspectedPort2.includes('active: false'), inspectedPort2);
});
});
port1.postMessage(2);
Expand Down

0 comments on commit 8d5ab42

Please sign in to comment.