Skip to content

Commit

Permalink
events: optimize condition for optimal scenario
Browse files Browse the repository at this point in the history
Instead of always checking whether we've already warned about a
possible EventEmitter memory leak, first run the rest of the
code as accessing random properties on an Array is expensive.

In addition, remove an unnecessary truthy check.

PR-URL: #20452
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
  • Loading branch information
apapirovski authored and MylesBorins committed May 8, 2018
1 parent 74685f1 commit 58a65d6
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 17 deletions.
2 changes: 1 addition & 1 deletion benchmark/events/ee-add-remove.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const common = require('../common.js');
const events = require('events');

const bench = common.createBenchmark(main, { n: [25e4] });
const bench = common.createBenchmark(main, { n: [1e6] });

function main({ n }) {
const ee = new events.EventEmitter();
Expand Down
30 changes: 14 additions & 16 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -235,22 +235,20 @@ function _addListener(target, type, listener, prepend) {
}

// Check for listener leak
if (!existing.warned) {
m = $getMaxListeners(target);
if (m && m > 0 && existing.length > m) {
existing.warned = true;
// No error code for this since it is a Warning
// eslint-disable-next-line no-restricted-syntax
const w = new Error('Possible EventEmitter memory leak detected. ' +
`${existing.length} ${String(type)} listeners ` +
'added. Use emitter.setMaxListeners() to ' +
'increase limit');
w.name = 'MaxListenersExceededWarning';
w.emitter = target;
w.type = type;
w.count = existing.length;
process.emitWarning(w);
}
m = $getMaxListeners(target);
if (m > 0 && existing.length > m && !existing.warned) {
existing.warned = true;
// No error code for this since it is a Warning
// eslint-disable-next-line no-restricted-syntax
const w = new Error('Possible EventEmitter memory leak detected. ' +
`${existing.length} ${String(type)} listeners ` +
'added. Use emitter.setMaxListeners() to ' +
'increase limit');
w.name = 'MaxListenersExceededWarning';
w.emitter = target;
w.type = type;
w.count = existing.length;
process.emitWarning(w);
}
}

Expand Down

0 comments on commit 58a65d6

Please sign in to comment.