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 20, 2018
1 parent 624e516 commit e580a44
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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 e580a44

Please sign in to comment.