Skip to content

events: improve events performance #50422

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
rluvaton wants to merge 3 commits into from
Closed

events: improve events performance #50422

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb29e97
events: improve creation performance
rluvaton Oct 27, 2023
249c2b8
events: replace _events with Map
rluvaton Oct 27, 2023
9d626b9
events: remove `__proto__` null from `_events`
rluvaton Oct 27, 2023
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
Prev Previous commit
events: remove __proto__ null from _events
  • Loading branch information
@rluvaton
rluvaton committed Oct 27, 2023
commit 9d626b91ee52db1ac97bb6d9dffc54657eb2873e
122 changes: 47 additions & 75 deletions lib/events.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,13 @@ const {
PromiseReject,
PromiseResolve,
ReflectApply,
ReflectOwnKeys,
String,
ObjectFromEntries,
ArrayFrom,
StringPrototypeSplit,
Symbol,
SymbolFor,
SymbolAsyncIterator,
SymbolDispose,
SafeMap,
} = primordials;
const kRejection = SymbolFor('nodejs.rejection');

Expand Down Expand Up @@ -87,7 +85,6 @@ const {
validateString,
} = require('internal/validators');

const kEvents = Symbol('kEvents');
const kCapture = Symbol('kCapture');
const kErrorMonitor = Symbol('events.errorMonitor');
const kMaxEventTargetListeners = Symbol('events.maxEventTargetListeners');
Expand Down Expand Up @@ -265,7 +262,8 @@ ObjectDefineProperty(EventEmitter.prototype, kCapture, {
enumerable: false,
});

EventEmitter.prototype[kEvents] = undefined;
EventEmitter.prototype._events = undefined;
EventEmitter.prototype._eventsCount = 0;
EventEmitter.prototype._maxListeners = undefined;

// By default EventEmitters will print a warning if more than 10 listeners are
Expand All @@ -289,37 +287,6 @@ ObjectDefineProperty(EventEmitter, 'defaultMaxListeners', {
},
});

// _events and _eventsCount are Legacy
ObjectDefineProperty(EventEmitter.prototype, '_events', {
__proto__: null,
enumerable: true,
get: function() {
// TODO - maybe in order to not break the ecosystem, create a Proxy that will update the events map
const events = this[kEvents];
if (events === undefined) {
return undefined;
}

return ObjectFromEntries(events.entries());
},
set: function(events) {
this[kEvents] = new SafeMap(events);
},
});

ObjectDefineProperty(EventEmitter.prototype, '_eventsCount', {
__proto__: null,
enumerable: true,
get: function() {
const events = this[kEvents];
if (events === undefined) {
return 0;
}

return events.size;
},
});

ObjectDefineProperties(EventEmitter, {
kMaxEventTargetListeners: {
__proto__: null,
Expand Down Expand Up @@ -372,9 +339,11 @@ EventEmitter.setMaxListeners =
// If you're updating this function definition, please also update any
// re-definitions, such as the one in the Domain module (lib/domain.js).
EventEmitter.init = function(opts) {
if (this[kEvents] === undefined ||
this[kEvents] === ObjectGetPrototypeOf(this)[kEvents]) {
this[kEvents] = new SafeMap();

if (this._events === undefined ||
this._events === ObjectGetPrototypeOf(this)._events) {
this._events = { };
this._eventsCount = 0;
}

this._maxListeners = this._maxListeners || undefined;
Expand Down Expand Up @@ -493,11 +462,11 @@ function enhanceStackTrace(err, own) {
EventEmitter.prototype.emit = function emit(type, ...args) {
let doError = (type === 'error');

const events = this[kEvents];
const events = this._events;
if (events !== undefined) {
if (doError && events.has(kErrorMonitor))
if (doError && events[kErrorMonitor] !== undefined)
this.emit(kErrorMonitor, ...args);
doError = (doError && !events.has('error'));
doError = (doError && events.error === undefined);
} else if (!doError)
return false;

Expand Down Expand Up @@ -537,7 +506,7 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
throw err; // Unhandled 'error' event
}

const handler = events.get(type);
const handler = events[type];

if (handler === undefined)
return false;
Expand Down Expand Up @@ -578,32 +547,33 @@ function _addListener(target, type, listener, prepend) {

checkListener(listener);

events = target[kEvents];
events = target._events;
if (events === undefined) {
events = target[kEvents] = new SafeMap();
events = target._events = { __proto__: null };
target._eventsCount = 0;
} else {
// To avoid recursion in the case that type === "newListener"! Before
// adding it to the listeners, first emit "newListener".
if (events.newListener !== undefined) {
target.emit('newListener', type,
listener.listener ?? listener);

// TODO - revisit this comment, user can set it to new object?
// Re-assign `events` because a newListener handler could have caused the
// this[kEvents] to be assigned to a new object
events = target[kEvents];
// this._events to be assigned to a new object
events = target._events;
}
existing = events.get(type);
existing = events[type];
}

if (existing === undefined) {
// Optimize the case of one listener. Don't need the extra array object.
events.set(type, listener);
events[type] = listener;
++target._eventsCount;
} else {
if (typeof existing === 'function') {
// Adding the second element, need to change to array.
existing = prepend ? [listener, existing] : [existing, listener];
events.set(type, existing);
existing = events[type] =
prepend ? [listener, existing] : [existing, listener];
// If we've already got an array, just append.
} else if (prepend) {
existing.unshift(listener);
Expand Down Expand Up @@ -707,20 +677,20 @@ EventEmitter.prototype.removeListener =
function removeListener(type, listener) {
checkListener(listener);

const events = this[kEvents];
const events = this._events;
if (events === undefined)
return this;

const list = events.get(type);
const list = events[type];
if (list === undefined)
return this;

if (list === listener || list.listener === listener) {
if (this[kEvents].size === 1)
this[kEvents] = new SafeMap();
if (--this._eventsCount === 0)
this._events = { __proto__: null };
else {
events.delete(type);
if (events.has('removeListener'))
delete events[type];
if (events.removeListener)
this.emit('removeListener', type, list.listener || listener);
}
} else if (typeof list !== 'function') {
Expand All @@ -745,9 +715,9 @@ EventEmitter.prototype.removeListener =
}

if (list.length === 1)
events.set(type, list[0]);
events[type] = list[0];

if (events.has('removeListener'))
if (events.removeListener !== undefined)
this.emit('removeListener', type, listener);
}

Expand All @@ -765,35 +735,37 @@ EventEmitter.prototype.off = EventEmitter.prototype.removeListener;
*/
EventEmitter.prototype.removeAllListeners =
function removeAllListeners(type) {
const events = this[kEvents];
const events = this._events;
if (events === undefined)
return this;

// Not listening for removeListener, no need to emit
if (!events.has('removeListener')) {
if (events.removeListener === undefined) {
if (arguments.length === 0) {
this[kEvents] = new SafeMap();
} else if (events.has(type)) {
if (this[kEvents].size === 1)
this[kEvents] = new SafeMap();
this._events = { __proto__: null };
this._eventsCount = 0;
} else if (events[type] !== undefined) {
if (--this._eventsCount === 0)
this._events = { __proto__: null };
else
this[kEvents].delete(type);
delete events[type];
}
return this;
}

// Emit removeListener for all listeners on all events
if (arguments.length === 0) {
for (const key of events.keys()) {
for (const key of ReflectOwnKeys(events)) {
if (key === 'removeListener') continue;
this.removeAllListeners(key);
}
this.removeAllListeners('removeListener');
this[kEvents] = new SafeMap();
this._events = { __proto__: null };
this._eventsCount = 0;
return this;
}

const listeners = events.get(type);
const listeners = events[type];

if (typeof listeners === 'function') {
this.removeListener(type, listeners);
Expand All @@ -808,12 +780,12 @@ EventEmitter.prototype.removeAllListeners =
};

function _listeners(target, type, unwrap) {
const events = target[kEvents];
const events = target._events;

if (events === undefined)
return [];

const evlistener = events.get(type);
const evlistener = events[type];
if (evlistener === undefined)
return [];

Expand Down Expand Up @@ -869,10 +841,10 @@ EventEmitter.prototype.listenerCount = listenerCount;
* @returns {number}
*/
function listenerCount(type, listener) {
const events = this[kEvents];
const events = this._events;

if (events !== undefined) {
const evlistener = events.get(type);
const evlistener = events[type];

if (typeof evlistener === 'function') {
if (listener != null) {
Expand Down Expand Up @@ -906,7 +878,7 @@ function listenerCount(type, listener) {
* @returns {any[]}
*/
EventEmitter.prototype.eventNames = function eventNames() {
return this[kEvents].size > 0 ? ArrayFrom(this[kEvents].keys()) : [];
return this._eventsCount > 0 ? ReflectOwnKeys(this._events) : [];
};

function arrayClone(arr) {
Expand Down