Skip to content

Commit 9256a61

Browse files
jasnelladuh95
authored andcommitted
perf_hooks: fix truncation of monitorEventLoopDelay() resolution
`IntervalHistogram` stored the interval as `int32_t`, so a resolution above 2^31 - 1 ms wrapped: `resolution: 2 ** 32 + 1` sampled every millisecond. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: OpenCode PR-URL: #66115 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>
1 parent 478d846 commit 9256a61

10 files changed

Lines changed: 353 additions & 32 deletions

‎doc/api/perf_hooks.md‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1926,6 +1926,9 @@ are not guaranteed to reflect any correct state of the event loop.
19261926
<!-- YAML
19271927
added: v11.10.0
19281928
changes:
1929+
- version: REPLACEME
1930+
pr-url: https://github.com/nodejs/node/pull/66115
1931+
description: Added the `lowest`, `highest`, and `figures` options.
19291932
- version: v26.5.0
19301933
pr-url: https://github.com/nodejs/node/pull/62935
19311934
description: Added the `samplePerIteration` option.
@@ -1937,6 +1940,14 @@ changes:
19371940
* `resolution` {number} The sampling rate in milliseconds for interval-based
19381941
sampling. Must be greater than zero. This option is ignored when
19391942
`samplePerIteration` is `true`. **Default:** `10`.
1943+
* `lowest` {number|bigint} The lowest discernible delay, in nanoseconds. Must
1944+
be an integer value greater than `0`. **Default:** `1` when
1945+
`samplePerIteration` is `true`, otherwise `1000`.
1946+
* `highest` {number|bigint} The highest recordable delay, in nanoseconds.
1947+
Must be an integer value that is equal to or greater than two times
1948+
`lowest`. **Default:** `2n ** 63n - 1n`.
1949+
* `figures` {number} The number of accuracy digits. Must be an integer
1950+
between `1` and `5`. **Default:** `3`.
19401951
* Returns: {ELDHistogram}
19411952

19421953
_This property is an extension by Node.js. It is not available in Web browsers._
@@ -1952,6 +1963,16 @@ the application is idle.
19521963
The two sampling modes produce significantly different results and should not
19531964
be compared directly.
19541965

1966+
The `lowest`, `highest`, and `figures` options configure the histogram as they
1967+
do for [`perf_hooks.createHistogram()`][]. `lowest` must be greater than `0`
1968+
because an event loop delay of zero is not possible: the event loop has a
1969+
minimal overhead, and the measurement itself depends on the event loop turning.
1970+
Delays greater than `highest` are not recorded, and are counted by
1971+
[`histogram.exceeds`][] instead. With interval-based sampling, every sample
1972+
includes the `resolution`, so `highest` should be well above
1973+
`resolution * 1e6`. The histogram's memory use depends on these options, not
1974+
on the number of samples.
1975+
19551976
```mjs
19561977
import { monitorEventLoopDelay } from 'node:perf_hooks';
19571978

@@ -2239,8 +2260,8 @@ added: v11.10.0
22392260

22402261
* Type: {number}
22412262

2242-
The number of times the event loop delay exceeded the maximum 1 hour event
2243-
loop delay threshold.
2263+
The number of values that were not recorded because they exceeded the
2264+
histogram's highest recordable value.
22442265

22452266
### `histogram.exceedsBigInt`
22462267

@@ -2252,8 +2273,8 @@ added:
22522273

22532274
* Type: {bigint}
22542275

2255-
The number of times the event loop delay exceeded the maximum 1 hour event
2256-
loop delay threshold.
2276+
The number of values that were not recorded because they exceeded the
2277+
histogram's highest recordable value.
22572278

22582279
### `histogram.export()`
22592280

@@ -3392,7 +3413,9 @@ dns.promises.resolve('localhost');
33923413
[`'exit'`]: process.md#event-exit
33933414
[`child_process.spawnSync()`]: child_process.md#child_processspawnsynccommand-args-options
33943415
[`histogram.diff()`]: #histogramdiffother
3416+
[`histogram.exceeds`]: #histogramexceeds
33953417
[`histogram.export()`]: #histogramexport
3418+
[`perf_hooks.createHistogram()`]: #perf_hookscreatehistogramoptions
33963419
[`perf_hooks.createSlidingWindowHistogram()`]: #perf_hookscreateslidingwindowhistogramoptions
33973420
[`perf_hooks.eventLoopUtilization()`]: #perf_hookseventlooputilizationutilization1-utilization2
33983421
[`perf_hooks.importHistogram()`]: #perf_hooksimporthistogramdata

‎lib/internal/histogram.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,8 +1041,10 @@ module.exports = {
10411041
isHistogram,
10421042
kDestroy,
10431043
kHandle,
1044+
kMaxInt64,
10441045
kSkipThrow,
10451046
createHistogram,
10461047
createSlidingWindowHistogram,
10471048
importHistogram,
1049+
validateHistogramOptions,
10481050
};

‎lib/internal/perf/event_loop_delay.js‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use strict';
22
const {
3+
BigInt,
34
Symbol,
45
SymbolDispose,
56
} = primordials;
@@ -24,7 +25,9 @@ const {
2425
const {
2526
Histogram,
2627
kHandle,
28+
kMaxInt64,
2729
kSkipThrow,
30+
validateHistogramOptions,
2831
} = require('internal/histogram');
2932

3033
const {
@@ -37,6 +40,12 @@ const {
3740

3841
const kEnabled = Symbol('kEnabled');
3942

43+
// Default histogram options. The lowest discernible delay is in nanoseconds,
44+
// and its default depends on the sampling mode.
45+
const kDefaultIntervalLowest = 1000;
46+
const kDefaultIterationLowest = 1;
47+
const kDefaultFigures = 3;
48+
4049
class ELDHistogram extends Histogram {
4150
constructor(skipThrowSymbol = undefined) {
4251
if (skipThrowSymbol !== kSkipThrow) {
@@ -76,8 +85,11 @@ class ELDHistogram extends Histogram {
7685

7786
/**
7887
* @param {{
79-
* samplePerIteration : boolean,
80-
* resolution : number
88+
* samplePerIteration? : boolean,
89+
* resolution? : number,
90+
* lowest? : number|bigint,
91+
* highest? : number|bigint,
92+
* figures? : number,
8193
* }} [options]
8294
* @returns {ELDHistogram}
8395
*/
@@ -88,10 +100,22 @@ function monitorEventLoopDelay(options = kEmptyObject) {
88100
validateBoolean(samplePerIteration, 'options.samplePerIteration');
89101
validateInteger(resolution, 'options.resolution', 1);
90102

103+
const {
104+
lowest = samplePerIteration ?
105+
kDefaultIterationLowest : kDefaultIntervalLowest,
106+
highest = kMaxInt64,
107+
figures = kDefaultFigures,
108+
} = options;
109+
validateHistogramOptions(lowest, highest, figures);
110+
111+
// Throws if the native histogram cannot be created with these options.
112+
const handle = createELDHistogram(
113+
resolution, samplePerIteration, BigInt(lowest), BigInt(highest), figures);
114+
91115
const histogram = new ELDHistogram(kSkipThrow);
92116
markTransferMode(histogram, true, false);
93117
histogram[kEnabled] = false;
94-
histogram[kHandle] = createELDHistogram(resolution, samplePerIteration);
118+
histogram[kHandle] = handle;
95119
return histogram;
96120
}
97121

‎src/histogram.cc‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2616,11 +2616,11 @@ void IntervalHistogram::RegisterExternalReferences(
26162616
IntervalHistogram::IntervalHistogram(Environment* env,
26172617
Local<Object> wrap,
26182618
AsyncWrap::ProviderType type,
2619-
int32_t interval,
2619+
uint64_t interval,
26202620
OnInterval on_interval,
2621-
const Histogram::Options& options)
2621+
std::shared_ptr<Histogram> histogram)
26222622
: HandleWrap(env, wrap, reinterpret_cast<uv_handle_t*>(&timer_), type),
2623-
HistogramImpl(options),
2623+
HistogramImpl(std::move(histogram)),
26242624
interval_(interval),
26252625
on_interval_(on_interval) {
26262626
MakeWeak();
@@ -2633,9 +2633,9 @@ IntervalHistogram::IntervalHistogram(Environment* env,
26332633

26342634
BaseObjectPtr<IntervalHistogram> IntervalHistogram::Create(
26352635
Environment* env,
2636-
int32_t interval,
2636+
uint64_t interval,
26372637
OnInterval on_interval,
2638-
const Histogram::Options& options,
2638+
std::shared_ptr<Histogram> histogram,
26392639
AsyncWrap::ProviderType type) {
26402640
Local<Object> obj;
26412641
if (!GetConstructorTemplate(env)
@@ -2646,7 +2646,7 @@ BaseObjectPtr<IntervalHistogram> IntervalHistogram::Create(
26462646
}
26472647

26482648
return MakeBaseObject<IntervalHistogram>(
2649-
env, obj, type, interval, on_interval, options);
2649+
env, obj, type, interval, on_interval, std::move(histogram));
26502650
}
26512651

26522652
void IntervalHistogram::TimerCB(uv_timer_t* handle) {
@@ -2712,10 +2712,10 @@ void IterationHistogram::RegisterExternalReferences(
27122712
IterationHistogram::IterationHistogram(Environment* env,
27132713
Local<Object> wrap,
27142714
AsyncWrap::ProviderType type,
2715-
const Histogram::Options& options)
2715+
std::shared_ptr<Histogram> histogram)
27162716
: HandleWrap(
27172717
env, wrap, reinterpret_cast<uv_handle_t*>(&check_handle_), type),
2718-
HistogramImpl(options) {
2718+
HistogramImpl(std::move(histogram)) {
27192719
MakeWeak();
27202720
wrap->SetAlignedPointerInInternalField(
27212721
HistogramImpl::InternalFields::kImplField,
@@ -2729,7 +2729,7 @@ IterationHistogram::IterationHistogram(Environment* env,
27292729

27302730
BaseObjectPtr<IterationHistogram> IterationHistogram::Create(
27312731
Environment* env,
2732-
const Histogram::Options& options,
2732+
std::shared_ptr<Histogram> histogram,
27332733
AsyncWrap::ProviderType type) {
27342734
Local<Object> obj;
27352735
if (!GetConstructorTemplate(env)
@@ -2739,7 +2739,8 @@ BaseObjectPtr<IterationHistogram> IterationHistogram::Create(
27392739
return nullptr;
27402740
}
27412741

2742-
return MakeBaseObject<IterationHistogram>(env, obj, type, options);
2742+
return MakeBaseObject<IterationHistogram>(
2743+
env, obj, type, std::move(histogram));
27432744
}
27442745

27452746
void IterationHistogram::PrepareCB(uv_prepare_t* handle) {

‎src/histogram.h‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -499,17 +499,17 @@ class IntervalHistogram final : public HandleWrap,
499499

500500
static BaseObjectPtr<IntervalHistogram> Create(
501501
Environment* env,
502-
int32_t interval,
502+
uint64_t interval,
503503
OnInterval on_interval,
504-
const Histogram::Options& options,
504+
std::shared_ptr<Histogram> histogram,
505505
AsyncWrap::ProviderType type = AsyncWrap::PROVIDER_ELDHISTOGRAM);
506506

507507
IntervalHistogram(Environment* env,
508508
v8::Local<v8::Object> wrap,
509509
AsyncWrap::ProviderType type,
510-
int32_t interval,
510+
uint64_t interval,
511511
OnInterval on_interval,
512-
const Histogram::Options& options = Histogram::Options{});
512+
std::shared_ptr<Histogram> histogram);
513513

514514
static void FastStart(v8::Local<v8::Value> receiver, bool reset);
515515
static void FastStop(v8::Local<v8::Value> receiver);
@@ -534,7 +534,7 @@ class IntervalHistogram final : public HandleWrap,
534534
template <typename T>
535535
friend void StopHandleHistogram(v8::Local<v8::Value>);
536536

537-
int32_t interval_ = 0;
537+
uint64_t interval_ = 0;
538538
OnInterval on_interval_ = nullptr;
539539
uv_timer_t timer_;
540540

@@ -559,13 +559,13 @@ class IterationHistogram final
559559

560560
static BaseObjectPtr<IterationHistogram> Create(
561561
Environment* env,
562-
const Histogram::Options& options,
562+
std::shared_ptr<Histogram> histogram,
563563
AsyncWrap::ProviderType type = AsyncWrap::PROVIDER_ELDHISTOGRAM);
564564

565565
IterationHistogram(Environment* env,
566566
v8::Local<v8::Object> wrap,
567567
AsyncWrap::ProviderType type,
568-
const Histogram::Options& options = Histogram::Options{});
568+
std::shared_ptr<Histogram> histogram);
569569

570570
static void FastStart(v8::Local<v8::Value> receiver, bool reset);
571571
static void FastStop(v8::Local<v8::Value> receiver);

‎src/node_perf.cc‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#include "histogram-inl.h"
55
#include "memory_tracker-inl.h"
66
#include "node_buffer.h"
7+
#include "node_errors.h"
78
#include "node_external_reference.h"
89
#include "node_internals.h"
910
#include "node_process-inl.h"
@@ -14,6 +15,7 @@
1415
namespace node {
1516
namespace performance {
1617

18+
using v8::BigInt;
1719
using v8::Context;
1820
using v8::DontDelete;
1921
using v8::Function;
@@ -28,6 +30,7 @@ using v8::Object;
2830
using v8::ObjectTemplate;
2931
using v8::PropertyAttribute;
3032
using v8::ReadOnly;
33+
using v8::Uint32;
3134
using v8::Value;
3235

3336
// Microseconds in a millisecond, as a float.
@@ -318,14 +321,34 @@ void CreateELDHistogram(const FunctionCallbackInfo<Value>& args) {
318321
Environment* env = Environment::GetCurrent(args);
319322
int64_t interval = args[0].As<Integer>()->Value();
320323
CHECK_GT(interval, 0);
324+
CHECK(args[2]->IsBigInt());
325+
CHECK(args[3]->IsBigInt());
326+
CHECK(args[4]->IsUint32());
327+
bool lossless = true;
328+
const int64_t lowest = args[2].As<BigInt>()->Int64Value(&lossless);
329+
CHECK(lossless);
330+
const int64_t highest = args[3].As<BigInt>()->Int64Value(&lossless);
331+
CHECK(lossless);
332+
const int figures = static_cast<int>(args[4].As<Uint32>()->Value());
333+
334+
// The options are validated in JS, but hdr_init() still rejects some
335+
// combinations, such as a very large lowest value.
336+
std::shared_ptr<Histogram> histogram =
337+
Histogram::Create(Histogram::Options{lowest, highest, figures});
338+
if (!histogram) {
339+
return THROW_ERR_INVALID_ARG_VALUE(env, "Invalid histogram options");
340+
}
341+
321342
if (args[1]->IsTrue()) {
322-
BaseObjectPtr<IterationHistogram> histogram =
323-
IterationHistogram::Create(env, Histogram::Options{1});
324-
args.GetReturnValue().Set(histogram->object());
343+
BaseObjectPtr<IterationHistogram> eld =
344+
IterationHistogram::Create(env, std::move(histogram));
345+
if (eld) args.GetReturnValue().Set(eld->object());
325346
return;
326347
}
327-
BaseObjectPtr<IntervalHistogram> histogram =
328-
IntervalHistogram::Create(env, interval, [](Histogram& histogram) {
348+
BaseObjectPtr<IntervalHistogram> eld = IntervalHistogram::Create(
349+
env,
350+
interval,
351+
[](Histogram& histogram) {
329352
uint64_t delta = histogram.RecordDelta();
330353
TRACE_COUNTER1(TRACING_CATEGORY_NODE2(perf, event_loop),
331354
"delay", delta);
@@ -337,8 +360,9 @@ void CreateELDHistogram(const FunctionCallbackInfo<Value>& args) {
337360
"mean", histogram.Mean());
338361
TRACE_COUNTER1(TRACING_CATEGORY_NODE2(perf, event_loop),
339362
"stddev", histogram.Stddev());
340-
}, Histogram::Options { 1000 });
341-
args.GetReturnValue().Set(histogram->object());
363+
},
364+
std::move(histogram));
365+
if (eld) args.GetReturnValue().Set(eld->object());
342366
}
343367

344368
void MarkBootstrapComplete(const FunctionCallbackInfo<Value>& args) {

‎test/parallel/test-perf-hooks-monitor-event-loop-delay-fast-calls.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ const assert = require('assert');
77
const { internalBinding } = require('internal/test/binding');
88
const { createELDHistogram } = internalBinding('performance');
99

10-
const histogram = createELDHistogram(1, true);
10+
const histogram = createELDHistogram(1, true, 1n, 2n ** 63n - 1n, 3);
1111

1212
function testFastMethods() {
1313
histogram.start(true);

0 commit comments

Comments
 (0)