Skip to content

Commit

Permalink
Revert "perf_hooks: make PerformanceObserver an AsyncResource"
Browse files Browse the repository at this point in the history
This reverts commit 009e418.

AFAIU the discussion at [1], PerformanceObserver had been made to
inherit from AsyncResource more or less as a band-aid in lack of a
better async_context candidate to invoke it in. In order to enable
access to AsyncLocalStores from PerformanceObservers invoked
synchronously through e.g. measure() or mark(), the current
async_context, if any, should be retained.

Note that this is a breaking change, but
- as has been commented at [1], PerformanceObserver being derived from
  AsyncResource is a "minor divergence from the spec" anyway,
- to my knowledge this is an internal implementation detail which has
  never been documented and
- I can't think of a good reason why existing PerformanceObserver
  implementations would possibly rely on it.

OTOH, it's probably worthwhile to not potentially invoke before() and
after() async_hooks for each and every PerformanceObserver notification.

[1] #18789

Co-Authored-By: ZauberNerd <zaubernerd@zaubernerd.de>

PR-URL: #36343
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
nicstange authored and targos committed Dec 21, 2020
1 parent e2ced0d commit f368d69
Showing 1 changed file with 2 additions and 7 deletions.
9 changes: 2 additions & 7 deletions lib/perf_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ const {
NODE_PERFORMANCE_MILESTONE_ENVIRONMENT
} = constants;

const { AsyncResource } = require('async_hooks');
const L = require('internal/linkedlist');
const kInspect = require('internal/util').customInspectSymbol;

Expand Down Expand Up @@ -340,12 +339,11 @@ class PerformanceObserverEntryList {
}
}

class PerformanceObserver extends AsyncResource {
class PerformanceObserver {
constructor(callback) {
if (typeof callback !== 'function') {
throw new ERR_INVALID_CALLBACK(callback);
}
super('PerformanceObserver');
ObjectDefineProperties(this, {
[kTypes]: {
enumerable: false,
Expand Down Expand Up @@ -553,10 +551,7 @@ function getObserversList(type) {

function doNotify(observer) {
observer[kQueued] = false;
observer.runInAsyncScope(observer[kCallback],
observer,
observer[kBuffer],
observer);
observer[kCallback](observer[kBuffer], observer);
observer[kBuffer][kEntries] = [];
L.init(observer[kBuffer][kEntries]);
}
Expand Down

0 comments on commit f368d69

Please sign in to comment.