Skip to content

Commit ee126e3

Browse files
jasnelladuh95
authored andcommitted
perf_hooks: allow RecordableHistogram to record 0
Previously the lowest value accepted was 1. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode PR-URL: #66114 Fixes: #41641 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent afb61d6 commit ee126e3

11 files changed

Lines changed: 224 additions & 18 deletions

‎doc/api/perf_hooks.md‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2858,9 +2858,17 @@ Adds the values from `other` to this histogram.
28582858
added:
28592859
- v15.9.0
28602860
- v14.18.0
2861+
changes:
2862+
- version: REPLACEME
2863+
pr-url: https://github.com/nodejs/node/pull/66114
2864+
description: Recording `0` is now supported.
28612865
-->
28622866

2863-
* `val` {number|bigint} The amount to record in the histogram.
2867+
* `val` {number|bigint} The amount to record in the histogram. Must be an
2868+
integer greater than or equal to `0`.
2869+
2870+
Values smaller than the histogram's `lowest` option, including `0`, might not
2871+
be distinguishable from each other.
28642872

28652873
### `histogram.recordDelta()`
28662874

@@ -2877,9 +2885,14 @@ previous call to `recordDelta()` and records that amount in the histogram.
28772885

28782886
<!-- YAML
28792887
added: v26.8.0
2888+
changes:
2889+
- version: REPLACEME
2890+
pr-url: https://github.com/nodejs/node/pull/66114
2891+
description: Recording `0` is now supported.
28802892
-->
28812893

2882-
* `val` {number|bigint} The value to record.
2894+
* `val` {number|bigint} The value to record. Must be an integer greater than or
2895+
equal to `0`.
28832896
* `expectedInterval` {number|bigint} The expected recording interval.
28842897

28852898
Records a value with coordinated omission correction. When a system stall
@@ -2920,7 +2933,8 @@ call `snapshot()` to materialize the current window as a {Histogram}.
29202933
added: v26.10.0
29212934
-->
29222935

2923-
* `val` {number|bigint} The amount to record.
2936+
* `val` {number|bigint} The amount to record. Must be an integer greater than or
2937+
equal to `0`.
29242938

29252939
Records `val` in the current chunk. For a count-based window, every call that
29262940
reaches the native histogram counts toward rotation, including values which

‎lib/internal/bench_runner/benchmark.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -526,10 +526,7 @@ function summarizeSamples(samples) {
526526
const scale = MathMin(1_000_000, NumberMAX_SAFE_INTEGER / max);
527527
const histogram = createHistogram({ __proto__: null, figures: 5 });
528528
for (let i = 0; i < rates.length; i++) {
529-
const value = MathMax(
530-
1,
531-
MathMin(NumberMAX_SAFE_INTEGER, MathRound(rates[i] * scale)),
532-
);
529+
const value = MathMin(NumberMAX_SAFE_INTEGER, MathRound(rates[i] * scale));
533530
histogram.record(value);
534531
}
535532

‎lib/internal/histogram.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ class RecordableHistogram extends Histogram {
769769
return;
770770
}
771771

772-
validateInteger(val, 'val', 1);
772+
validateInteger(val, 'val', 0);
773773

774774
this[kHandle]?.record(val);
775775
}
@@ -802,7 +802,7 @@ class RecordableHistogram extends Histogram {
802802
this[kHandle]?.recordCorrected(val, expectedInterval);
803803
return;
804804
}
805-
validateInteger(val, 'val', 1);
805+
validateInteger(val, 'val', 0);
806806
validateInteger(expectedInterval, 'expectedInterval', 1);
807807
this[kHandle]?.recordCorrected(val, expectedInterval);
808808
}
@@ -864,7 +864,7 @@ class SlidingWindowHistogram {
864864
return;
865865
}
866866

867-
validateInteger(val, 'val', 1);
867+
validateInteger(val, 'val', 0);
868868
this[kSlidingWindowHandle].record(val);
869869
}
870870

‎src/histogram.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,15 +2149,15 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
21492149
int64_t value = args[0]->IsBigInt()
21502150
? args[0].As<BigInt>()->Int64Value(&lossless)
21512151
: static_cast<int64_t>(args[0].As<Number>()->Value());
2152-
if (!lossless || value < 1)
2152+
if (!lossless || value < 0)
21532153
return THROW_ERR_OUT_OF_RANGE(env, "value is out of range");
21542154
HistogramBase* histogram;
21552155
ASSIGN_OR_RETURN_UNWRAP(&histogram, args.This());
21562156
(*histogram)->Record(value);
21572157
}
21582158

21592159
void HistogramBase::FastRecord(Local<Value> receiver, const int64_t value) {
2160-
CHECK_GE(value, 1);
2160+
CHECK_GE(value, 0);
21612161
TRACK_V8_FAST_API_CALL("histogram.record");
21622162
HistogramBase* histogram;
21632163
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
@@ -2198,7 +2198,7 @@ void HistogramBase::RecordCorrected(const FunctionCallbackInfo<Value>& args) {
21982198
int64_t value = args[0]->IsBigInt()
21992199
? args[0].As<BigInt>()->Int64Value(&lossless)
22002200
: static_cast<int64_t>(args[0].As<Number>()->Value());
2201-
if (!lossless || value < 1)
2201+
if (!lossless || value < 0)
22022202
return THROW_ERR_OUT_OF_RANGE(env, "value is out of range");
22032203
int64_t expected_interval =
22042204
args[1]->IsBigInt() ? args[1].As<BigInt>()->Int64Value(&lossless)
@@ -2523,7 +2523,7 @@ void SlidingWindowHistogram::Record(const FunctionCallbackInfo<Value>& args) {
25232523
const int64_t value =
25242524
args[0]->IsBigInt() ? args[0].As<BigInt>()->Int64Value(&lossless)
25252525
: static_cast<int64_t>(args[0].As<Number>()->Value());
2526-
if (!lossless || value < 1)
2526+
if (!lossless || value < 0)
25272527
return THROW_ERR_OUT_OF_RANGE(env, "value is out of range");
25282528

25292529
SlidingWindowHistogram* histogram;
@@ -2535,7 +2535,7 @@ void SlidingWindowHistogram::FastRecord(Local<Value> receiver,
25352535
int64_t value,
25362536
// NOLINTNEXTLINE(runtime/references)
25372537
FastApiCallbackOptions& options) {
2538-
CHECK_GE(value, 1);
2538+
CHECK_GE(value, 0);
25392539
TRACK_V8_FAST_API_CALL("histogram.slidingWindow.record");
25402540
SlidingWindowHistogram* histogram;
25412541
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,29 @@ const { createRunner } = require('node:bench');
103103
operations: 1,
104104
}), { code: 'ERR_INVALID_STATE' });
105105
assert.throws(() => closedContext.done(), { code: 'ERR_INVALID_STATE' });
106+
107+
// A rate of 2e-7 operations per second is below the resolution of the
108+
// histogram used to summarize these samples, so it is recorded as zero. It
109+
// must not raise the median confidence interval above the median.
110+
const slowRunner = createRunner({ yieldBetweenSamples: false });
111+
const slowSample =
112+
{ __proto__: null, duration_ns: 5_000_000_000_000_000n, operations: 1 };
113+
const fastSample =
114+
{ __proto__: null, duration_ns: 1_000_000_000n, operations: 1 };
115+
const slowSamples =
116+
[slowSample, slowSample, slowSample, fastSample, fastSample];
117+
const slowCompletion = slowRunner.bench('sub-resolution rates', {
118+
samples: slowSamples.length,
119+
}, common.mustCall((b) => {
120+
b.record(slowSamples[b.index]);
121+
}, slowSamples.length));
122+
123+
await slowRunner.run().toArray();
124+
const slow = await slowCompletion;
125+
assert.deepStrictEqual(
126+
slow.samples.map(({ rate }) => rate), [2e-7, 2e-7, 2e-7, 1, 1]);
127+
const { median, medianConfidenceInterval } = slow.summary;
128+
assert.strictEqual(median, 2e-7);
129+
assert.strictEqual(medianConfidenceInterval.lower <= median, true);
130+
assert.strictEqual(median <= medianConfidenceInterval.upper, true);
106131
})().then(common.mustCall());

‎test/parallel/test-perf-hooks-histogram-analysis.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ const { inspect } = require('util');
400400
{ code: 'ERR_INVALID_ARG_TYPE' });
401401

402402
// Out of range
403-
assert.throws(() => h.recordCorrected(0, 10),
403+
assert.throws(() => h.recordCorrected(-1, 10),
404404
{ code: 'ERR_OUT_OF_RANGE' });
405405
assert.throws(() => h.recordCorrected(100, 0),
406406
{ code: 'ERR_OUT_OF_RANGE' });

‎test/parallel/test-perf-hooks-histogram-fast-calls.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,3 +33,15 @@ if (common.isDebug) {
3333
assert.strictEqual(getV8FastApiCallCount('histogram.percentile'), 1);
3434
assert.strictEqual(getV8FastApiCallCount('histogram.reset'), 1);
3535
}
36+
37+
{
38+
// Zero is accepted by the fast API call.
39+
histogram.record(0);
40+
assert.strictEqual(histogram.count, 1);
41+
assert.strictEqual(histogram.min, 0);
42+
43+
if (common.isDebug) {
44+
const { getV8FastApiCallCount } = internalBinding('debug');
45+
assert.strictEqual(getV8FastApiCallCount('histogram.record'), 2);
46+
}
47+
}
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
'use strict';
2+
3+
// Tests that histograms can record a value of zero.
4+
5+
require('../common');
6+
const assert = require('assert');
7+
const {
8+
createHistogram,
9+
createSlidingWindowHistogram,
10+
importHistogram,
11+
} = require('perf_hooks');
12+
13+
{
14+
const h = createHistogram();
15+
h.record(0);
16+
h.record(-0);
17+
h.record(0n);
18+
19+
assert.strictEqual(h.count, 3);
20+
assert.strictEqual(h.exceeds, 0);
21+
assert.strictEqual(h.min, 0);
22+
assert.strictEqual(h.minBigInt, 0n);
23+
assert.strictEqual(h.max, 0);
24+
assert.strictEqual(h.maxBigInt, 0n);
25+
assert.strictEqual(h.mean, 0);
26+
assert.strictEqual(h.stddev, 0);
27+
assert.strictEqual(h.percentile(50), 0);
28+
assert.strictEqual(h.percentileBigInt(100), 0n);
29+
assert.deepStrictEqual(h.percentiles, new Map([[0, 0], [100, 0]]));
30+
}
31+
32+
{
33+
const h = createHistogram();
34+
h.record(5);
35+
// A zero recorded after a non-zero value becomes the minimum.
36+
h.record(0);
37+
h.record(0);
38+
h.record(3);
39+
40+
assert.strictEqual(h.count, 4);
41+
assert.strictEqual(h.min, 0);
42+
assert.strictEqual(h.max, 5);
43+
assert.strictEqual(h.mean, 2);
44+
assert.strictEqual(h.countAt(0), 2);
45+
assert.strictEqual(h.cdf(0), 0.5);
46+
assert.strictEqual(h.percentile(50), 0);
47+
assert.strictEqual(h.percentile(75), 3);
48+
}
49+
50+
{
51+
const h = createHistogram();
52+
for (const value of [-1, -1n, Number.MIN_SAFE_INTEGER, -(2n ** 63n)]) {
53+
assert.throws(() => h.record(value), { code: 'ERR_OUT_OF_RANGE' });
54+
}
55+
assert.strictEqual(h.count, 0);
56+
}
57+
58+
{
59+
const h = createHistogram();
60+
h.recordCorrected(0, 10);
61+
h.recordCorrected(0n, 10n);
62+
63+
assert.strictEqual(h.count, 2);
64+
assert.strictEqual(h.min, 0);
65+
assert.strictEqual(h.max, 0);
66+
67+
for (const args of [[-1, 10], [-1n, 10n], [0, 0], [0n, 0n]]) {
68+
assert.throws(() => h.recordCorrected(...args),
69+
{ code: 'ERR_OUT_OF_RANGE' });
70+
}
71+
assert.strictEqual(h.count, 2);
72+
}
73+
74+
{
75+
const a = createHistogram();
76+
a.record(0);
77+
a.record(0);
78+
a.record(7);
79+
80+
const b = createHistogram();
81+
b.add(a);
82+
assert.strictEqual(b.count, 3);
83+
assert.strictEqual(b.min, 0);
84+
assert.strictEqual(b.max, 7);
85+
assert.strictEqual(b.countAt(0), 2);
86+
87+
const zero = createHistogram();
88+
zero.record(0);
89+
90+
b.subtract(zero);
91+
assert.strictEqual(b.count, 2);
92+
assert.strictEqual(b.min, 0);
93+
assert.strictEqual(b.countAt(0), 1);
94+
95+
b.subtract(zero);
96+
assert.strictEqual(b.count, 1);
97+
assert.strictEqual(b.min, 7);
98+
assert.strictEqual(b.countAt(0), 0);
99+
}
100+
101+
for (const values of [[0], [0, 0, 7]]) {
102+
const h = createHistogram();
103+
for (const value of values) h.record(value);
104+
105+
const imported = importHistogram(h.export());
106+
assert.strictEqual(imported.count, values.length);
107+
assert.strictEqual(imported.min, 0);
108+
assert.strictEqual(imported.max, h.max);
109+
assert.strictEqual(imported.countAt(0), h.countAt(0));
110+
assert.deepStrictEqual(imported.percentiles, h.percentiles);
111+
}
112+
113+
{
114+
// Values smaller than `lowest`, including zero, might not be distinguishable
115+
// from each other.
116+
const h = createHistogram({ lowest: 1000 });
117+
h.record(0);
118+
h.record(1);
119+
120+
assert.strictEqual(h.count, 2);
121+
assert.strictEqual(h.min, 0);
122+
assert.strictEqual(h.countAt(0), 2);
123+
}
124+
125+
{
126+
const histogram = createSlidingWindowHistogram({
127+
chunks: 2,
128+
recordsPerChunk: 2,
129+
});
130+
histogram.record(0);
131+
histogram.record(0n);
132+
histogram.record(3);
133+
134+
const snapshot = histogram.snapshot();
135+
assert.strictEqual(snapshot.count, 3);
136+
assert.strictEqual(snapshot.min, 0);
137+
assert.strictEqual(snapshot.max, 3);
138+
assert.strictEqual(snapshot.countAt(0), 2);
139+
140+
for (const value of [-1, -1n]) {
141+
assert.throws(() => histogram.record(value), { code: 'ERR_OUT_OF_RANGE' });
142+
}
143+
assert.strictEqual(histogram.snapshot().count, 3);
144+
}

‎test/parallel/test-perf-hooks-histogram.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const { inspect } = require('util');
3636
code: 'ERR_INVALID_ARG_TYPE'
3737
});
3838
});
39-
[0, Number.MAX_SAFE_INTEGER + 1].forEach((i) => {
39+
[-1, Number.MAX_SAFE_INTEGER + 1].forEach((i) => {
4040
assert.throws(() => h.record(i), {
4141
code: 'ERR_OUT_OF_RANGE'
4242
});

‎test/parallel/test-perf-hooks-sliding-window-histogram-fast-calls.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,17 @@ if (common.isDebug) {
2929
assert.strictEqual(
3030
getV8FastApiCallCount('histogram.slidingWindow.record'), 1);
3131
}
32+
33+
{
34+
// Zero is accepted by the fast API call.
35+
histogram.record(0);
36+
const snapshot = histogram.snapshot();
37+
assert.strictEqual(snapshot.count, 2);
38+
assert.strictEqual(snapshot.min, 0);
39+
40+
if (common.isDebug) {
41+
const { getV8FastApiCallCount } = internalBinding('debug');
42+
assert.strictEqual(
43+
getV8FastApiCallCount('histogram.slidingWindow.record'), 2);
44+
}
45+
}

0 commit comments

Comments
 (0)