perf_hooks: make GC tracking state per-Environment

Otherwise this is global state that may be subject to race conditions e.g. when running `perf_hooks` inside of Worker threads. Tracking the GC type is removed entirely since the variable was unused. PR-URL: #25053 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Dec 17, 2018 · 8dfd757 · 8dfd757
1 parent cc0e177
commit 8dfd757
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 9 deletions.
diff --git a/src/node_perf.cc b/src/node_perf.cc
@@ -42,9 +42,6 @@ const double timeOriginTimestamp = GetCurrentTimeInMicroseconds();
 uint64_t performance_node_start;
 uint64_t performance_v8_start;
 
-uint64_t performance_last_gc_start_mark_ = 0;
-GCType performance_last_gc_type_ = GCType::kGCTypeAll;
-
 void performance_state::Mark(enum PerformanceMilestone milestone,
                              uint64_t ts) {
   this->milestones[milestone] = ts;
@@ -268,9 +265,10 @@ void PerformanceGCCallback(Environment* env, void* ptr) {
 // Marks the start of a GC cycle
 void MarkGarbageCollectionStart(Isolate* isolate,
                                 GCType type,
-                                GCCallbackFlags flags) {
-  performance_last_gc_start_mark_ = PERFORMANCE_NOW();
-  performance_last_gc_type_ = type;
+                                GCCallbackFlags flags,
+                                void* data) {
+  Environment* env = static_cast<Environment*>(data);
+  env->performance_state()->performance_last_gc_start_mark = PERFORMANCE_NOW();
 }
 
 // Marks the end of a GC cycle
@@ -279,21 +277,23 @@ void MarkGarbageCollectionEnd(Isolate* isolate,
                               GCCallbackFlags flags,
                               void* data) {
   Environment* env = static_cast<Environment*>(data);
+  performance_state* state = env->performance_state();
   // If no one is listening to gc performance entries, do not create them.
-  if (!env->performance_state()->observers[NODE_PERFORMANCE_ENTRY_TYPE_GC])
+  if (!state->observers[NODE_PERFORMANCE_ENTRY_TYPE_GC])
     return;
   GCPerformanceEntry* entry =
       new GCPerformanceEntry(env,
                              static_cast<PerformanceGCKind>(type),
-                             performance_last_gc_start_mark_,
+                             state->performance_last_gc_start_mark,
                              PERFORMANCE_NOW());
   env->SetUnrefImmediate(PerformanceGCCallback,
                          entry);
 }
 
 
 inline void SetupGarbageCollectionTracking(Environment* env) {
-  env->isolate()->AddGCPrologueCallback(MarkGarbageCollectionStart);
+  env->isolate()->AddGCPrologueCallback(MarkGarbageCollectionStart,
+                                        static_cast<void*>(env));
   env->isolate()->AddGCEpilogueCallback(MarkGarbageCollectionEnd,
                                         static_cast<void*>(env));
 }

diff --git a/src/node_perf_common.h b/src/node_perf_common.h
@@ -75,6 +75,8 @@ class performance_state {
   AliasedBuffer<double, v8::Float64Array> milestones;
   AliasedBuffer<uint32_t, v8::Uint32Array> observers;
 
+  uint64_t performance_last_gc_start_mark = 0;
+
   void Mark(enum PerformanceMilestone milestone,
             uint64_t ts = PERFORMANCE_NOW());