Skip to content

Commit

Permalink
perf_hooks: add warning when too many entries in the timeline
Browse files Browse the repository at this point in the history
PR-URL: #18087
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
jasnell authored and MylesBorins committed Feb 21, 2018
1 parent 9ee15fa commit 75975c7
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 4 deletions.
14 changes: 14 additions & 0 deletions doc/api/perf_hooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 45 additions & 4 deletions lib/perf_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -250,20 +253,32 @@ 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]);
}

[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
}
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
}
Expand Down
29 changes: 29 additions & 0 deletions test/parallel/test-performance-warning.js
Original file line number Diff line number Diff line change
@@ -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');

0 comments on commit 75975c7

Please sign in to comment.