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 <[email protected]> Reviewed-By: Michaël Zasso <[email protected]>
nodejs · Oct 3, 2017 · e8da556 · e8da556
1 parent b4ad15b
commit e8da556
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 15 deletions.
diff --git a/src/node_perf.cc b/src/node_perf.cc
@@ -170,13 +170,14 @@ void SetupPerformanceObservers(const FunctionCallbackInfo<Value>& args) {
   env->set_performance_entry_callback(args[0].As<Function>());
 }
 
-inline void PerformanceGCCallback(uv_async_t* handle) {
+void PerformanceGCCallback(uv_async_t* handle) {
   PerformanceEntry::Data* data =
       static_cast<PerformanceEntry::Data*>(handle->data);
-  Isolate* isolate = Isolate::GetCurrent();
+  Environment* env = data->env();
+  Isolate* isolate = env->isolate();
   HandleScope scope(isolate);
-  Environment* env = Environment::GetCurrent(isolate);
   Local<Context> context = env->context();
+  Context::Scope context_scope(context);
   Local<Function> fn;
   Local<Object> obj;
   PerformanceGCKind kind = static_cast<PerformanceGCKind>(data->data());
@@ -199,28 +200,31 @@ inline void PerformanceGCCallback(uv_async_t* handle) {
   uv_close(reinterpret_cast<uv_handle_t*>(handle), closeCB);
 }
 
-inline void MarkGarbageCollectionStart(Isolate* isolate,
-                                       v8::GCType type,
-                                       v8::GCCallbackFlags flags) {
+void MarkGarbageCollectionStart(Isolate* isolate,
+                                v8::GCType type,
+                                v8::GCCallbackFlags flags) {
   performance_last_gc_start_mark_ = PERFORMANCE_NOW();
   performance_last_gc_type_ = type;
 }
 
-inline void MarkGarbageCollectionEnd(Isolate* isolate,
-                                     v8::GCType type,
-                                     v8::GCCallbackFlags flags) {
+void MarkGarbageCollectionEnd(Isolate* isolate,
+                              v8::GCType type,
+                              v8::GCCallbackFlags flags,
+                              void* data) {
+  Environment* env = static_cast<Environment*>(data);
   uv_async_t *async = new uv_async_t;
   async->data =
-      new PerformanceEntry::Data("gc", "gc",
+      new PerformanceEntry::Data(env, "gc", "gc",
                                  performance_last_gc_start_mark_,
                                  PERFORMANCE_NOW(), type);
-  uv_async_init(uv_default_loop(), async, PerformanceGCCallback);
+  uv_async_init(env->event_loop(), async, PerformanceGCCallback);
   uv_async_send(async);
 }
 
-inline void SetupGarbageCollectionTracking(Isolate* isolate) {
-  isolate->AddGCPrologueCallback(MarkGarbageCollectionStart);
-  isolate->AddGCEpilogueCallback(MarkGarbageCollectionEnd);
+inline void SetupGarbageCollectionTracking(Environment* env) {
+  env->isolate()->AddGCPrologueCallback(MarkGarbageCollectionStart);
+  env->isolate()->AddGCEpilogueCallback(MarkGarbageCollectionEnd,
+                                        static_cast<void*>(env));
 }
 
 inline Local<Value> GetName(Local<Function> fn) {
@@ -376,7 +380,7 @@ void Init(Local<Object> target,
                             constants,
                             attr).ToChecked();
 
-  SetupGarbageCollectionTracking(isolate);
+  SetupGarbageCollectionTracking(env);
 }
 
 }  // namespace performance

diff --git a/src/node_perf.h b/src/node_perf.h
@@ -53,17 +53,23 @@ class PerformanceEntry : public BaseObject {
   class Data {
    public:
     Data(
+      Environment* env,
       const char* name,
       const char* type,
       uint64_t startTime,
       uint64_t endTime,
       int data = 0) :
+      env_(env),
       name_(name),
       type_(type),
       startTime_(startTime),
       endTime_(endTime),
       data_(data) {}
 
+    Environment* env() const {
+      return env_;
+    }
+
     std::string name() const {
       return name_;
     }
@@ -85,6 +91,7 @@ class PerformanceEntry : public BaseObject {
     }
 
    private:
+    Environment* env_;
     std::string name_;
     std::string type_;
     uint64_t startTime_;