Skip to content

Commit e9a0912

Browse files
Renegade334nodejs-github-bot
authored andcommitted
perf_hooks: fix histogram fast call signatures
PR-URL: #59600 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent f5e2ecc commit e9a0912

File tree

4 files changed

+77
-58
lines changed

4 files changed

+77
-58
lines changed

src/histogram.cc

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#include "base_object-inl.h"
33
#include "histogram-inl.h"
44
#include "memory_tracker-inl.h"
5+
#include "node_debug.h"
56
#include "node_errors.h"
67
#include "node_external_reference.h"
78
#include "util.h"
@@ -11,10 +12,8 @@ namespace node {
1112
using v8::BigInt;
1213
using v8::CFunction;
1314
using v8::Context;
14-
using v8::FastApiCallbackOptions;
1515
using v8::FunctionCallbackInfo;
1616
using v8::FunctionTemplate;
17-
using v8::HandleScope;
1817
using v8::Integer;
1918
using v8::Isolate;
2019
using v8::Local;
@@ -162,8 +161,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo<Value>& args) {
162161
(*histogram)->RecordDelta();
163162
}
164163

165-
void HistogramBase::FastRecordDelta(Local<Value> unused,
166-
Local<Value> receiver) {
164+
void HistogramBase::FastRecordDelta(Local<Value> receiver) {
165+
TRACK_V8_FAST_API_CALL("histogram.recordDelta");
167166
HistogramBase* histogram;
168167
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
169168
(*histogram)->RecordDelta();
@@ -183,15 +182,9 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
183182
(*histogram)->Record(value);
184183
}
185184

186-
void HistogramBase::FastRecord(Local<Value> unused,
187-
Local<Value> receiver,
188-
const int64_t value,
189-
FastApiCallbackOptions& options) {
190-
if (value < 1) {
191-
HandleScope scope(options.isolate);
192-
THROW_ERR_OUT_OF_RANGE(options.isolate, "value is out of range");
193-
return;
194-
}
185+
void HistogramBase::FastRecord(Local<Value> receiver, const int64_t value) {
186+
CHECK_GE(value, 1);
187+
TRACK_V8_FAST_API_CALL("histogram.record");
195188
HistogramBase* histogram;
196189
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
197190
(*histogram)->Record(value);
@@ -428,9 +421,8 @@ void IntervalHistogram::Start(const FunctionCallbackInfo<Value>& args) {
428421
histogram->OnStart(args[0]->IsTrue() ? StartFlags::RESET : StartFlags::NONE);
429422
}
430423

431-
void IntervalHistogram::FastStart(Local<Value> unused,
432-
Local<Value> receiver,
433-
bool reset) {
424+
void IntervalHistogram::FastStart(Local<Value> receiver, bool reset) {
425+
TRACK_V8_FAST_API_CALL("histogram.start");
434426
IntervalHistogram* histogram;
435427
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
436428
histogram->OnStart(reset ? StartFlags::RESET : StartFlags::NONE);
@@ -442,7 +434,8 @@ void IntervalHistogram::Stop(const FunctionCallbackInfo<Value>& args) {
442434
histogram->OnStop();
443435
}
444436

445-
void IntervalHistogram::FastStop(Local<Value> unused, Local<Value> receiver) {
437+
void IntervalHistogram::FastStop(Local<Value> receiver) {
438+
TRACK_V8_FAST_API_CALL("histogram.stop");
446439
IntervalHistogram* histogram;
447440
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
448441
histogram->OnStop();
@@ -555,46 +548,51 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo<Value>& args) {
555548
(*histogram)->Reset();
556549
}
557550

558-
void HistogramImpl::FastReset(Local<Value> unused, Local<Value> receiver) {
551+
void HistogramImpl::FastReset(Local<Value> receiver) {
552+
TRACK_V8_FAST_API_CALL("histogram.reset");
559553
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
560554
(*histogram)->Reset();
561555
}
562556

563-
double HistogramImpl::FastGetCount(Local<Value> unused, Local<Value> receiver) {
557+
double HistogramImpl::FastGetCount(Local<Value> receiver) {
558+
TRACK_V8_FAST_API_CALL("histogram.count");
564559
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
565560
return static_cast<double>((*histogram)->Count());
566561
}
567562

568-
double HistogramImpl::FastGetMin(Local<Value> unused, Local<Value> receiver) {
563+
double HistogramImpl::FastGetMin(Local<Value> receiver) {
564+
TRACK_V8_FAST_API_CALL("histogram.min");
569565
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
570566
return static_cast<double>((*histogram)->Min());
571567
}
572568

573-
double HistogramImpl::FastGetMax(Local<Value> unused, Local<Value> receiver) {
569+
double HistogramImpl::FastGetMax(Local<Value> receiver) {
570+
TRACK_V8_FAST_API_CALL("histogram.max");
574571
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
575572
return static_cast<double>((*histogram)->Max());
576573
}
577574

578-
double HistogramImpl::FastGetMean(Local<Value> unused, Local<Value> receiver) {
575+
double HistogramImpl::FastGetMean(Local<Value> receiver) {
576+
TRACK_V8_FAST_API_CALL("histogram.mean");
579577
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
580578
return (*histogram)->Mean();
581579
}
582580

583-
double HistogramImpl::FastGetExceeds(Local<Value> unused,
584-
Local<Value> receiver) {
581+
double HistogramImpl::FastGetExceeds(Local<Value> receiver) {
582+
TRACK_V8_FAST_API_CALL("histogram.exceeds");
585583
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
586584
return static_cast<double>((*histogram)->Exceeds());
587585
}
588586

589-
double HistogramImpl::FastGetStddev(Local<Value> unused,
590-
Local<Value> receiver) {
587+
double HistogramImpl::FastGetStddev(Local<Value> receiver) {
588+
TRACK_V8_FAST_API_CALL("histogram.stddev");
591589
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
592590
return (*histogram)->Stddev();
593591
}
594592

595-
double HistogramImpl::FastGetPercentile(Local<Value> unused,
596-
Local<Value> receiver,
593+
double HistogramImpl::FastGetPercentile(Local<Value> receiver,
597594
const double percentile) {
595+
TRACK_V8_FAST_API_CALL("histogram.percentile");
598596
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
599597
return static_cast<double>((*histogram)->Percentile(percentile));
600598
}

src/histogram.h

Lines changed: 12 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -101,22 +101,14 @@ class HistogramImpl {
101101
static void GetPercentilesBigInt(
102102
const v8::FunctionCallbackInfo<v8::Value>& args);
103103

104-
static void FastReset(v8::Local<v8::Value> unused,
105-
v8::Local<v8::Value> receiver);
106-
static double FastGetCount(v8::Local<v8::Value> unused,
107-
v8::Local<v8::Value> receiver);
108-
static double FastGetMin(v8::Local<v8::Value> unused,
109-
v8::Local<v8::Value> receiver);
110-
static double FastGetMax(v8::Local<v8::Value> unused,
111-
v8::Local<v8::Value> receiver);
112-
static double FastGetMean(v8::Local<v8::Value> unused,
113-
v8::Local<v8::Value> receiver);
114-
static double FastGetExceeds(v8::Local<v8::Value> unused,
115-
v8::Local<v8::Value> receiver);
116-
static double FastGetStddev(v8::Local<v8::Value> unused,
117-
v8::Local<v8::Value> receiver);
118-
static double FastGetPercentile(v8::Local<v8::Value> unused,
119-
v8::Local<v8::Value> receiver,
104+
static void FastReset(v8::Local<v8::Value> receiver);
105+
static double FastGetCount(v8::Local<v8::Value> receiver);
106+
static double FastGetMin(v8::Local<v8::Value> receiver);
107+
static double FastGetMax(v8::Local<v8::Value> receiver);
108+
static double FastGetMean(v8::Local<v8::Value> receiver);
109+
static double FastGetExceeds(v8::Local<v8::Value> receiver);
110+
static double FastGetStddev(v8::Local<v8::Value> receiver);
111+
static double FastGetPercentile(v8::Local<v8::Value> receiver,
120112
const double percentile);
121113

122114
static void AddMethods(v8::Isolate* isolate,
@@ -165,13 +157,8 @@ class HistogramBase final : public BaseObject, public HistogramImpl {
165157
static void RecordDelta(const v8::FunctionCallbackInfo<v8::Value>& args);
166158
static void Add(const v8::FunctionCallbackInfo<v8::Value>& args);
167159

168-
static void FastRecord(
169-
v8::Local<v8::Value> unused,
170-
v8::Local<v8::Value> receiver,
171-
const int64_t value,
172-
v8::FastApiCallbackOptions& options); // NOLINT(runtime/references)
173-
static void FastRecordDelta(v8::Local<v8::Value> unused,
174-
v8::Local<v8::Value> receiver);
160+
static void FastRecord(v8::Local<v8::Value> receiver, const int64_t value);
161+
static void FastRecordDelta(v8::Local<v8::Value> receiver);
175162

176163
HistogramBase(
177164
Environment* env,
@@ -243,11 +230,8 @@ class IntervalHistogram final : public HandleWrap, public HistogramImpl {
243230
static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
244231
static void Stop(const v8::FunctionCallbackInfo<v8::Value>& args);
245232

246-
static void FastStart(v8::Local<v8::Value> unused,
247-
v8::Local<v8::Value> receiver,
248-
bool reset);
249-
static void FastStop(v8::Local<v8::Value> unused,
250-
v8::Local<v8::Value> receiver);
233+
static void FastStart(v8::Local<v8::Value> receiver, bool reset);
234+
static void FastStop(v8::Local<v8::Value> receiver);
251235

252236
BaseObject::TransferMode GetTransferMode() const override {
253237
return TransferMode::kCloneable;
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Flags: --expose-internals --no-warnings --allow-natives-syntax
2+
'use strict';
3+
4+
const common = require('../common');
5+
const assert = require('assert');
6+
7+
const { internalBinding } = require('internal/test/binding');
8+
9+
const histogram = require('perf_hooks').createHistogram();
10+
11+
function testFastMethods() {
12+
histogram.record(1);
13+
histogram.recordDelta();
14+
histogram.percentile(50);
15+
histogram.reset();
16+
}
17+
18+
eval('%PrepareFunctionForOptimization(histogram.record)');
19+
eval('%PrepareFunctionForOptimization(histogram.recordDelta)');
20+
eval('%PrepareFunctionForOptimization(histogram.percentile)');
21+
eval('%PrepareFunctionForOptimization(histogram.reset)');
22+
testFastMethods();
23+
eval('%OptimizeFunctionOnNextCall(histogram.record)');
24+
eval('%OptimizeFunctionOnNextCall(histogram.recordDelta)');
25+
eval('%OptimizeFunctionOnNextCall(histogram.percentile)');
26+
eval('%OptimizeFunctionOnNextCall(histogram.reset)');
27+
testFastMethods();
28+
29+
if (common.isDebug) {
30+
const { getV8FastApiCallCount } = internalBinding('debug');
31+
assert.strictEqual(getV8FastApiCallCount('histogram.record'), 1);
32+
assert.strictEqual(getV8FastApiCallCount('histogram.recordDelta'), 1);
33+
assert.strictEqual(getV8FastApiCallCount('histogram.percentile'), 1);
34+
assert.strictEqual(getV8FastApiCallCount('histogram.reset'), 1);
35+
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,10 @@ const { inspect } = require('util');
4141
code: 'ERR_INVALID_ARG_TYPE'
4242
});
4343
});
44-
throws(() => h.record(0, Number.MAX_SAFE_INTEGER + 1), {
45-
code: 'ERR_OUT_OF_RANGE'
44+
[0, Number.MAX_SAFE_INTEGER + 1].forEach((i) => {
45+
throws(() => h.record(i), {
46+
code: 'ERR_OUT_OF_RANGE'
47+
});
4648
});
4749

4850
strictEqual(h.min, 1);

0 commit comments

Comments
 (0)