Skip to content

perf_hooks: simplify perf_hooks #19563

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jasnell wants to merge 2 commits into from
Closed

perf_hooks: simplify perf_hooks #19563

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f9ff4c
perf_hooks: simplify perf_hooks
jasnell Mar 23, 2018
72367b9
[Squash] fix issues
jasnell Apr 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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.
  • Loading branch information
@jasnell
jasnell committed Apr 2, 2018
commit 9f9ff4c5992d8d42d0d2d4a6c7e345466efe8fe0
106 changes: 8 additions & 98 deletions doc/api/perf_hooks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
});
```

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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'] });

Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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 });

Expand Down
99 changes: 7 additions & 92 deletions lib/perf_hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const {
PerformanceEntry,
mark: _mark,
clearMark: _clearMark,
measure: _measure,
milestones,
observerCounts,
Expand Down Expand Up @@ -50,14 +51,10 @@ 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;
Expand Down Expand Up @@ -286,17 +283,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];
Expand Down Expand Up @@ -407,72 +399,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() {
Expand Down Expand Up @@ -507,27 +438,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) {
Expand Down Expand Up @@ -563,7 +480,6 @@ class Performance extends PerformanceObserverEntryList {

[kInspect]() {
return {
maxEntries: this.maxEntries,
nodeTiming: this.nodeTiming,
timeOrigin: this.timeOrigin
};
Expand Down Expand Up @@ -595,7 +511,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);
Expand Down
12 changes: 12 additions & 0 deletions src/node_perf.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 1 addition & 9 deletions test/parallel/test-http2-perf_hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -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'] });

Expand Down Expand Up @@ -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);
});
Loading