test: deflake test-perf-hooks-timerify-histogram-sync · nodejs/node@ee333c9 · GitHub

nodejs / node Public

• Notifications You must be signed in to change notification settings
• Fork 36.6k
• Star 120k

• Code
• Issues 635
• Pull requests 755
• Actions
• Projects
• Security and quality
• Insights

Additional navigation options

Commit ee333c9

Browse filesBrowse the repository at this point in the historyBrowse files

joyeecheung

authored and

aduh95

committed

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

3 files changed

+15-9Lines changed: 15 additions & 9 deletions

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

• test
  • common
    • index.js
    • index.mjs
  • parallel
    • test-perf-hooks-timerify-histogram-sync.mjs

Expand file treeCollapse file tree

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 number	Diff line number	Diff line change
@@ -931,6 +931,12 @@ function expectRequiredTLAError(err) {
931	931	}
932	932	}
933	933
	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	+
934	940	const common = {
935	941	allowGlobals,
936	942	buildType,
@@ -982,6 +988,7 @@ const common = {
982	988	skipIfInspectorDisabled,
983	989	skipIfSQLiteMissing,
984	990	spawnPromisified,
	991	+ sleepSync,
985	992
986	993	get enoughTestMem() {
987	994	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 number	Diff line number	Diff line change
@@ -50,6 +50,7 @@ const {
50	50	skipIfInspectorDisabled,
51	51	skipIfSQLiteMissing,
52	52	spawnPromisified,
	53	+ sleepSync,
53	54	} = common;
54	55
55	56	const getPort = () => common.PORT;
@@ -103,4 +104,5 @@ export {
103	104	skipIfInspectorDisabled,
104	105	skipIfSQLiteMissing,
105	106	spawnPromisified,
	107	+ sleepSync,
106	108	};

Collapse file

‎test/parallel/test-perf-hooks-timerify-histogram-sync.mjs‎

Copy file name to clipboard

+6-9Lines changed: 6 additions & 9 deletions

Original file line number	Diff line number	Diff line change
@@ -1,22 +1,19 @@
1	1	// Test that timerify works with histogram option for synchronous functions.
2	2
3		- import '../common/index.mjs';
	3	+ import { sleepSync } from '../common/index.mjs';
4	4	import assert from 'assert';
5	5	import { createHistogram, timerify } from 'perf_hooks';
6		- import { setTimeout as sleep } from 'timers/promises';
7		-
8		- let _deadCode;
9	6
10	7	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);
14	13	};
15	14	const n = timerify(m, { histogram });
16	15	assert.strictEqual(histogram.max, 0);
17	16	for (let i = 0; i < 10; i++) {
18	17	n();
19		- await sleep(10);
20	18	}
21		- assert(_deadCode >= 0);
22	19	assert.notStrictEqual(histogram.max, 0);

0 commit comments

Comments

0 (0)