From d84a953e2ee9edc3c004147c32831b3530bc15e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Thu, 27 Aug 2015 19:12:41 +0200 Subject: [PATCH] events: use a Map to store listeners --- lib/_stream_readable.js | 10 +- lib/events.js | 106 +++++++++--------- lib/internal/symbols.js | 3 + node.gyp | 1 + src/env.h | 1 - src/node.cc | 3 - ...test-event-emitter-check-listener-leaks.js | 30 ++--- ...st-event-emitter-listeners-side-effects.js | 19 ++-- ...-emitter-set-max-listeners-side-effects.js | 6 +- test/parallel/test-event-emitter-subclass.js | 4 +- test/parallel/test-readline-interface.js | 9 +- 11 files changed, 100 insertions(+), 92 deletions(-) create mode 100644 lib/internal/symbols.js diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 681c3eaec245be..64771bd005c355 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -4,6 +4,7 @@ module.exports = Readable; Readable.ReadableState = ReadableState; const EE = require('events').EventEmitter; +const EEEvents = require('internal/symbols').EEEvents; const Stream = require('stream'); const Buffer = require('buffer').Buffer; const util = require('util'); @@ -542,12 +543,13 @@ Readable.prototype.pipe = function(dest, pipeOpts) { } // This is a brutally ugly hack to make sure that our error handler // is attached before any userland ones. NEVER DO THIS. - if (!dest._events || !dest._events.error) + const events = dest[EEEvents]; + if (!events || !events.has('error')) dest.on('error', onerror); - else if (Array.isArray(dest._events.error)) - dest._events.error.unshift(onerror); + else if (Array.isArray(events.get('error'))) + events.get('error').unshift(onerror); else - dest._events.error = [onerror, dest._events.error]; + events.set('error', [onerror, events.get('error')]); // Both close and finish should trigger unpipe, but only once. diff --git a/lib/events.js b/lib/events.js index 722b64537e109d..5f8efd6df308b4 100644 --- a/lib/events.js +++ b/lib/events.js @@ -1,5 +1,7 @@ 'use strict'; +const EEEvents = require('internal/symbols').EEEvents; + var domain; function EventEmitter() { @@ -13,7 +15,7 @@ EventEmitter.EventEmitter = EventEmitter; EventEmitter.usingDomains = false; EventEmitter.prototype.domain = undefined; -EventEmitter.prototype._events = undefined; +EventEmitter.prototype[EEEvents] = undefined; EventEmitter.prototype._maxListeners = undefined; // By default EventEmitters will print a warning if more than 10 listeners are @@ -30,10 +32,9 @@ EventEmitter.init = function() { } } - if (!this._events || this._events === Object.getPrototypeOf(this)._events) { - this._events = {}; - this._eventsCount = 0; - } + if (!this[EEEvents] || + this[EEEvents] === Object.getPrototypeOf(this)[EEEvents]) + this[EEEvents] = new Map(); this._maxListeners = this._maxListeners || undefined; }; @@ -119,9 +120,9 @@ EventEmitter.prototype.emit = function emit(type) { var needDomainExit = false; var doError = (type === 'error'); - events = this._events; + events = this[EEEvents]; if (events) - doError = (doError && events.error == null); + doError = (doError && !events.has('error')); else if (!doError) return false; @@ -148,7 +149,7 @@ EventEmitter.prototype.emit = function emit(type) { return false; } - handler = events[type]; + handler = events.get(type); if (!handler) return false; @@ -196,32 +197,31 @@ EventEmitter.prototype.addListener = function addListener(type, listener) { if (typeof listener !== 'function') throw new TypeError('listener must be a function'); - events = this._events; + events = this[EEEvents]; if (!events) { - events = this._events = {}; - this._eventsCount = 0; + events = this[EEEvents] = new Map(); } else { // To avoid recursion in the case that type === "newListener"! Before // adding it to the listeners, first emit "newListener". - if (events.newListener) { + if (events.has('newListener')) { this.emit('newListener', type, listener.listener ? listener.listener : listener); // Re-assign `events` because a newListener handler could have caused the - // this._events to be assigned to a new object - events = this._events; + // this[EEEvents] to be assigned to a new object + events = this[EEEvents]; } - existing = events[type]; + existing = events.get(type); } if (!existing) { // Optimize the case of one listener. Don't need the extra array object. - existing = events[type] = listener; - ++this._eventsCount; + events.set(type, listener); } else { if (typeof existing === 'function') { // Adding the second element, need to change to array. - existing = events[type] = [existing, listener]; + existing = [existing, listener]; + events.set(type, existing); } else { // If we've already got an array, just append. existing.push(listener); @@ -267,7 +267,7 @@ EventEmitter.prototype.once = function once(type, listener) { return this; }; -// emits a 'removeListener' event iff the listener was removed +// emits a 'removeListener' event if the listener was removed EventEmitter.prototype.removeListener = function removeListener(type, listener) { var list, events, position, i; @@ -275,22 +275,18 @@ EventEmitter.prototype.removeListener = if (typeof listener !== 'function') throw new TypeError('listener must be a function'); - events = this._events; + events = this[EEEvents]; if (!events) return this; - list = events[type]; + list = events.get(type); if (!list) return this; if (list === listener || (list.listener && list.listener === listener)) { - if (--this._eventsCount === 0) - this._events = {}; - else { - delete events[type]; - if (events.removeListener) - this.emit('removeListener', type, listener); - } + events.delete(type); + if (events.has('removeListener')) + this.emit('removeListener', type, listener); } else if (typeof list !== 'function') { position = -1; @@ -307,17 +303,12 @@ EventEmitter.prototype.removeListener = if (list.length === 1) { list[0] = undefined; - if (--this._eventsCount === 0) { - this._events = {}; - return this; - } else { - delete events[type]; - } + events.delete(type); } else { spliceOne(list, position); } - if (events.removeListener) + if (events.has('removeListener')) this.emit('removeListener', type, listener); } @@ -328,39 +319,32 @@ EventEmitter.prototype.removeAllListeners = function removeAllListeners(type) { var listeners, events; - events = this._events; + events = this[EEEvents]; if (!events) return this; // not listening for removeListener, no need to emit - if (!events.removeListener) { + if (!events.has('removeListener')) { if (arguments.length === 0) { - this._events = {}; - this._eventsCount = 0; - } else if (events[type]) { - if (--this._eventsCount === 0) - this._events = {}; - else - delete events[type]; + events.clear(); + } else { + events.delete(type); } return this; } // emit removeListener for all listeners on all events if (arguments.length === 0) { - var keys = Object.keys(events); - for (var i = 0, key; i < keys.length; ++i) { - key = keys[i]; + for (var key of events.keys()) { if (key === 'removeListener') continue; this.removeAllListeners(key); } this.removeAllListeners('removeListener'); - this._events = {}; - this._eventsCount = 0; + events.clear(); return this; } - listeners = events[type]; + listeners = events.get(type); if (typeof listeners === 'function') { this.removeListener(type, listeners); @@ -377,12 +361,12 @@ EventEmitter.prototype.removeAllListeners = EventEmitter.prototype.listeners = function listeners(type) { var evlistener; var ret; - var events = this._events; + var events = this[EEEvents]; if (!events) ret = []; else { - evlistener = events[type]; + evlistener = events.get(type); if (!evlistener) ret = []; else if (typeof evlistener === 'function') @@ -399,10 +383,10 @@ EventEmitter.listenerCount = function(emitter, type) { }; EventEmitter.prototype.listenerCount = function listenerCount(type) { - const events = this._events; + const events = this[EEEvents]; if (events) { - const evlistener = events[type]; + const evlistener = events.get(type); if (typeof evlistener === 'function') { return 1; @@ -414,6 +398,20 @@ EventEmitter.prototype.listenerCount = function listenerCount(type) { return 0; }; +Object.defineProperty(EventEmitter.prototype, '_events', { + get() { + const thisEvents = this[EEEvents]; + if (!thisEvents) + return; + + const events = {}; + for (var event of thisEvents) { + events[event[0]] = event[1]; + } + return events; + } +}); + // About 1.5x faster than the two-arg version of Array#splice(). function spliceOne(list, index) { for (var i = index, k = i + 1, n = list.length; k < n; i += 1, k += 1) diff --git a/lib/internal/symbols.js b/lib/internal/symbols.js new file mode 100644 index 00000000000000..2d0512f941bfae --- /dev/null +++ b/lib/internal/symbols.js @@ -0,0 +1,3 @@ +'use strict'; + +exports.EEEvents = Symbol('EventEmitter events'); diff --git a/node.gyp b/node.gyp index 7716287d345529..da26ca6a4f3e46 100644 --- a/node.gyp +++ b/node.gyp @@ -71,6 +71,7 @@ 'lib/internal/child_process.js', 'lib/internal/freelist.js', 'lib/internal/socket_list.js', + 'lib/internal/symbols.js', 'lib/internal/repl.js', 'lib/internal/util.js', ], diff --git a/src/env.h b/src/env.h index ce972d598edf99..1b6fbb750e50e7 100644 --- a/src/env.h +++ b/src/env.h @@ -78,7 +78,6 @@ namespace node { V(env_string, "env") \ V(errno_string, "errno") \ V(error_string, "error") \ - V(events_string, "_events") \ V(exec_argv_string, "execArgv") \ V(exec_path_string, "execPath") \ V(exiting_string, "_exiting") \ diff --git a/src/node.cc b/src/node.cc index 084fe900cd35d4..a5767c07f0ae82 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2925,9 +2925,6 @@ void SetupProcessObject(Environment* env, env->SetMethod(process, "_setupNextTick", SetupNextTick); env->SetMethod(process, "_setupPromises", SetupPromises); env->SetMethod(process, "_setupDomainUse", SetupDomainUse); - - // pre-set _events object for faster emit checks - process->Set(env->events_string(), Object::New(env->isolate())); } diff --git a/test/parallel/test-event-emitter-check-listener-leaks.js b/test/parallel/test-event-emitter-check-listener-leaks.js index 90f686b5422c43..4ae52f465acb25 100644 --- a/test/parallel/test-event-emitter-check-listener-leaks.js +++ b/test/parallel/test-event-emitter-check-listener-leaks.js @@ -1,7 +1,9 @@ 'use strict'; +// Flags: --expose-internals var common = require('../common'); var assert = require('assert'); var events = require('events'); +const EEEvents = require('internal/symbols').EEEvents; var e = new events.EventEmitter(); @@ -9,32 +11,32 @@ var e = new events.EventEmitter(); for (var i = 0; i < 10; i++) { e.on('default', function() {}); } -assert.ok(!e._events['default'].hasOwnProperty('warned')); +assert.ok(!e[EEEvents].get('default').hasOwnProperty('warned')); e.on('default', function() {}); -assert.ok(e._events['default'].warned); +assert.ok(e[EEEvents].get('default').warned); // specific e.setMaxListeners(5); for (var i = 0; i < 5; i++) { e.on('specific', function() {}); } -assert.ok(!e._events['specific'].hasOwnProperty('warned')); +assert.ok(!e[EEEvents].get('specific').hasOwnProperty('warned')); e.on('specific', function() {}); -assert.ok(e._events['specific'].warned); +assert.ok(e[EEEvents].get('specific').warned); // only one e.setMaxListeners(1); e.on('only one', function() {}); -assert.ok(!e._events['only one'].hasOwnProperty('warned')); +assert.ok(!e[EEEvents].get('only one').hasOwnProperty('warned')); e.on('only one', function() {}); -assert.ok(e._events['only one'].hasOwnProperty('warned')); +assert.ok(e[EEEvents].get('only one').hasOwnProperty('warned')); // unlimited e.setMaxListeners(0); for (var i = 0; i < 1000; i++) { e.on('unlimited', function() {}); } -assert.ok(!e._events['unlimited'].hasOwnProperty('warned')); +assert.ok(!e[EEEvents].get('unlimited').hasOwnProperty('warned')); // process-wide events.EventEmitter.defaultMaxListeners = 42; @@ -43,25 +45,25 @@ e = new events.EventEmitter(); for (var i = 0; i < 42; ++i) { e.on('fortytwo', function() {}); } -assert.ok(!e._events['fortytwo'].hasOwnProperty('warned')); +assert.ok(!e[EEEvents].get('fortytwo').hasOwnProperty('warned')); e.on('fortytwo', function() {}); -assert.ok(e._events['fortytwo'].hasOwnProperty('warned')); -delete e._events['fortytwo'].warned; +assert.ok(e[EEEvents].get('fortytwo').hasOwnProperty('warned')); +delete e[EEEvents].get('fortytwo').warned; events.EventEmitter.defaultMaxListeners = 44; e.on('fortytwo', function() {}); -assert.ok(!e._events['fortytwo'].hasOwnProperty('warned')); +assert.ok(!e[EEEvents].get('fortytwo').hasOwnProperty('warned')); e.on('fortytwo', function() {}); -assert.ok(e._events['fortytwo'].hasOwnProperty('warned')); +assert.ok(e[EEEvents].get('fortytwo').hasOwnProperty('warned')); // but _maxListeners still has precedence over defaultMaxListeners events.EventEmitter.defaultMaxListeners = 42; e = new events.EventEmitter(); e.setMaxListeners(1); e.on('uno', function() {}); -assert.ok(!e._events['uno'].hasOwnProperty('warned')); +assert.ok(!e[EEEvents].get('uno').hasOwnProperty('warned')); e.on('uno', function() {}); -assert.ok(e._events['uno'].hasOwnProperty('warned')); +assert.ok(e[EEEvents].get('uno').hasOwnProperty('warned')); // chainable assert.strictEqual(e, e.setMaxListeners(1)); diff --git a/test/parallel/test-event-emitter-listeners-side-effects.js b/test/parallel/test-event-emitter-listeners-side-effects.js index 23076128eddfc1..3d5bcaf14535b9 100644 --- a/test/parallel/test-event-emitter-listeners-side-effects.js +++ b/test/parallel/test-event-emitter-listeners-side-effects.js @@ -1,11 +1,11 @@ 'use strict'; - +// Flags: --expose-internals var common = require('../common'); var assert = require('assert'); var events = require('events'); +const EEEvents = require('internal/symbols').EEEvents; var EventEmitter = require('events').EventEmitter; -var assert = require('assert'); var e = new EventEmitter(); var fl; // foo listeners @@ -13,25 +13,26 @@ var fl; // foo listeners fl = e.listeners('foo'); assert(Array.isArray(fl)); assert(fl.length === 0); -assert.deepEqual(e._events, {}); +assert.strictEqual(e[EEEvents].size, 0); e.on('foo', assert.fail); fl = e.listeners('foo'); -assert(e._events.foo === assert.fail); +assert(e[EEEvents].get('foo') === assert.fail); assert(Array.isArray(fl)); assert(fl.length === 1); assert(fl[0] === assert.fail); e.listeners('bar'); -assert(!e._events.hasOwnProperty('bar')); +assert(!e[EEEvents].has('bar')); e.on('foo', assert.ok); fl = e.listeners('foo'); -assert(Array.isArray(e._events.foo)); -assert(e._events.foo.length === 2); -assert(e._events.foo[0] === assert.fail); -assert(e._events.foo[1] === assert.ok); +const foo = e[EEEvents].get('foo'); +assert(Array.isArray(foo)); +assert(foo.length === 2); +assert(foo[0] === assert.fail); +assert(foo[1] === assert.ok); assert(Array.isArray(fl)); assert(fl.length === 2); diff --git a/test/parallel/test-event-emitter-set-max-listeners-side-effects.js b/test/parallel/test-event-emitter-set-max-listeners-side-effects.js index f09f130ab489ed..e9ffa7c6e3d379 100644 --- a/test/parallel/test-event-emitter-set-max-listeners-side-effects.js +++ b/test/parallel/test-event-emitter-set-max-listeners-side-effects.js @@ -1,10 +1,12 @@ 'use strict'; +// Flags: --expose-internals var common = require('../common'); var assert = require('assert'); var events = require('events'); +const EEEvents = require('internal/symbols').EEEvents; var e = new events.EventEmitter(); -assert.deepEqual(e._events, {}); +assert.strictEqual(e[EEEvents].size, 0); e.setMaxListeners(5); -assert.deepEqual(e._events, {}); +assert.strictEqual(e[EEEvents].size, 0); diff --git a/test/parallel/test-event-emitter-subclass.js b/test/parallel/test-event-emitter-subclass.js index fe915be34efc2b..365ed002eb5e28 100644 --- a/test/parallel/test-event-emitter-subclass.js +++ b/test/parallel/test-event-emitter-subclass.js @@ -1,8 +1,10 @@ 'use strict'; +// Flags: --expose-internals var common = require('../common'); var assert = require('assert'); var EventEmitter = require('events').EventEmitter; var util = require('util'); +const EEEvents = require('internal/symbols').EEEvents; util.inherits(MyEE, EventEmitter); @@ -30,7 +32,7 @@ assert.throws(function() { process.on('exit', function() { assert(called); - assert.deepEqual(myee._events, {}); + assert.strictEqual(myee[EEEvents].size, 0); console.log('ok'); }); diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 5880d022502711..bafff3b9427180 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -1,8 +1,10 @@ 'use strict'; +// Flags: --expose-internals var assert = require('assert'); var readline = require('readline'); var EventEmitter = require('events').EventEmitter; var inherits = require('util').inherits; +const EEEvents = require('internal/symbols').EEEvents; function FakeInput() { EventEmitter.call(this); @@ -14,8 +16,7 @@ FakeInput.prototype.write = function() {}; FakeInput.prototype.end = function() {}; function isWarned(emitter) { - for (var name in emitter) { - var listeners = emitter[name]; + for (var listeners of emitter.values()) { if (listeners.warned) return true; } return false; @@ -343,8 +344,8 @@ function isWarned(emitter) { output: process.stdout }); rl.close(); - assert.equal(isWarned(process.stdin._events), false); - assert.equal(isWarned(process.stdout._events), false); + assert.equal(isWarned(process.stdin[EEEvents]), false); + assert.equal(isWarned(process.stdout[EEEvents]), false); } //can create a new readline Interface with a null output arugument