test: make node:bench test samples survive a coarse clock · nodejs/node@31f4bd7 · GitHub
Skip to content

Commit 31f4bd7

Browse files
codebytereaduh95
codebytere
authored and
aduh95
committed
test: make node:bench test samples survive a coarse clock
Most node:bench tests record a sample with `b.start(); process.hrtime.bigint(); b.end(1)`, counting on the extra clock read to make end() see a later timestamp than start(). On a debian12-x64 CI host the monotonic clock is coarse enough that all three reads returned the same value, so end() threw ERR_INVALID_STATE ("insufficient clock precision for benchmark sample") and test-bench-harness-errors failed; every test using the idiom can fail the same way there. Add test/common/bench.js with completeSample(), which spins until process.hrtime.bigint() moves between start() and end(), and use it at all of those sites, bench-runner fixtures included. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65780 Fixes: #64226 Refs: #59146 Refs: #54534 Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent e0fa189 commit 31f4bd7

21 files changed

Lines changed: 82 additions & 102 deletions

‎test/common/README.md‎

Lines changed: 16 additions & 0 deletions

‎test/common/bench.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
'use strict';
2+
3+
// Records a sample with start()/end() and at least one hrtime tick in
4+
// between, so its duration is non-zero even on hosts with a coarse clock.
5+
function completeSample(b, operations = 1, options = undefined) {
6+
b.start();
7+
const started = process.hrtime.bigint();
8+
let now = started;
9+
while (now === started) now = process.hrtime.bigint();
10+
return b.end(operations, options);
11+
}
12+
13+
module.exports = { completeSample };

‎test/fixtures/bench-runner/identity-child-a.cjs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
'use strict';
22

3+
const { completeSample } = require('../../common/bench');
34
const { bench } = require('node:bench');
45

56
module.exports = function declareChildA() {
67
bench('child a', { samples: 1 }, (b) => {
7-
b.start();
8-
process.hrtime.bigint();
9-
b.end(1);
8+
completeSample(b);
109
});
1110
};

‎test/fixtures/bench-runner/identity-child-b.cjs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
'use strict';
22

3+
const { completeSample } = require('../../common/bench');
34
const { bench } = require('node:bench');
45

56
module.exports = function declareChildB() {
67
bench('child b', { samples: 1 }, (b) => {
7-
b.start();
8-
process.hrtime.bigint();
9-
b.end(1);
8+
completeSample(b);
109
});
1110
};

‎test/fixtures/bench-runner/identity-preload.cjs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
'use strict';
22

3+
const { completeSample } = require('../../common/bench');
34
const { bench } = require('node:bench');
45

56
bench('preload identity', { samples: 1 }, (b) => {
6-
b.start();
7-
process.hrtime.bigint();
8-
b.end(1);
7+
completeSample(b);
98
});

‎test/fixtures/bench-runner/identity-shared.cjs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
'use strict';
22

3+
const { completeSample } = require('../../common/bench');
34
const { bench } = require('node:bench');
45

56
module.exports = function registerSharedIdentity() {
67
bench('shared identity', { samples: 1 }, (b) => {
7-
b.start();
8-
process.hrtime.bigint();
9-
b.end(1);
8+
completeSample(b);
109
});
1110
};

‎test/fixtures/bench-runner/load-error-after-declaration.cjs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
'use strict';
22

3+
const { completeSample } = require('../../common/bench');
34
const { bench } = require('node:bench');
45

56
bench('declared before load error', { samples: 1 }, (b) => {
6-
b.start();
7-
process.hrtime.bigint();
8-
b.end(1);
7+
completeSample(b);
98
});
109

1110
throw new Error('load failed after declaration');

‎test/fixtures/bench-runner/tools.cjs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22

3+
const { completeSample } = require('../../common/bench');
34
const { bench } = require('node:bench');
45

56
if (process.env.NODE_BENCH_PID_LOG !== undefined) {
@@ -10,11 +11,5 @@ if (process.env.NODE_BENCH_PID_LOG !== undefined) {
1011
for (const size of [1, 2]) {
1112
bench('tools/simple.js', {
1213
params: { method: 'loop', size },
13-
}, (b) => {
14-
let value = 0;
15-
b.start();
16-
for (let i = 0; i < 1_000; i++) value += size;
17-
b.end(1_000);
18-
if (value === 0) throw new Error('unreachable');
19-
});
14+
}, (b) => completeSample(b, 1_000));
2015
}

‎test/parallel/test-bench-context-control.js‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
'use strict';
33

44
const common = require('../common');
5+
const { completeSample } = require('../common/bench');
56
const assert = require('assert');
67
const { createRunner } = require('node:bench');
78

@@ -16,9 +17,7 @@ const { createRunner } = require('node:bench');
1617
}, common.mustCall((b) => {
1718
invocations.push(`${b.phase}:${b.index}`);
1819
const detail = { index: b.index, phase: b.phase };
19-
b.start();
20-
process.hrtime.bigint();
21-
const sample = b.end(2, { detail });
20+
const sample = completeSample(b, 2, { detail });
2221
detail.index = -1;
2322

2423
assert.strictEqual(sample.operations, 2);

‎test/parallel/test-bench-context-errors.js‎

Lines changed: 2 additions & 3 deletions

0 commit comments

Comments
 (0)