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: https://github.com/nodejs/node/pull/66114
Fixes: https://github.com/nodejs/node/issues/41641
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

doc/api/perf_hooks.md

@@ -2894,9 +2894,17 @@ Adds the values from `other` to this histogram.
 added:
   - v15.9.0
   - v14.18.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/66114
+    description: Recording `0` is now supported.
 -->
 
-* `val` {number|bigint} The amount to record in the histogram.
+* `val` {number|bigint} The amount to record in the histogram. Must be an
+  integer greater than or equal to `0`.
+
+Values smaller than the histogram's `lowest` option, including `0`, might not
+be distinguishable from each other.
 
 ### `histogram.recordDelta()`
 
@@ -2915,9 +2923,14 @@ previous call to `recordDelta()` and records that amount in the histogram.
 added:
  - v26.8.0
  - v24.21.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/66114
+    description: Recording `0` is now supported.
 -->
 
-* `val` {number|bigint} The value to record.
+* `val` {number|bigint} The value to record. Must be an integer greater than or
+  equal to `0`.
 * `expectedInterval` {number|bigint} The expected recording interval.
 
 Records a value with coordinated omission correction. When a system stall
@@ -2960,7 +2973,8 @@ call `snapshot()` to materialize the current window as a {Histogram}.
 added: v26.10.0
 -->
 
-* `val` {number|bigint} The amount to record.
+* `val` {number|bigint} The amount to record. Must be an integer greater than or
+  equal to `0`.
 
 Records `val` in the current chunk. For a count-based window, every call that
 reaches the native histogram counts toward rotation, including values which

lib/internal/bench_runner/benchmark.js

@@ -526,10 +526,7 @@ function summarizeSamples(samples) {
   const scale = MathMin(1_000_000, NumberMAX_SAFE_INTEGER / max);
   const histogram = createHistogram({ __proto__: null, figures: 5 });
   for (let i = 0; i < rates.length; i++) {
-    const value = MathMax(
-      1,
-      MathMin(NumberMAX_SAFE_INTEGER, MathRound(rates[i] * scale)),
-    );
+    const value = MathMin(NumberMAX_SAFE_INTEGER, MathRound(rates[i] * scale));
     histogram.record(value);
   }
 

lib/internal/histogram.js

@@ -769,7 +769,7 @@ class RecordableHistogram extends Histogram {
       return;
     }
 
-    validateInteger(val, 'val', 1);
+    validateInteger(val, 'val', 0);
 
     this[kHandle]?.record(val);
   }
@@ -802,7 +802,7 @@ class RecordableHistogram extends Histogram {
       this[kHandle]?.recordCorrected(val, expectedInterval);
       return;
     }
-    validateInteger(val, 'val', 1);
+    validateInteger(val, 'val', 0);
     validateInteger(expectedInterval, 'expectedInterval', 1);
     this[kHandle]?.recordCorrected(val, expectedInterval);
   }
@@ -864,7 +864,7 @@ class SlidingWindowHistogram {
       return;
     }
 
-    validateInteger(val, 'val', 1);
+    validateInteger(val, 'val', 0);
     this[kSlidingWindowHandle].record(val);
   }
 

src/histogram.cc

@@ -2149,7 +2149,7 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
   int64_t value = args[0]->IsBigInt()
                       ? args[0].As<BigInt>()->Int64Value(&lossless)
                       : static_cast<int64_t>(args[0].As<Number>()->Value());
-  if (!lossless || value < 1)
+  if (!lossless || value < 0)
     return THROW_ERR_OUT_OF_RANGE(env, "value is out of range");
   HistogramBase* histogram;
   ASSIGN_OR_RETURN_UNWRAP(&histogram, args.This());
@@ -2157,7 +2157,7 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
 }
 
 void HistogramBase::FastRecord(Local<Value> receiver, const int64_t value) {
-  CHECK_GE(value, 1);
+  CHECK_GE(value, 0);
   TRACK_V8_FAST_API_CALL("histogram.record");
   HistogramBase* histogram;
   ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
@@ -2198,7 +2198,7 @@ void HistogramBase::RecordCorrected(const FunctionCallbackInfo<Value>& args) {
   int64_t value = args[0]->IsBigInt()
                       ? args[0].As<BigInt>()->Int64Value(&lossless)
                       : static_cast<int64_t>(args[0].As<Number>()->Value());
-  if (!lossless || value < 1)
+  if (!lossless || value < 0)
     return THROW_ERR_OUT_OF_RANGE(env, "value is out of range");
   int64_t expected_interval =
       args[1]->IsBigInt() ? args[1].As<BigInt>()->Int64Value(&lossless)
@@ -2523,7 +2523,7 @@ void SlidingWindowHistogram::Record(const FunctionCallbackInfo<Value>& args) {
   const int64_t value =
       args[0]->IsBigInt() ? args[0].As<BigInt>()->Int64Value(&lossless)
                           : static_cast<int64_t>(args[0].As<Number>()->Value());
-  if (!lossless || value < 1)
+  if (!lossless || value < 0)
     return THROW_ERR_OUT_OF_RANGE(env, "value is out of range");
 
   SlidingWindowHistogram* histogram;
@@ -2535,7 +2535,7 @@ void SlidingWindowHistogram::FastRecord(Local<Value> receiver,
                                         int64_t value,
                                         // NOLINTNEXTLINE(runtime/references)
                                         FastApiCallbackOptions& options) {
-  CHECK_GE(value, 1);
+  CHECK_GE(value, 0);
   TRACK_V8_FAST_API_CALL("histogram.slidingWindow.record");
   SlidingWindowHistogram* histogram;
   ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);

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

@@ -103,4 +103,29 @@ const { createRunner } = require('node:bench');
     operations: 1,
   }), { code: 'ERR_INVALID_STATE' });
   assert.throws(() => closedContext.done(), { code: 'ERR_INVALID_STATE' });
+
+  // A rate of 2e-7 operations per second is below the resolution of the
+  // histogram used to summarize these samples, so it is recorded as zero. It
+  // must not raise the median confidence interval above the median.
+  const slowRunner = createRunner({ yieldBetweenSamples: false });
+  const slowSample =
+    { __proto__: null, duration_ns: 5_000_000_000_000_000n, operations: 1 };
+  const fastSample =
+    { __proto__: null, duration_ns: 1_000_000_000n, operations: 1 };
+  const slowSamples =
+    [slowSample, slowSample, slowSample, fastSample, fastSample];
+  const slowCompletion = slowRunner.bench('sub-resolution rates', {
+    samples: slowSamples.length,
+  }, common.mustCall((b) => {
+    b.record(slowSamples[b.index]);
+  }, slowSamples.length));
+
+  await slowRunner.run().toArray();
+  const slow = await slowCompletion;
+  assert.deepStrictEqual(
+    slow.samples.map(({ rate }) => rate), [2e-7, 2e-7, 2e-7, 1, 1]);
+  const { median, medianConfidenceInterval } = slow.summary;
+  assert.strictEqual(median, 2e-7);
+  assert.strictEqual(medianConfidenceInterval.lower <= median, true);
+  assert.strictEqual(median <= medianConfidenceInterval.upper, true);
 })().then(common.mustCall());

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

@@ -400,7 +400,7 @@ const { inspect } = require('util');
                 { code: 'ERR_INVALID_ARG_TYPE' });
 
   // Out of range
-  assert.throws(() => h.recordCorrected(0, 10),
+  assert.throws(() => h.recordCorrected(-1, 10),
                 { code: 'ERR_OUT_OF_RANGE' });
   assert.throws(() => h.recordCorrected(100, 0),
                 { code: 'ERR_OUT_OF_RANGE' });

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

@@ -33,3 +33,15 @@ if (common.isDebug) {
   assert.strictEqual(getV8FastApiCallCount('histogram.percentile'), 1);
   assert.strictEqual(getV8FastApiCallCount('histogram.reset'), 1);
 }
+
+{
+  // Zero is accepted by the fast API call.
+  histogram.record(0);
+  assert.strictEqual(histogram.count, 1);
+  assert.strictEqual(histogram.min, 0);
+
+  if (common.isDebug) {
+    const { getV8FastApiCallCount } = internalBinding('debug');
+    assert.strictEqual(getV8FastApiCallCount('histogram.record'), 2);
+  }
+}

test/parallel/test-perf-hooks-histogram-record-zero.js

@@ -0,0 +1,144 @@
+'use strict';
+
+// Tests that histograms can record a value of zero.
+
+require('../common');
+const assert = require('assert');
+const {
+  createHistogram,
+  createSlidingWindowHistogram,
+  importHistogram,
+} = require('perf_hooks');
+
+{
+  const h = createHistogram();
+  h.record(0);
+  h.record(-0);
+  h.record(0n);
+
+  assert.strictEqual(h.count, 3);
+  assert.strictEqual(h.exceeds, 0);
+  assert.strictEqual(h.min, 0);
+  assert.strictEqual(h.minBigInt, 0n);
+  assert.strictEqual(h.max, 0);
+  assert.strictEqual(h.maxBigInt, 0n);
+  assert.strictEqual(h.mean, 0);
+  assert.strictEqual(h.stddev, 0);
+  assert.strictEqual(h.percentile(50), 0);
+  assert.strictEqual(h.percentileBigInt(100), 0n);
+  assert.deepStrictEqual(h.percentiles, new Map([[0, 0], [100, 0]]));
+}
+
+{
+  const h = createHistogram();
+  h.record(5);
+  // A zero recorded after a non-zero value becomes the minimum.
+  h.record(0);
+  h.record(0);
+  h.record(3);
+
+  assert.strictEqual(h.count, 4);
+  assert.strictEqual(h.min, 0);
+  assert.strictEqual(h.max, 5);
+  assert.strictEqual(h.mean, 2);
+  assert.strictEqual(h.countAt(0), 2);
+  assert.strictEqual(h.cdf(0), 0.5);
+  assert.strictEqual(h.percentile(50), 0);
+  assert.strictEqual(h.percentile(75), 3);
+}
+
+{
+  const h = createHistogram();
+  for (const value of [-1, -1n, Number.MIN_SAFE_INTEGER, -(2n ** 63n)]) {
+    assert.throws(() => h.record(value), { code: 'ERR_OUT_OF_RANGE' });
+  }
+  assert.strictEqual(h.count, 0);
+}
+
+{
+  const h = createHistogram();
+  h.recordCorrected(0, 10);
+  h.recordCorrected(0n, 10n);
+
+  assert.strictEqual(h.count, 2);
+  assert.strictEqual(h.min, 0);
+  assert.strictEqual(h.max, 0);
+
+  for (const args of [[-1, 10], [-1n, 10n], [0, 0], [0n, 0n]]) {
+    assert.throws(() => h.recordCorrected(...args),
+                  { code: 'ERR_OUT_OF_RANGE' });
+  }
+  assert.strictEqual(h.count, 2);
+}
+
+{
+  const a = createHistogram();
+  a.record(0);
+  a.record(0);
+  a.record(7);
+
+  const b = createHistogram();
+  b.add(a);
+  assert.strictEqual(b.count, 3);
+  assert.strictEqual(b.min, 0);
+  assert.strictEqual(b.max, 7);
+  assert.strictEqual(b.countAt(0), 2);
+
+  const zero = createHistogram();
+  zero.record(0);
+
+  b.subtract(zero);
+  assert.strictEqual(b.count, 2);
+  assert.strictEqual(b.min, 0);
+  assert.strictEqual(b.countAt(0), 1);
+
+  b.subtract(zero);
+  assert.strictEqual(b.count, 1);
+  assert.strictEqual(b.min, 7);
+  assert.strictEqual(b.countAt(0), 0);
+}
+
+for (const values of [[0], [0, 0, 7]]) {
+  const h = createHistogram();
+  for (const value of values) h.record(value);
+
+  const imported = importHistogram(h.export());
+  assert.strictEqual(imported.count, values.length);
+  assert.strictEqual(imported.min, 0);
+  assert.strictEqual(imported.max, h.max);
+  assert.strictEqual(imported.countAt(0), h.countAt(0));
+  assert.deepStrictEqual(imported.percentiles, h.percentiles);
+}
+
+{
+  // Values smaller than `lowest`, including zero, might not be distinguishable
+  // from each other.
+  const h = createHistogram({ lowest: 1000 });
+  h.record(0);
+  h.record(1);
+
+  assert.strictEqual(h.count, 2);
+  assert.strictEqual(h.min, 0);
+  assert.strictEqual(h.countAt(0), 2);
+}
+
+{
+  const histogram = createSlidingWindowHistogram({
+    chunks: 2,
+    recordsPerChunk: 2,
+  });
+  histogram.record(0);
+  histogram.record(0n);
+  histogram.record(3);
+
+  const snapshot = histogram.snapshot();
+  assert.strictEqual(snapshot.count, 3);
+  assert.strictEqual(snapshot.min, 0);
+  assert.strictEqual(snapshot.max, 3);
+  assert.strictEqual(snapshot.countAt(0), 2);
+
+  for (const value of [-1, -1n]) {
+    assert.throws(() => histogram.record(value), { code: 'ERR_OUT_OF_RANGE' });
+  }
+  assert.strictEqual(histogram.snapshot().count, 3);
+}

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

@@ -36,7 +36,7 @@ const { inspect } = require('util');
       code: 'ERR_INVALID_ARG_TYPE'
     });
   });
-  [0, Number.MAX_SAFE_INTEGER + 1].forEach((i) => {
+  [-1, Number.MAX_SAFE_INTEGER + 1].forEach((i) => {
     assert.throws(() => h.record(i), {
       code: 'ERR_OUT_OF_RANGE'
     });

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

@@ -29,3 +29,17 @@ if (common.isDebug) {
   assert.strictEqual(
     getV8FastApiCallCount('histogram.slidingWindow.record'), 1);
 }
+
+{
+  // Zero is accepted by the fast API call.
+  histogram.record(0);
+  const snapshot = histogram.snapshot();
+  assert.strictEqual(snapshot.count, 2);
+  assert.strictEqual(snapshot.min, 0);
+
+  if (common.isDebug) {
+    const { getV8FastApiCallCount } = internalBinding('debug');
+    assert.strictEqual(
+      getV8FastApiCallCount('histogram.slidingWindow.record'), 2);
+  }
+}

test/parallel/test-perf-hooks-sliding-window-histogram.js

@@ -49,7 +49,7 @@ const {
   assert.strictEqual(histogram.snapshot().count, 0);
   histogram.record(10n);
   assert.strictEqual(histogram.snapshot().maxBigInt, 10n);
-  for (const value of [0n, 2n ** 63n]) {
+  for (const value of [-1n, 2n ** 63n]) {
     assert.throws(() => histogram.record(value), {
       code: 'ERR_OUT_OF_RANGE',
     });