Skip to content

Commit 2197425

Browse files
Renegade334marco-ippolito
authored andcommitted
perf_hooks: fix histogram fast call signatures
PR-URL: #59600 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
1 parent d7a976c commit 2197425

4 files changed

Lines changed: 77 additions & 56 deletions

File tree

src/histogram.cc

Lines changed: 26 additions & 26 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,7 +12,6 @@ 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;
1717
using v8::Integer;
@@ -161,8 +161,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo<Value>& args) {
161161
(*histogram)->RecordDelta();
162162
}
163163

164-
void HistogramBase::FastRecordDelta(Local<Value> unused,
165-
Local<Value> receiver) {
164+
void HistogramBase::FastRecordDelta(Local<Value> receiver) {
165+
TRACK_V8_FAST_API_CALL("histogram.recordDelta");
166166
HistogramBase* histogram;
167167
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
168168
(*histogram)->RecordDelta();
@@ -182,14 +182,9 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
182182
(*histogram)->Record(value);
183183
}
184184

185-
void HistogramBase::FastRecord(Local<Value> unused,
186-
Local<Value> receiver,
187-
const int64_t value,
188-
FastApiCallbackOptions& options) {
189-
if (value < 1) {
190-
options.fallback = true;
191-
return;
192-
}
185+
void HistogramBase::FastRecord(Local<Value> receiver, const int64_t value) {
186+
CHECK_GE(value, 1);
187+
TRACK_V8_FAST_API_CALL("histogram.record");
193188
HistogramBase* histogram;
194189
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
195190
(*histogram)->Record(value);
@@ -426,9 +421,8 @@ void IntervalHistogram::Start(const FunctionCallbackInfo<Value>& args) {
426421
histogram->OnStart(args[0]->IsTrue() ? StartFlags::RESET : StartFlags::NONE);
427422
}
428423

429-
void IntervalHistogram::FastStart(Local<Value> unused,
430-
Local<Value> receiver,
431-
bool reset) {
424+
void IntervalHistogram::FastStart(Local<Value> receiver, bool reset) {
425+
TRACK_V8_FAST_API_CALL("histogram.start");
432426
IntervalHistogram* histogram;
433427
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
434428
histogram->OnStart(reset ? StartFlags::RESET : StartFlags::NONE);
@@ -440,7 +434,8 @@ void IntervalHistogram::Stop(const FunctionCallbackInfo<Value>& args) {
440434
histogram->OnStop();
441435
}
442436

443-
void IntervalHistogram::FastStop(Local<Value> unused, Local<Value> receiver) {
437+
void IntervalHistogram::FastStop(Local<Value> receiver) {
438+
TRACK_V8_FAST_API_CALL("histogram.stop");
444439
IntervalHistogram* histogram;
445440
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
446441
histogram->OnStop();
@@ -553,46 +548,51 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo<Value>& args) {
553548
(*histogram)->Reset();
554549
}
555550

556-
void HistogramImpl::FastReset(Local<Value> unused, Local<Value> receiver) {
551+
void HistogramImpl::FastReset(Local<Value> receiver) {
552+
TRACK_V8_FAST_API_CALL("histogram.reset");
557553
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
558554
(*histogram)->Reset();
559555
}
560556

561-
double HistogramImpl::FastGetCount(Local<Value> unused, Local<Value> receiver) {
557+
double HistogramImpl::FastGetCount(Local<Value> receiver) {
558+
TRACK_V8_FAST_API_CALL("histogram.count");
562559
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
563560
return static_cast<double>((*histogram)->Count());
564561
}
565562

566-
double HistogramImpl::FastGetMin(Local<Value> unused, Local<Value> receiver) {
563+
double HistogramImpl::FastGetMin(Local<Value> receiver) {
564+
TRACK_V8_FAST_API_CALL("histogram.min");
567565
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
568566
return static_cast<double>((*histogram)->Min());
569567
}
570568

571-
double HistogramImpl::FastGetMax(Local<Value> unused, Local<Value> receiver) {
569+
double HistogramImpl::FastGetMax(Local<Value> receiver) {
570+
TRACK_V8_FAST_API_CALL("histogram.max");
572571
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
573572
return static_cast<double>((*histogram)->Max());
574573
}
575574

576-
double HistogramImpl::FastGetMean(Local<Value> unused, Local<Value> receiver) {
575+
double HistogramImpl::FastGetMean(Local<Value> receiver) {
576+
TRACK_V8_FAST_API_CALL("histogram.mean");
577577
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
578578
return (*histogram)->Mean();
579579
}
580580

581-
double HistogramImpl::FastGetExceeds(Local<Value> unused,
582-
Local<Value> receiver) {
581+
double HistogramImpl::FastGetExceeds(Local<Value> receiver) {
582+
TRACK_V8_FAST_API_CALL("histogram.exceeds");
583583
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
584584
return static_cast<double>((*histogram)->Exceeds());
585585
}
586586

587-
double HistogramImpl::FastGetStddev(Local<Value> unused,
588-
Local<Value> receiver) {
587+
double HistogramImpl::FastGetStddev(Local<Value> receiver) {
588+
TRACK_V8_FAST_API_CALL("histogram.stddev");
589589
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
590590
return (*histogram)->Stddev();
591591
}
592592

593-
double HistogramImpl::FastGetPercentile(Local<Value> unused,
594-
Local<Value> receiver,
593+
double HistogramImpl::FastGetPercentile(Local<Value> receiver,
595594
const double percentile) {
595+
TRACK_V8_FAST_API_CALL("histogram.percentile");
596596
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
597597
return static_cast<double>((*histogram)->Percentile(percentile));
598598
}

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)