perf_hooks: simplify perf_hooks

Remove the `performance.getEntries()` and `performance.clear*()`
variants and eliminate the accumulation of the global timeline
entries. The design of this particular bit of the API is a memory
leak and performance footgun. The `PerformanceObserver` API is
a better approach to consuming the data in a more transient way.

PR-URL: #19563
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: jasnell

doc/api/perf_hooks.md

@@ -10,14 +10,18 @@ is to support collection of high resolution performance metrics.
 This is the same Performance API as implemented in modern Web browsers.
 
 ```js
-const { performance } = require('perf_hooks');
+const { PerformanceObserver, performance } = require('perf_hooks');
+
+const obs = new PerformanceObserver((items) => {
+  console.log(items.getEntries()[0].duration);
+  performance.clearMarks();
+});
+obs.observe({ entryTypes: ['measure'] });
+
 performance.mark('A');
 doSomeLongRunningProcess(() => {
   performance.mark('B');
   performance.measure('A to B', 'A', 'B');
-  const measure = performance.getEntriesByName('A to B')[0];
-  console.log(measure.duration);
-  // Prints the number of milliseconds between Mark 'A' and Mark 'B'
 });
 ```
 
@@ -26,35 +30,6 @@ doSomeLongRunningProcess(() => {
 added: v8.5.0
 -->
 
-The `Performance` provides access to performance metric data. A single
-instance of this class is provided via the `performance` property.
-
-### performance.clearEntries(name)
-<!-- YAML
-added: v9.5.0
--->
-
-Remove all performance entry objects with `entryType` equal to `name` from the
-Performance Timeline.
-
-### performance.clearFunctions([name])
-<!-- YAML
-added: v8.5.0
--->
-
-* `name` {string}
-
-If `name` is not provided, removes all `PerformanceFunction` objects from the
-Performance Timeline. If `name` is provided, removes entries with `name`.
-
-### performance.clearGC()
-<!-- YAML
-added: v8.5.0
--->
-
-Remove all performance entry objects with `entryType` equal to `gc` from the
-Performance Timeline.
-
 ### performance.clearMarks([name])
 <!-- YAML
 added: v8.5.0
@@ -65,53 +40,6 @@ added: v8.5.0
 If `name` is not provided, removes all `PerformanceMark` objects from the
 Performance Timeline. If `name` is provided, removes only the named mark.
 
-### performance.clearMeasures([name])
-<!-- YAML
-added: v8.5.0
--->
-
-* `name` {string}
-
-If `name` is not provided, removes all `PerformanceMeasure` objects from the
-Performance Timeline. If `name` is provided, removes only objects whose
-`performanceEntry.name` matches `name`.
-
-### performance.getEntries()
-<!-- YAML
-added: v8.5.0
--->
-
-* Returns: {Array}
-
-Returns a list of all `PerformanceEntry` objects in chronological order
-with respect to `performanceEntry.startTime`.
-
-### performance.getEntriesByName(name[, type])
-<!-- YAML
-added: v8.5.0
--->
-
-* `name` {string}
-* `type` {string}
-* Returns: {Array}
-
-Returns a list of all `PerformanceEntry` objects in chronological order
-with respect to `performanceEntry.startTime` whose `performanceEntry.name` is
-equal to `name`, and optionally, whose `performanceEntry.entryType` is equal to
-`type`.
-
-### performance.getEntriesByType(type)
-<!-- YAML
-added: v8.5.0
--->
-
-* `type` {string}
-* Returns: {Array}
-
-Returns a list of all `PerformanceEntry` objects in chronological order
-with respect to `performanceEntry.startTime` whose `performanceEntry.entryType`
-is equal to `type`.
-
 ### performance.mark([name])
 <!-- YAML
 added: v8.5.0
@@ -125,20 +53,6 @@ Creates a new `PerformanceMark` entry in the Performance Timeline. A
 `performanceEntry.duration` is always `0`. Performance marks are used
 to mark specific significant moments in the Performance Timeline.
 
-### performance.maxEntries
-<!-- YAML
-added: v9.6.0
--->
-
-Value: {number}
-
-The maximum number of Performance Entry items that should be added to the
-Performance Timeline. This limit is not strictly enforced, but a process
-warning will be emitted if the number of entries in the timeline exceeds
-this limit.
-
-Defaults to 150.
-
 ### performance.measure(name, startMark, endMark)
 <!-- YAML
 added: v8.5.0
@@ -220,7 +134,6 @@ const wrapped = performance.timerify(someFunction);
 const obs = new PerformanceObserver((list) => {
   console.log(list.getEntries()[0].duration);
   obs.disconnect();
-  performance.clearFunctions();
 });
 obs.observe({ entryTypes: ['function'] });
 
@@ -608,7 +521,6 @@ hook.enable();
 const obs = new PerformanceObserver((list, observer) => {
   console.log(list.getEntries()[0]);
   performance.clearMarks();
-  performance.clearMeasures();
   observer.disconnect();
 });
 obs.observe({ entryTypes: ['measure'], buffered: true });
@@ -642,8 +554,6 @@ const obs = new PerformanceObserver((list) => {
     console.log(`require('${entry[0]}')`, entry.duration);
   });
   obs.disconnect();
-  // Free memory
-  performance.clearFunctions();
 });
 obs.observe({ entryTypes: ['function'], buffered: true });
 

lib/perf_hooks.js

@@ -3,6 +3,7 @@
 const {
   PerformanceEntry,
   mark: _mark,
+  clearMark: _clearMark,
   measure: _measure,
   milestones,
   observerCounts,
@@ -50,17 +51,11 @@ const kBuffering = Symbol('buffering');
 const kQueued = Symbol('queued');
 const kTimerified = Symbol('timerified');
 const kInsertEntry = Symbol('insert-entry');
-const kIndexEntry = Symbol('index-entry');
-const kClearEntry = Symbol('clear-entry');
 const kGetEntries = Symbol('get-entries');
 const kIndex = Symbol('index');
 const kMarks = Symbol('marks');
 const kCount = Symbol('count');
-const kMaxCount = Symbol('max-count');
-const kDefaultMaxCount = 150;
 
-observerCounts[NODE_PERFORMANCE_ENTRY_TYPE_MARK] = 1;
-observerCounts[NODE_PERFORMANCE_ENTRY_TYPE_MEASURE] = 1;
 const observers = {};
 const observerableTypes = [
   'node',
@@ -286,17 +281,12 @@ class PerformanceObserverEntryList {
     const item = { entry };
     L.append(this[kEntries], item);
     this[kCount]++;
-    this[kIndexEntry](item);
   }
 
   get length() {
     return this[kCount];
   }
 
-  [kIndexEntry](entry) {
-    // Default implementation does nothing
-  }
-
   [kGetEntries](name, type) {
     const ret = [];
     const list = this[kEntries];
@@ -407,72 +397,11 @@ class PerformanceObserver extends AsyncResource {
   }
 }
 
-class Performance extends PerformanceObserverEntryList {
+class Performance {
   constructor() {
-    super();
     this[kIndex] = {
       [kMarks]: new Set()
     };
-    this[kMaxCount] = kDefaultMaxCount;
-    this[kInsertEntry](nodeTiming);
-  }
-
-  set maxEntries(val) {
-    if (typeof val !== 'number' || val >>> 0 !== val) {
-      const errors = lazyErrors();
-      throw new errors.ERR_INVALID_ARG_TYPE('val', 'number', val);
-    }
-    this[kMaxCount] = Math.max(1, val >>> 0);
-  }
-
-  get maxEntries() {
-    return this[kMaxCount];
-  }
-
-  [kIndexEntry](item) {
-    const index = this[kIndex];
-    const type = item.entry.entryType;
-    let items = index[type];
-    if (!items) {
-      items = index[type] = {};
-      L.init(items);
-    }
-    const entry = item.entry;
-    L.append(items, { entry, item });
-    const count = this[kCount];
-    if (count > this[kMaxCount]) {
-      const text = count === 1 ? 'is 1 entry' : `are ${count} entries`;
-      process.emitWarning('Possible perf_hooks memory leak detected. ' +
-                          `There ${text} in the ` +
-                          'Performance Timeline. Use the clear methods ' +
-                          'to remove entries that are no longer needed or ' +
-                          'set performance.maxEntries equal to a higher ' +
-                          'value (currently the maxEntries is ' +
-                          `${this[kMaxCount]}).`);
-    }
-  }
-
-  [kClearEntry](type, name) {
-    const index = this[kIndex];
-    const items = index[type];
-    if (!items) return;
-    let item = L.peek(items);
-    while (item && item !== items) {
-      const entry = item.entry;
-      const next = item._idlePrev;
-      if (name !== undefined) {
-        if (entry.name === `${name}`) {
-          L.remove(item); // remove from the index
-          L.remove(item.item); // remove from the master
-          this[kCount]--;
-        }
-      } else {
-        L.remove(item); // remove from the index
-        L.remove(item.item); // remove from the master
-        this[kCount]--;
-      }
-      item = next;
-    }
   }
 
   get nodeTiming() {
@@ -507,27 +436,13 @@ class Performance extends PerformanceObserverEntryList {
 
   clearMarks(name) {
     name = name !== undefined ? `${name}` : name;
-    this[kClearEntry]('mark', name);
-    if (name !== undefined)
+    if (name !== undefined) {
       this[kIndex][kMarks].delete(name);
-    else
+      _clearMark(name);
+    } else {
       this[kIndex][kMarks].clear();
-  }
-
-  clearMeasures(name) {
-    this[kClearEntry]('measure', name);
-  }
-
-  clearGC() {
-    this[kClearEntry]('gc');
-  }
-
-  clearFunctions(name) {
-    this[kClearEntry]('function', name);
-  }
-
-  clearEntries(name) {
-    this[kClearEntry](name);
+      _clearMark();
+    }
   }
 
   timerify(fn) {
@@ -563,7 +478,6 @@ class Performance extends PerformanceObserverEntryList {
 
   [kInspect]() {
     return {
-      maxEntries: this.maxEntries,
       nodeTiming: this.nodeTiming,
       timeOrigin: this.timeOrigin
     };
@@ -595,7 +509,6 @@ function observersCallback(entry) {
   if (type === NODE_PERFORMANCE_ENTRY_TYPE_HTTP2)
     collectHttp2Stats(entry);
 
-  performance[kInsertEntry](entry);
   const list = getObserversList(type);
 
   let current = L.peek(list);

src/node_perf.cc

@@ -130,7 +130,7 @@ void PerformanceEntry::Notify(Environment* env,
                        object.As<Object>(),
                        env->performance_entry_callback(),
                        1, &object,
-                       node::async_context{0, 0}).ToLocalChecked();
+                       node::async_context{0, 0});
   }
 }
 
@@ -153,6 +153,17 @@ void Mark(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(obj);
 }
 
+void ClearMark(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  auto marks = env->performance_marks();
+
+  if (args.Length() == 0) {
+    marks->clear();
+  } else {
+    Utf8Value name(env->isolate(), args[0]);
+    marks->erase(*name);
+  }
+}
 
 inline uint64_t GetPerformanceMark(Environment* env, std::string name) {
   auto marks = env->performance_marks();
@@ -395,6 +406,7 @@ void Initialize(Local<Object> target,
   target->Set(context, performanceEntryString, fn).FromJust();
   env->set_performance_entry_template(fn);
 
+  env->SetMethod(target, "clearMark", ClearMark);
   env->SetMethod(target, "mark", Mark);
   env->SetMethod(target, "measure", Measure);
   env->SetMethod(target, "markMilestone", MarkMilestone);

test/parallel/test-http2-perf_hooks.js

@@ -6,7 +6,7 @@ if (!common.hasCrypto)
 const assert = require('assert');
 const h2 = require('http2');
 
-const { PerformanceObserver, performance } = require('perf_hooks');
+const { PerformanceObserver } = require('perf_hooks');
 
 const obs = new PerformanceObserver(common.mustCall((items) => {
   const entry = items.getEntries()[0];
@@ -46,7 +46,6 @@ const obs = new PerformanceObserver(common.mustCall((items) => {
     default:
       assert.fail('invalid entry name');
   }
-  performance.clearEntries('http2');
 }, 4));
 obs.observe({ entryTypes: ['http2'] });
 
@@ -101,10 +100,3 @@ server.on('listening', common.mustCall(() => {
   }));
 
 }));
-
-process.on('exit', () => {
-  const entries = performance.getEntries();
-  // There shouldn't be any http2 entries left over.
-  assert.strictEqual(entries.length, 1);
-  assert.strictEqual(entries[0], performance.nodeTiming);
-});