From 0a47b98f041a9a570f2cd29bb002e20f68d17797 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Wed, 10 Jan 2018 09:42:10 -0800
Subject: [PATCH] perf_hooks: add warning when too many entries in the timeline

PR-URL: https://github.com/nodejs/node/pull/18087
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 doc/api/perf_hooks.md                     | 14 +++++++
 lib/perf_hooks.js                         | 49 +++++++++++++++++++++--
 test/parallel/test-performance-warning.js | 29 ++++++++++++++
 3 files changed, 88 insertions(+), 4 deletions(-)
 create mode 100644 test/parallel/test-performance-warning.js

diff --git a/doc/api/perf_hooks.md b/doc/api/perf_hooks.md
index 6d98d25af37cbc..608bee8d574680 100644
--- a/doc/api/perf_hooks.md
+++ b/doc/api/perf_hooks.md
@@ -125,6 +125,20 @@ 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: REPLACEME
+-->
+
+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
diff --git a/lib/perf_hooks.js b/lib/perf_hooks.js
index 15256a63c0b97c..6fd6e4a6b768ce 100644
--- a/lib/perf_hooks.js
+++ b/lib/perf_hooks.js
@@ -53,6 +53,9 @@ 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;
@@ -250,10 +253,17 @@ const nodeTiming = new PerformanceNodeTiming();
 // Maintains a list of entries as a linked list stored in insertion order.
 class PerformanceObserverEntryList {
   constructor() {
-    Object.defineProperty(this, kEntries, {
-      writable: true,
-      enumerable: false,
-      value: {}
+    Object.defineProperties(this, {
+      [kEntries]: {
+        writable: true,
+        enumerable: false,
+        value: {}
+      },
+      [kCount]: {
+        writable: true,
+        enumerable: false,
+        value: 0
+      }
     });
     L.init(this[kEntries]);
   }
@@ -261,9 +271,14 @@ class PerformanceObserverEntryList {
   [kInsertEntry](entry) {
     const item = { entry };
     L.append(this[kEntries], item);
+    this[kCount]++;
     this[kIndexEntry](item);
   }
 
+  get length() {
+    return this[kCount];
+  }
+
   [kIndexEntry](entry) {
     // Default implementation does nothing
   }
@@ -384,9 +399,22 @@ class Performance extends PerformanceObserverEntryList {
     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.TypeError('ERR_INVALID_ARG_TYPE', 'val', 'number');
+    }
+    this[kMaxCount] = Math.max(1, val >>> 0);
+  }
+
+  get maxEntries() {
+    return this[kMaxCount];
+  }
+
   [kIndexEntry](item) {
     const index = this[kIndex];
     const type = item.entry.entryType;
@@ -397,6 +425,17 @@ class Performance extends PerformanceObserverEntryList {
     }
     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) {
@@ -411,10 +450,12 @@ class Performance extends PerformanceObserverEntryList {
         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;
     }
diff --git a/test/parallel/test-performance-warning.js b/test/parallel/test-performance-warning.js
new file mode 100644
index 00000000000000..f3104677a7c649
--- /dev/null
+++ b/test/parallel/test-performance-warning.js
@@ -0,0 +1,29 @@
+// Flags: --no-warnings
+'use strict';
+
+const common = require('../common');
+const { performance } = require('perf_hooks');
+const assert = require('assert');
+
+assert.strictEqual(performance.length, 1);
+assert.strictEqual(performance.maxEntries, 150);
+
+performance.maxEntries = 1;
+
+[-1, 0xffffffff + 1, '', null, undefined, Infinity].forEach((i) => {
+  common.expectsError(
+    () => performance.maxEntries = i,
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      type: TypeError
+    }
+  );
+});
+
+common.expectWarning('Warning', [
+  'Possible perf_hooks memory leak detected. There are 2 entries 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 1).']);
+
+performance.mark('test');