Skip to content

Commit

Permalink
perf_hooks: remove GC callbacks on zero observers count
Browse files Browse the repository at this point in the history
When all existed PerformanceObserver instances removed for type `gc` GC
callbacks should be removed.

PR-URL: #29444
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Minwoo Jung <[email protected]>
  • Loading branch information
fanatid authored and targos committed Sep 20, 2019
1 parent a9d16b5 commit 9b4a49c
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 16 deletions.
20 changes: 12 additions & 8 deletions lib/perf_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ const {
timeOriginTimestamp,
timerify,
constants,
setupGarbageCollectionTracking
installGarbageCollectionTracking,
removeGarbageCollectionTracking
} = internalBinding('performance');

const {
Expand Down Expand Up @@ -281,8 +282,6 @@ class PerformanceObserverEntryList {
}
}

let gcTrackingIsEnabled = false;

class PerformanceObserver extends AsyncResource {
constructor(callback) {
if (typeof callback !== 'function') {
Expand Down Expand Up @@ -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++) {
Expand All @@ -329,6 +329,10 @@ class PerformanceObserver extends AsyncResource {
}
}
this[kTypes] = {};
if (observerCountsGC === 1 &&
observerCounts[NODE_PERFORMANCE_ENTRY_TYPE_GC] === 0) {
removeGarbageCollectionTracking();
}
}

observe(options) {
Expand All @@ -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);
Expand All @@ -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();
}
}
}

Expand Down
30 changes: 22 additions & 8 deletions src/node_perf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 9b4a49c

Please sign in to comment.