From 246c7ca061d2335a946c6fda703ae46a52bf870a Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Tue, 17 Jul 2018 01:04:16 +0300 Subject: [PATCH 1/3] events: use Map in events.js to store _events --- lib/events.js | 60 +++++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/lib/events.js b/lib/events.js index ff1648d6aa13e7..89ebacc0f17a55 100644 --- a/lib/events.js +++ b/lib/events.js @@ -68,7 +68,7 @@ EventEmitter.init = function() { if (this._events === undefined || this._events === Object.getPrototypeOf(this)._events) { - this._events = Object.create(null); + this._events = new Map(); this._eventsCount = 0; } @@ -142,7 +142,7 @@ EventEmitter.prototype.emit = function emit(type, ...args) { const events = this._events; if (events !== undefined) - doError = (doError && events.error === undefined); + doError = (doError && !events.has('error')); else if (!doError) return false; @@ -173,7 +173,7 @@ EventEmitter.prototype.emit = function emit(type, ...args) { throw err; // Unhandled 'error' event } - const handler = events[type]; + const handler = events.get(type); if (handler === undefined) return false; @@ -202,12 +202,12 @@ function _addListener(target, type, listener, prepend) { events = target._events; if (events === undefined) { - events = target._events = Object.create(null); + events = target._events = new Map(); 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) { + if (events.has('newListener')) { target.emit('newListener', type, listener.listener ? listener.listener : listener); @@ -215,18 +215,18 @@ function _addListener(target, type, listener, prepend) { // this._events to be assigned to a new object events = target._events; } - existing = events[type]; + existing = events.get(type); } if (existing === undefined) { // Optimize the case of one listener. Don't need the extra array object. - existing = events[type] = listener; + events.set(type, listener); ++target._eventsCount; } else { if (typeof existing === 'function') { // Adding the second element, need to change to array. - existing = events[type] = - prepend ? [listener, existing] : [existing, listener]; + existing = prepend ? [listener, existing] : [existing, listener]; + events.set(type, existing); // If we've already got an array, just append. } else if (prepend) { existing.unshift(listener); @@ -315,16 +315,17 @@ EventEmitter.prototype.removeListener = if (events === undefined) return this; - list = events[type]; + list = events.get(type); if (list === undefined) return this; if (list === listener || list.listener === listener) { - if (--this._eventsCount === 0) - this._events = Object.create(null); - else { - delete events[type]; - if (events.removeListener) + if (--this._eventsCount === 0) { + // this._events.clear(); + this._events = new Map(); + } else { + events.delete(type); + if (events.has('removeListener')) this.emit('removeListener', type, list.listener || listener); } } else if (typeof list !== 'function') { @@ -350,9 +351,9 @@ EventEmitter.prototype.removeListener = } if (list.length === 1) - events[type] = list[0]; + events.set(type, list[0]); - if (events.removeListener !== undefined) + if (events.has('removeListener')) this.emit('removeListener', type, originalListener || listener); } @@ -370,22 +371,24 @@ EventEmitter.prototype.removeAllListeners = return this; // not listening for removeListener, no need to emit - if (events.removeListener === undefined) { + if (!events.has('removeListener')) { if (arguments.length === 0) { - this._events = Object.create(null); + // this._events.clear(); + this._events = new Map(); this._eventsCount = 0; - } else if (events[type] !== undefined) { + } else if (events.has(type)) { if (--this._eventsCount === 0) - this._events = Object.create(null); + this._events = new Map(); + // this._events.clear(); else - delete events[type]; + events.delete(type); } return this; } // emit removeListener for all listeners on all events if (arguments.length === 0) { - var keys = Object.keys(events); + var keys = Array.from(events.keys()); var key; for (i = 0; i < keys.length; ++i) { key = keys[i]; @@ -393,12 +396,13 @@ EventEmitter.prototype.removeAllListeners = this.removeAllListeners(key); } this.removeAllListeners('removeListener'); - this._events = Object.create(null); + // this._events.clear(); + this._events = new Map(); this._eventsCount = 0; return this; } - listeners = events[type]; + listeners = events.get(type); if (typeof listeners === 'function') { this.removeListener(type, listeners); @@ -418,7 +422,7 @@ function _listeners(target, type, unwrap) { if (events === undefined) return []; - const evlistener = events[type]; + const evlistener = events.get(type); if (evlistener === undefined) return []; @@ -450,7 +454,7 @@ function listenerCount(type) { const events = this._events; if (events !== undefined) { - const evlistener = events[type]; + const evlistener = events.get(type); if (typeof evlistener === 'function') { return 1; @@ -463,7 +467,7 @@ function listenerCount(type) { } EventEmitter.prototype.eventNames = function eventNames() { - return this._eventsCount > 0 ? Reflect.ownKeys(this._events) : []; + return this._eventsCount > 0 ? Array.from(this._events.keys()) : []; }; function arrayClone(arr, n) { From a3af552af91a7f0ac0222f783d81dbdf06f5fc5b Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Wed, 18 Jul 2018 01:42:53 +0300 Subject: [PATCH 2/3] fixup! implement proxy for each EE, add benchmarks --- benchmark/events/ee-add-remove.js | 13 +- benchmark/events/ee-emit-multi.js | 28 +++++ benchmark/events/ee-emit.js | 6 +- benchmark/events/ee-event-names.js | 28 +++++ benchmark/events/ee-listeners-many.js | 2 +- benchmark/events/ee-once.js | 13 +- lib/events.js | 165 ++++++++++++++++++------- test/parallel/test-benchmark-events.js | 2 +- 8 files changed, 202 insertions(+), 55 deletions(-) create mode 100644 benchmark/events/ee-emit-multi.js create mode 100644 benchmark/events/ee-event-names.js diff --git a/benchmark/events/ee-add-remove.js b/benchmark/events/ee-add-remove.js index 54e680f74ae3e1..70b9400ecaa30a 100644 --- a/benchmark/events/ee-add-remove.js +++ b/benchmark/events/ee-add-remove.js @@ -2,16 +2,23 @@ const common = require('../common.js'); const events = require('events'); -const bench = common.createBenchmark(main, { n: [1e6] }); +const bench = common.createBenchmark(main, { + n: [5e6], + staleEventsCount: [0, 5], + listenerCount: [1, 5], +}); -function main({ n }) { +function main({ n, staleEventsCount, listenerCount }) { const ee = new events.EventEmitter(); const listeners = []; var k; - for (k = 0; k < 10; k += 1) + for (k = 0; k < listenerCount; k += 1) listeners.push(function() {}); + for (k = 0; k < staleEventsCount; k++) + ee.on(`dummyunused${k}`, () => {}); + bench.start(); for (var i = 0; i < n; i += 1) { const dummy = (i % 2 === 0) ? 'dummy0' : 'dummy1'; diff --git a/benchmark/events/ee-emit-multi.js b/benchmark/events/ee-emit-multi.js new file mode 100644 index 00000000000000..09194e63215e22 --- /dev/null +++ b/benchmark/events/ee-emit-multi.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common.js'); +const EventEmitter = require('events').EventEmitter; + +const bench = common.createBenchmark(main, { + n: [2e7], + listenerCount: [1, 5, 10], +}); + +function main({ n, listenerCount }) { + const ee = new EventEmitter(); + + for (var k = 0; k < listenerCount; k += 1) { + ee.on('dummy', function() {}); + ee.on(`dummy${k}`, function() {}); + } + + bench.start(); + for (var i = 0; i < n; i += 1) { + if (i % 3 === 0) + ee.emit('dummy', true, 5); + else if (i % 2 === 0) + ee.emit('dummy', true, 5, 10, false); + else + ee.emit('dummy'); + } + bench.end(n); +} diff --git a/benchmark/events/ee-emit.js b/benchmark/events/ee-emit.js index 686ed10d3ecbfd..a976f47a3077d7 100644 --- a/benchmark/events/ee-emit.js +++ b/benchmark/events/ee-emit.js @@ -5,13 +5,13 @@ const EventEmitter = require('events').EventEmitter; const bench = common.createBenchmark(main, { n: [2e6], argc: [0, 2, 4, 10], - listeners: [1, 5, 10], + listenerCount: [1, 5, 10], }); -function main({ n, argc, listeners }) { +function main({ n, argc, listenerCount }) { const ee = new EventEmitter(); - for (var k = 0; k < listeners; k += 1) + for (var k = 0; k < listenerCount; k += 1) ee.on('dummy', function() {}); var i; diff --git a/benchmark/events/ee-event-names.js b/benchmark/events/ee-event-names.js new file mode 100644 index 00000000000000..0f262c6a471aec --- /dev/null +++ b/benchmark/events/ee-event-names.js @@ -0,0 +1,28 @@ +'use strict'; +const common = require('../common.js'); +const EventEmitter = require('events').EventEmitter; + +const bench = common.createBenchmark(main, { n: [1e6] }); + +function main({ n }) { + const ee = new EventEmitter(); + + for (var k = 0; k < 9; k += 1) { + ee.on(`dummy0${k}`, function() {}); + ee.on(`dummy1${k}`, function() {}); + ee.on(`dummy2${k}`, function() {}); + } + + ee.removeAllListeners('dummy01'); + ee.removeAllListeners('dummy11'); + ee.removeAllListeners('dummy21'); + ee.removeAllListeners('dummy06'); + ee.removeAllListeners('dummy16'); + ee.removeAllListeners('dummy26'); + + bench.start(); + for (var i = 0; i < n; i += 1) { + ee.eventNames(); + } + bench.end(n); +} diff --git a/benchmark/events/ee-listeners-many.js b/benchmark/events/ee-listeners-many.js index 9a1562eb2c005c..4542619cc9dc36 100644 --- a/benchmark/events/ee-listeners-many.js +++ b/benchmark/events/ee-listeners-many.js @@ -2,7 +2,7 @@ const common = require('../common.js'); const EventEmitter = require('events').EventEmitter; -const bench = common.createBenchmark(main, { n: [5e6] }); +const bench = common.createBenchmark(main, { n: [1e7] }); function main({ n }) { const ee = new EventEmitter(); diff --git a/benchmark/events/ee-once.js b/benchmark/events/ee-once.js index e1a09fb4b71167..6b2d0c97a73ed5 100644 --- a/benchmark/events/ee-once.js +++ b/benchmark/events/ee-once.js @@ -2,17 +2,22 @@ const common = require('../common.js'); const EventEmitter = require('events').EventEmitter; -const bench = common.createBenchmark(main, { n: [2e7] }); +const bench = common.createBenchmark(main, { + n: [5e6], + listenerCount: [1, 5, 10], +}); -function main({ n }) { + +function main({ n, listenerCount }) { const ee = new EventEmitter(); function listener() {} bench.start(); - for (var i = 0; i < n; i += 1) { + for (var i = 0; i < n; ++i) { const dummy = (i % 2 === 0) ? 'dummy0' : 'dummy1'; - ee.once(dummy, listener); + for (var j = 0; j < listenerCount; ++j) + ee.once(dummy, listener); ee.emit(dummy); } bench.end(n); diff --git a/lib/events.js b/lib/events.js index 89ebacc0f17a55..91d114bbaa091c 100644 --- a/lib/events.js +++ b/lib/events.js @@ -21,6 +21,9 @@ 'use strict'; +const kEvents = Symbol('events'); +const kEventsProxy = Symbol('events-proxy'); + var spliceOne; function EventEmitter() { @@ -33,7 +36,8 @@ EventEmitter.EventEmitter = EventEmitter; EventEmitter.usingDomains = false; -EventEmitter.prototype._events = undefined; +EventEmitter.prototype[kEvents] = undefined; +EventEmitter.prototype[kEventsProxy] = undefined; EventEmitter.prototype._eventsCount = 0; EventEmitter.prototype._maxListeners = undefined; @@ -64,11 +68,80 @@ Object.defineProperty(EventEmitter, 'defaultMaxListeners', { } }); +const proxyEventsHandler = { + get(emitter, prop) { + const events = emitter[kEvents]; + if (events === undefined) + return undefined; + return events.get(prop); + }, + set(emitter, prop, value) { + let events = emitter[kEvents]; + if (events === undefined) { + events = emitter[kEvents] = new Map(); + emitter._eventsCount = 0; + } + events.set(prop, value); + return true; + }, + has(emitter, prop) { + const events = emitter[kEvents]; + return events !== undefined && events.has(prop); + }, + getPrototypeOf() { + return null; + }, + deleteProperty(emitter, prop) { + const events = emitter[kEvents]; + if (events !== undefined) + events.delete(prop); + return true; + }, + ownKeys(emitter) { + return emitter.eventNames(); + }, + defineProperty(emitter, prop, descriptor) { + if (!descriptor.writable || !descriptor.configurable) + return false; + proxyEventsHandler.set(emitter, prop, descriptor.value); + return true; + }, + getOwnPropertyDescriptor(emitter, prop) { + const value = proxyEventsHandler.get(emitter, prop); + if (value === undefined) + return undefined; + return { + value, + writable: true, + enumerable: true, + configurable: true, + }; + }, +}; + +Object.defineProperty(EventEmitter.prototype, '_events', { + enumerable: true, + get: function() { + let proxy = this[kEventsProxy]; + if (proxy === undefined && this[kEvents] !== undefined) + proxy = new Proxy(this, proxyEventsHandler); + return proxy; + }, + set: function(value) { + if (this[kEvents] === undefined) + this.init(); + if (!value || Array.isArray(value) && value.length === 0) { + this[kEvents] = new Map(); + this._eventsCount = 0; + } + } +}); + EventEmitter.init = function() { - if (this._events === undefined || - this._events === Object.getPrototypeOf(this)._events) { - this._events = new Map(); + if (this[kEvents] === undefined || + this[kEvents] === Object.getPrototypeOf(this)[kEvents]) { + this[kEvents] = new Map(); this._eventsCount = 0; } @@ -140,7 +213,7 @@ function enhanceStackTrace(err, own) { EventEmitter.prototype.emit = function emit(type, ...args) { let doError = (type === 'error'); - const events = this._events; + const events = this[kEvents]; if (events !== undefined) doError = (doError && !events.has('error')); else if (!doError) @@ -200,11 +273,8 @@ function _addListener(target, type, listener, prepend) { throw new errors.ERR_INVALID_ARG_TYPE('listener', 'Function', listener); } - events = target._events; - if (events === undefined) { - events = target._events = new Map(); - target._eventsCount = 0; - } else { + events = target[kEvents]; + if (events !== undefined) { // To avoid recursion in the case that type === "newListener"! Before // adding it to the listeners, first emit "newListener". if (events.has('newListener')) { @@ -212,10 +282,13 @@ function _addListener(target, type, listener, prepend) { 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 = target._events; + // this[kEvents] to be assigned to a new object + events = target[kEvents]; } existing = events.get(type); + } else { + events = target[kEvents] = new Map(); + target._eventsCount = 0; } if (existing === undefined) { @@ -228,10 +301,10 @@ function _addListener(target, type, listener, prepend) { existing = prepend ? [listener, existing] : [existing, listener]; events.set(type, existing); // If we've already got an array, just append. - } else if (prepend) { - existing.unshift(listener); - } else { + } else if (!prepend) { existing.push(listener); + } else { + existing.unshift(listener); } // Check for listener leak @@ -311,7 +384,7 @@ EventEmitter.prototype.removeListener = throw new errors.ERR_INVALID_ARG_TYPE('listener', 'Function', listener); } - events = this._events; + events = this[kEvents]; if (events === undefined) return this; @@ -320,14 +393,10 @@ EventEmitter.prototype.removeListener = return this; if (list === listener || list.listener === listener) { - if (--this._eventsCount === 0) { - // this._events.clear(); - this._events = new Map(); - } else { - events.delete(type); - if (events.has('removeListener')) - this.emit('removeListener', type, list.listener || listener); - } + --this._eventsCount; + events.delete(type); + if (events.has('removeListener')) + this.emit('removeListener', type, list.listener || listener); } else if (typeof list !== 'function') { position = -1; @@ -344,6 +413,8 @@ EventEmitter.prototype.removeListener = if (position === 0) list.shift(); + else if (position === list.length - 1) + list.length -= 1; else { if (spliceOne === undefined) spliceOne = require('internal/util').spliceOne; @@ -366,38 +437,37 @@ EventEmitter.prototype.removeAllListeners = function removeAllListeners(type) { var listeners, events, i; - events = this._events; + events = this[kEvents]; if (events === undefined) return this; // not listening for removeListener, no need to emit if (!events.has('removeListener')) { if (arguments.length === 0) { - // this._events.clear(); - this._events = new Map(); + this[kEvents] = new Map(); this._eventsCount = 0; - } else if (events.has(type)) { - if (--this._eventsCount === 0) - this._events = new Map(); - // this._events.clear(); - else - events.delete(type); + } else if (events.delete(type)) { + --this._eventsCount; } return this; } // emit removeListener for all listeners on all events if (arguments.length === 0) { - var keys = Array.from(events.keys()); + const len = events.size; + var keys = new Array(len); + i = 0; + for (var value of events.keys()) + keys[i++] = value; + var key; - for (i = 0; i < keys.length; ++i) { + for (i = 0; i < len; ++i) { key = keys[i]; if (key === 'removeListener') continue; this.removeAllListeners(key); } this.removeAllListeners('removeListener'); - // this._events.clear(); - this._events = new Map(); + this[kEvents] = new Map(); this._eventsCount = 0; return this; } @@ -417,7 +487,7 @@ EventEmitter.prototype.removeAllListeners = }; function _listeners(target, type, unwrap) { - const events = target._events; + const events = target[kEvents]; if (events === undefined) return []; @@ -451,23 +521,32 @@ EventEmitter.listenerCount = function(emitter, type) { EventEmitter.prototype.listenerCount = listenerCount; function listenerCount(type) { - const events = this._events; + const events = this[kEvents]; if (events !== undefined) { const evlistener = events.get(type); - if (typeof evlistener === 'function') { + if (typeof evlistener === 'function') return 1; - } else if (evlistener !== undefined) { + else if (evlistener !== undefined) return evlistener.length; - } } return 0; } EventEmitter.prototype.eventNames = function eventNames() { - return this._eventsCount > 0 ? Array.from(this._events.keys()) : []; + if (this._eventsCount === 0) + return []; + + const map = this[kEvents]; + const len = map.size; + const names = new Array(len); + var i = 0; + for (var key of map.keys()) { + names[i++] = key; + } + return names; }; function arrayClone(arr, n) { diff --git a/test/parallel/test-benchmark-events.js b/test/parallel/test-benchmark-events.js index 06be60a84abb5b..98c80566c7b3ec 100644 --- a/test/parallel/test-benchmark-events.js +++ b/test/parallel/test-benchmark-events.js @@ -5,5 +5,5 @@ require('../common'); const runBenchmark = require('../common/benchmark'); runBenchmark('events', - ['argc=0', 'listeners=1', 'n=1'], + ['argc=0', 'staleEventsCount=0', 'listenerCount=1', 'n=1'], { NODEJS_BENCHMARK_ZERO_ALLOWED: 1 }); From e694796a0de9c420f06bb95ce9344d702182278f Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Sat, 21 Jul 2018 15:58:38 +0300 Subject: [PATCH 3/3] try V8 fix of limiting Map shrink until capacity >= 16 --- deps/v8/src/builtins/builtins-collections-gen.cc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/deps/v8/src/builtins/builtins-collections-gen.cc b/deps/v8/src/builtins/builtins-collections-gen.cc index 5c3f26374689dc..6b706e03622608 100644 --- a/deps/v8/src/builtins/builtins-collections-gen.cc +++ b/deps/v8/src/builtins/builtins-collections-gen.cc @@ -1474,14 +1474,17 @@ TF_BUILTIN(MapPrototypeDelete, CollectionsBuiltinsAssembler) { LoadFixedArrayElement(table, OrderedHashMap::kNumberOfBucketsIndex); // If there fewer elements than #buckets / 2, shrink the table. - Label shrink(this); - GotoIf(SmiLessThan(SmiAdd(number_of_elements, number_of_elements), - number_of_buckets), - &shrink); - Return(TrueConstant()); + Label dont_shrink(this); + GotoIf(SmiGreaterThanOrEqual(SmiAdd(number_of_elements, number_of_elements), + number_of_buckets), + &dont_shrink); + // If #buckets is less than 8 then don't shrink the table + GotoIf(SmiLessThan(number_of_buckets, SmiConstant(8)), + &dont_shrink); - BIND(&shrink); CallRuntime(Runtime::kMapShrink, context, receiver); + + BIND(&dont_shrink); Return(TrueConstant()); }