perf_hooks: remove GC callbacks on zero observers count

When all existed PerformanceObserver instances removed for type `gc` GC callbacks should be removed. PR-URL: #29444 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
nodejs · Sep 20, 2019 · 9b4a49c · 9b4a49c
1 parent a9d16b5
commit 9b4a49c
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 16 deletions.
diff --git a/lib/perf_hooks.js b/lib/perf_hooks.js
@@ -15,7 +15,8 @@ const {
  timeOriginTimestamp,
  timerify,
  constants,
- setupGarbageCollectionTracking
+ installGarbageCollectionTracking,
+ removeGarbageCollectionTracking
 } = internalBinding('performance');
 
 const {
@@ -281,8 +282,6 @@ class PerformanceObserverEntryList {
  }
 }
 
-let gcTrackingIsEnabled = false;
-
 class PerformanceObserver extends AsyncResource {
  constructor(callback) {
  if (typeof callback !== 'function') {
@@ -319,6 +318,7 @@ class PerformanceObserver extends AsyncResource {
  }
 
  disconnect() {
+ const observerCountsGC = observerCounts[NODE_PERFORMANCE_ENTRY_TYPE_GC];
  const types = this[kTypes];
  const keys = Object.keys(types);
  for (var n = 0; n < keys.length; n++) {
@@ -329,6 +329,10 @@ class PerformanceObserver extends AsyncResource {
  }
  }
  this[kTypes] = {};
+ if (observerCountsGC === 1 &&
+ observerCounts[NODE_PERFORMANCE_ENTRY_TYPE_GC] === 0) {
+ removeGarbageCollectionTracking();
+ }
  }
 
  observe(options) {
@@ -342,12 +346,8 @@ class PerformanceObserver extends AsyncResource {
  if (entryTypes.length === 0) {
  throw new ERR_VALID_PERFORMANCE_ENTRY_TYPE();
  }
- if (entryTypes.includes(NODE_PERFORMANCE_ENTRY_TYPE_GC) &&
- !gcTrackingIsEnabled) {
- setupGarbageCollectionTracking();
- gcTrackingIsEnabled = true;
- }
  this.disconnect();
+ const observerCountsGC = observerCounts[NODE_PERFORMANCE_ENTRY_TYPE_GC];
  this[kBuffer][kEntries] = [];
  L.init(this[kBuffer][kEntries]);
  this[kBuffering] = Boolean(options.buffered);
@@ -359,6 +359,10 @@ class PerformanceObserver extends AsyncResource {
  L.append(list, item);
  observerCounts[entryType]++;
  }
+ if (observerCountsGC === 0 &&
+ observerCounts[NODE_PERFORMANCE_ENTRY_TYPE_GC] === 1) {
+ installGarbageCollectionTracking();
+ }
  }
 }
 

diff --git a/src/node_perf.cc b/src/node_perf.cc
@@ -277,19 +277,29 @@ void MarkGarbageCollectionEnd(Isolate* isolate,
  });
 }
 
-static void SetupGarbageCollectionTracking(
+void GarbageCollectionCleanupHook(void* data) {
+ Environment* env = static_cast<Environment*>(data);
+ env->isolate()->RemoveGCPrologueCallback(MarkGarbageCollectionStart, data);
+ env->isolate()->RemoveGCEpilogueCallback(MarkGarbageCollectionEnd, data);
+}
+
+static void InstallGarbageCollectionTracking(
  const FunctionCallbackInfo<Value>& args) {
  Environment* env = Environment::GetCurrent(args);
 
  env->isolate()->AddGCPrologueCallback(MarkGarbageCollectionStart,
  static_cast<void*>(env));
  env->isolate()->AddGCEpilogueCallback(MarkGarbageCollectionEnd,
  static_cast<void*>(env));
- env->AddCleanupHook([](void* data) {
- Environment* env = static_cast<Environment*>(data);
- env->isolate()->RemoveGCPrologueCallback(MarkGarbageCollectionStart, data);
- env->isolate()->RemoveGCEpilogueCallback(MarkGarbageCollectionEnd, data);
- }, env);
+ env->AddCleanupHook(GarbageCollectionCleanupHook, env);
+}
+
+static void RemoveGarbageCollectionTracking(
+ const FunctionCallbackInfo<Value> &args) {
+ Environment* env = Environment::GetCurrent(args);
+
+ env->RemoveCleanupHook(GarbageCollectionCleanupHook, env);
+ GarbageCollectionCleanupHook(env);
 }
 
 // Gets the name of a function
@@ -575,8 +585,12 @@ void Initialize(Local<Object> target,
  env->SetMethod(target, "markMilestone", MarkMilestone);
  env->SetMethod(target, "setupObservers", SetupPerformanceObservers);
  env->SetMethod(target, "timerify", Timerify);
- env->SetMethod(
- target, "setupGarbageCollectionTracking", SetupGarbageCollectionTracking);
+ env->SetMethod(target,
+ "installGarbageCollectionTracking",
+ InstallGarbageCollectionTracking);
+ env->SetMethod(target,
+ "removeGarbageCollectionTracking",
+ RemoveGarbageCollectionTracking);
  env->SetMethod(target, "notify", Notify);
 
  Local<Object> constants = Object::New(isolate);