From e9a0912848cf15be8a6b8d1887d7dc700cf9fdec Mon Sep 17 00:00:00 2001 From: Renegade334 Date: Wed, 9 Jul 2025 22:29:43 +0100 Subject: [PATCH] perf_hooks: fix histogram fast call signatures PR-URL: https://github.com/nodejs/node/pull/59600 Reviewed-By: Yagiz Nizipli Reviewed-By: Anna Henningsen --- src/histogram.cc | 54 +++++++++---------- src/histogram.h | 40 +++++--------- .../test-perf-hooks-histogram-fast-calls.js | 35 ++++++++++++ test/parallel/test-perf-hooks-histogram.js | 6 ++- 4 files changed, 77 insertions(+), 58 deletions(-) create mode 100644 test/parallel/test-perf-hooks-histogram-fast-calls.js diff --git a/src/histogram.cc b/src/histogram.cc index 982d3aa482..836a51b0e5 100644 --- a/src/histogram.cc +++ b/src/histogram.cc @@ -2,6 +2,7 @@ #include "base_object-inl.h" #include "histogram-inl.h" #include "memory_tracker-inl.h" +#include "node_debug.h" #include "node_errors.h" #include "node_external_reference.h" #include "util.h" @@ -11,10 +12,8 @@ namespace node { using v8::BigInt; using v8::CFunction; using v8::Context; -using v8::FastApiCallbackOptions; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; -using v8::HandleScope; using v8::Integer; using v8::Isolate; using v8::Local; @@ -162,8 +161,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo& args) { (*histogram)->RecordDelta(); } -void HistogramBase::FastRecordDelta(Local unused, - Local receiver) { +void HistogramBase::FastRecordDelta(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.recordDelta"); HistogramBase* histogram; ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver); (*histogram)->RecordDelta(); @@ -183,15 +182,9 @@ void HistogramBase::Record(const FunctionCallbackInfo& args) { (*histogram)->Record(value); } -void HistogramBase::FastRecord(Local unused, - Local receiver, - const int64_t value, - FastApiCallbackOptions& options) { - if (value < 1) { - HandleScope scope(options.isolate); - THROW_ERR_OUT_OF_RANGE(options.isolate, "value is out of range"); - return; - } +void HistogramBase::FastRecord(Local receiver, const int64_t value) { + CHECK_GE(value, 1); + TRACK_V8_FAST_API_CALL("histogram.record"); HistogramBase* histogram; ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver); (*histogram)->Record(value); @@ -428,9 +421,8 @@ void IntervalHistogram::Start(const FunctionCallbackInfo& args) { histogram->OnStart(args[0]->IsTrue() ? StartFlags::RESET : StartFlags::NONE); } -void IntervalHistogram::FastStart(Local unused, - Local receiver, - bool reset) { +void IntervalHistogram::FastStart(Local receiver, bool reset) { + TRACK_V8_FAST_API_CALL("histogram.start"); IntervalHistogram* histogram; ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver); histogram->OnStart(reset ? StartFlags::RESET : StartFlags::NONE); @@ -442,7 +434,8 @@ void IntervalHistogram::Stop(const FunctionCallbackInfo& args) { histogram->OnStop(); } -void IntervalHistogram::FastStop(Local unused, Local receiver) { +void IntervalHistogram::FastStop(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.stop"); IntervalHistogram* histogram; ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver); histogram->OnStop(); @@ -555,46 +548,51 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo& args) { (*histogram)->Reset(); } -void HistogramImpl::FastReset(Local unused, Local receiver) { +void HistogramImpl::FastReset(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.reset"); HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); (*histogram)->Reset(); } -double HistogramImpl::FastGetCount(Local unused, Local receiver) { +double HistogramImpl::FastGetCount(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.count"); HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Count()); } -double HistogramImpl::FastGetMin(Local unused, Local receiver) { +double HistogramImpl::FastGetMin(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.min"); HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Min()); } -double HistogramImpl::FastGetMax(Local unused, Local receiver) { +double HistogramImpl::FastGetMax(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.max"); HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Max()); } -double HistogramImpl::FastGetMean(Local unused, Local receiver) { +double HistogramImpl::FastGetMean(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.mean"); HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return (*histogram)->Mean(); } -double HistogramImpl::FastGetExceeds(Local unused, - Local receiver) { +double HistogramImpl::FastGetExceeds(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.exceeds"); HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Exceeds()); } -double HistogramImpl::FastGetStddev(Local unused, - Local receiver) { +double HistogramImpl::FastGetStddev(Local receiver) { + TRACK_V8_FAST_API_CALL("histogram.stddev"); HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return (*histogram)->Stddev(); } -double HistogramImpl::FastGetPercentile(Local unused, - Local receiver, +double HistogramImpl::FastGetPercentile(Local receiver, const double percentile) { + TRACK_V8_FAST_API_CALL("histogram.percentile"); HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver); return static_cast((*histogram)->Percentile(percentile)); } diff --git a/src/histogram.h b/src/histogram.h index 362e82e443..29303bd166 100644 --- a/src/histogram.h +++ b/src/histogram.h @@ -101,22 +101,14 @@ class HistogramImpl { static void GetPercentilesBigInt( const v8::FunctionCallbackInfo& args); - static void FastReset(v8::Local unused, - v8::Local receiver); - static double FastGetCount(v8::Local unused, - v8::Local receiver); - static double FastGetMin(v8::Local unused, - v8::Local receiver); - static double FastGetMax(v8::Local unused, - v8::Local receiver); - static double FastGetMean(v8::Local unused, - v8::Local receiver); - static double FastGetExceeds(v8::Local unused, - v8::Local receiver); - static double FastGetStddev(v8::Local unused, - v8::Local receiver); - static double FastGetPercentile(v8::Local unused, - v8::Local receiver, + static void FastReset(v8::Local receiver); + static double FastGetCount(v8::Local receiver); + static double FastGetMin(v8::Local receiver); + static double FastGetMax(v8::Local receiver); + static double FastGetMean(v8::Local receiver); + static double FastGetExceeds(v8::Local receiver); + static double FastGetStddev(v8::Local receiver); + static double FastGetPercentile(v8::Local receiver, const double percentile); static void AddMethods(v8::Isolate* isolate, @@ -165,13 +157,8 @@ class HistogramBase final : public BaseObject, public HistogramImpl { static void RecordDelta(const v8::FunctionCallbackInfo& args); static void Add(const v8::FunctionCallbackInfo& args); - static void FastRecord( - v8::Local unused, - v8::Local receiver, - const int64_t value, - v8::FastApiCallbackOptions& options); // NOLINT(runtime/references) - static void FastRecordDelta(v8::Local unused, - v8::Local receiver); + static void FastRecord(v8::Local receiver, const int64_t value); + static void FastRecordDelta(v8::Local receiver); HistogramBase( Environment* env, @@ -243,11 +230,8 @@ class IntervalHistogram final : public HandleWrap, public HistogramImpl { static void Start(const v8::FunctionCallbackInfo& args); static void Stop(const v8::FunctionCallbackInfo& args); - static void FastStart(v8::Local unused, - v8::Local receiver, - bool reset); - static void FastStop(v8::Local unused, - v8::Local receiver); + static void FastStart(v8::Local receiver, bool reset); + static void FastStop(v8::Local receiver); BaseObject::TransferMode GetTransferMode() const override { return TransferMode::kCloneable; diff --git a/test/parallel/test-perf-hooks-histogram-fast-calls.js b/test/parallel/test-perf-hooks-histogram-fast-calls.js new file mode 100644 index 0000000000..2017bf49ee --- /dev/null +++ b/test/parallel/test-perf-hooks-histogram-fast-calls.js @@ -0,0 +1,35 @@ +// Flags: --expose-internals --no-warnings --allow-natives-syntax +'use strict'; + +const common = require('../common'); +const assert = require('assert'); + +const { internalBinding } = require('internal/test/binding'); + +const histogram = require('perf_hooks').createHistogram(); + +function testFastMethods() { + histogram.record(1); + histogram.recordDelta(); + histogram.percentile(50); + histogram.reset(); +} + +eval('%PrepareFunctionForOptimization(histogram.record)'); +eval('%PrepareFunctionForOptimization(histogram.recordDelta)'); +eval('%PrepareFunctionForOptimization(histogram.percentile)'); +eval('%PrepareFunctionForOptimization(histogram.reset)'); +testFastMethods(); +eval('%OptimizeFunctionOnNextCall(histogram.record)'); +eval('%OptimizeFunctionOnNextCall(histogram.recordDelta)'); +eval('%OptimizeFunctionOnNextCall(histogram.percentile)'); +eval('%OptimizeFunctionOnNextCall(histogram.reset)'); +testFastMethods(); + +if (common.isDebug) { + const { getV8FastApiCallCount } = internalBinding('debug'); + assert.strictEqual(getV8FastApiCallCount('histogram.record'), 1); + assert.strictEqual(getV8FastApiCallCount('histogram.recordDelta'), 1); + assert.strictEqual(getV8FastApiCallCount('histogram.percentile'), 1); + assert.strictEqual(getV8FastApiCallCount('histogram.reset'), 1); +} diff --git a/test/parallel/test-perf-hooks-histogram.js b/test/parallel/test-perf-hooks-histogram.js index a93ae17e9f..7d9bd476a7 100644 --- a/test/parallel/test-perf-hooks-histogram.js +++ b/test/parallel/test-perf-hooks-histogram.js @@ -41,8 +41,10 @@ const { inspect } = require('util'); code: 'ERR_INVALID_ARG_TYPE' }); }); - throws(() => h.record(0, Number.MAX_SAFE_INTEGER + 1), { - code: 'ERR_OUT_OF_RANGE' + [0, Number.MAX_SAFE_INTEGER + 1].forEach((i) => { + throws(() => h.record(i), { + code: 'ERR_OUT_OF_RANGE' + }); }); strictEqual(h.min, 1);