Skip to content

Commit e8da556

Browse files
addaleaxMylesBorins
authored andcommitted
src: keep track of env properly in node_perf.cc
Currently, measuring GC timing using `node_perf` is somewhat broken, because Isolates and Node Environments do not necessarily match 1:1; each environment adds its own hook, so possibly the hook code runs multiple times, but since it can’t reliably compute its corresponding event loop based on the Isolate, each run targets the same Environment right now. This fixes that problem by using new overloads of the GC tracking APIs that can pass data to the callback through opaque pointers. PR-URL: #15391 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent b4ad15b commit e8da556

File tree

2 files changed

+26
-15
lines changed

2 files changed

+26
-15
lines changed

src/node_perf.cc

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -170,13 +170,14 @@ void SetupPerformanceObservers(const FunctionCallbackInfo<Value>& args) {
170170
env->set_performance_entry_callback(args[0].As<Function>());
171171
}
172172

173-
inline void PerformanceGCCallback(uv_async_t* handle) {
173+
void PerformanceGCCallback(uv_async_t* handle) {
174174
PerformanceEntry::Data* data =
175175
static_cast<PerformanceEntry::Data*>(handle->data);
176-
Isolate* isolate = Isolate::GetCurrent();
176+
Environment* env = data->env();
177+
Isolate* isolate = env->isolate();
177178
HandleScope scope(isolate);
178-
Environment* env = Environment::GetCurrent(isolate);
179179
Local<Context> context = env->context();
180+
Context::Scope context_scope(context);
180181
Local<Function> fn;
181182
Local<Object> obj;
182183
PerformanceGCKind kind = static_cast<PerformanceGCKind>(data->data());
@@ -199,28 +200,31 @@ inline void PerformanceGCCallback(uv_async_t* handle) {
199200
uv_close(reinterpret_cast<uv_handle_t*>(handle), closeCB);
200201
}
201202

202-
inline void MarkGarbageCollectionStart(Isolate* isolate,
203-
v8::GCType type,
204-
v8::GCCallbackFlags flags) {
203+
void MarkGarbageCollectionStart(Isolate* isolate,
204+
v8::GCType type,
205+
v8::GCCallbackFlags flags) {
205206
performance_last_gc_start_mark_ = PERFORMANCE_NOW();
206207
performance_last_gc_type_ = type;
207208
}
208209

209-
inline void MarkGarbageCollectionEnd(Isolate* isolate,
210-
v8::GCType type,
211-
v8::GCCallbackFlags flags) {
210+
void MarkGarbageCollectionEnd(Isolate* isolate,
211+
v8::GCType type,
212+
v8::GCCallbackFlags flags,
213+
void* data) {
214+
Environment* env = static_cast<Environment*>(data);
212215
uv_async_t *async = new uv_async_t;
213216
async->data =
214-
new PerformanceEntry::Data("gc", "gc",
217+
new PerformanceEntry::Data(env, "gc", "gc",
215218
performance_last_gc_start_mark_,
216219
PERFORMANCE_NOW(), type);
217-
uv_async_init(uv_default_loop(), async, PerformanceGCCallback);
220+
uv_async_init(env->event_loop(), async, PerformanceGCCallback);
218221
uv_async_send(async);
219222
}
220223

221-
inline void SetupGarbageCollectionTracking(Isolate* isolate) {
222-
isolate->AddGCPrologueCallback(MarkGarbageCollectionStart);
223-
isolate->AddGCEpilogueCallback(MarkGarbageCollectionEnd);
224+
inline void SetupGarbageCollectionTracking(Environment* env) {
225+
env->isolate()->AddGCPrologueCallback(MarkGarbageCollectionStart);
226+
env->isolate()->AddGCEpilogueCallback(MarkGarbageCollectionEnd,
227+
static_cast<void*>(env));
224228
}
225229

226230
inline Local<Value> GetName(Local<Function> fn) {
@@ -376,7 +380,7 @@ void Init(Local<Object> target,
376380
constants,
377381
attr).ToChecked();
378382

379-
SetupGarbageCollectionTracking(isolate);
383+
SetupGarbageCollectionTracking(env);
380384
}
381385

382386
} // namespace performance

src/node_perf.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,23 @@ class PerformanceEntry : public BaseObject {
5353
class Data {
5454
public:
5555
Data(
56+
Environment* env,
5657
const char* name,
5758
const char* type,
5859
uint64_t startTime,
5960
uint64_t endTime,
6061
int data = 0) :
62+
env_(env),
6163
name_(name),
6264
type_(type),
6365
startTime_(startTime),
6466
endTime_(endTime),
6567
data_(data) {}
6668

69+
Environment* env() const {
70+
return env_;
71+
}
72+
6773
std::string name() const {
6874
return name_;
6975
}
@@ -85,6 +91,7 @@ class PerformanceEntry : public BaseObject {
8591
}
8692

8793
private:
94+
Environment* env_;
8895
std::string name_;
8996
std::string type_;
9097
uint64_t startTime_;

0 commit comments

Comments
 (0)