Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit ee333c9

Browse filesBrowse files
joyeecheungaduh95
authored andcommitted
test: deflake test-perf-hooks-timerify-histogram-sync
The previous busy loop wasn't robust enough in making sure that the function runs for more than 1 nanosecond - and when it runs faster than that on a fast machine, it measures to 0 for nanosecond precision and throws a RangeErorr as histogram.record() only takes positive values. Update it to use Atomics.wait() to make sure that the function being measured runs for at least 1 millisecond so that the histogram always records a positive value. PR-URL: #60639 Fixes: #60638 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
1 parent a61e5d8 commit ee333c9
Copy full SHA for ee333c9

File tree

Expand file treeCollapse file tree

3 files changed

+15
-9
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+15
-9
lines changed
Open diff view settings
Collapse file

‎test/common/index.js‎

Copy file name to clipboardExpand all lines: test/common/index.js
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -931,6 +931,12 @@ function expectRequiredTLAError(err) {
931931
}
932932
}
933933

934+
function sleepSync(ms) {
935+
const sab = new SharedArrayBuffer(4);
936+
const i32 = new Int32Array(sab);
937+
Atomics.wait(i32, 0, 0, ms);
938+
}
939+
934940
const common = {
935941
allowGlobals,
936942
buildType,
@@ -982,6 +988,7 @@ const common = {
982988
skipIfInspectorDisabled,
983989
skipIfSQLiteMissing,
984990
spawnPromisified,
991+
sleepSync,
985992

986993
get enoughTestMem() {
987994
return require('os').totalmem() > 0x70000000; /* 1.75 Gb */
Collapse file

‎test/common/index.mjs‎

Copy file name to clipboardExpand all lines: test/common/index.mjs
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ const {
5050
skipIfInspectorDisabled,
5151
skipIfSQLiteMissing,
5252
spawnPromisified,
53+
sleepSync,
5354
} = common;
5455

5556
const getPort = () => common.PORT;
@@ -103,4 +104,5 @@ export {
103104
skipIfInspectorDisabled,
104105
skipIfSQLiteMissing,
105106
spawnPromisified,
107+
sleepSync,
106108
};
Collapse file
+6-9Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,19 @@
11
// Test that timerify works with histogram option for synchronous functions.
22

3-
import '../common/index.mjs';
3+
import { sleepSync } from '../common/index.mjs';
44
import assert from 'assert';
55
import { createHistogram, timerify } from 'perf_hooks';
6-
import { setTimeout as sleep } from 'timers/promises';
7-
8-
let _deadCode;
96

107
const histogram = createHistogram();
11-
const m = (a, b = 1) => {
12-
for (let i = 0; i < 1e3; i++)
13-
_deadCode = i;
8+
9+
const m = () => {
10+
// Deterministic blocking delay (~1 millisecond). The histogram operates on
11+
// nanosecond precision, so this should be sufficient to prevent zero timings.
12+
sleepSync(1);
1413
};
1514
const n = timerify(m, { histogram });
1615
assert.strictEqual(histogram.max, 0);
1716
for (let i = 0; i < 10; i++) {
1817
n();
19-
await sleep(10);
2018
}
21-
assert(_deadCode >= 0);
2219
assert.notStrictEqual(histogram.max, 0);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.