From 8ceefebaf22a99ad3a2b6a08b11dfc1706bf0b58 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Mon, 14 Oct 2024 12:24:32 +0200 Subject: [PATCH] events: optimize EventTarget.addEventListener MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/55312 Fixes: https://github.com/nodejs/node/issues/55311 Reviewed-By: Stephen Belanger Reviewed-By: Vinícius Lourenço Claro Cardoso Reviewed-By: Yagiz Nizipli Reviewed-By: Matteo Collina Reviewed-By: Ruben Bridgewater Reviewed-By: Antoine du Hamel Reviewed-By: Benjamin Gruenbaum --- .../events/eventtarget-add-remove-abort.js | 25 ++++++++ benchmark/events/eventtarget-add-remove.js | 4 +- lib/internal/event_target.js | 59 +++++++++++-------- 3 files changed, 62 insertions(+), 26 deletions(-) create mode 100644 benchmark/events/eventtarget-add-remove-abort.js diff --git a/benchmark/events/eventtarget-add-remove-abort.js b/benchmark/events/eventtarget-add-remove-abort.js new file mode 100644 index 00000000000000..b1565c1f432a3e --- /dev/null +++ b/benchmark/events/eventtarget-add-remove-abort.js @@ -0,0 +1,25 @@ +'use strict'; +const common = require('../common.js'); + +const bench = common.createBenchmark(main, { + n: [1e5], + nListener: [1, 5, 10], +}); + +function main({ n, nListener }) { + const target = new EventTarget(); + const listeners = []; + for (let k = 0; k < nListener; k += 1) + listeners.push(() => {}); + + bench.start(); + for (let i = 0; i < n; i += 1) { + for (let k = listeners.length; --k >= 0;) { + target.addEventListener('abort', listeners[k]); + } + for (let k = listeners.length; --k >= 0;) { + target.removeEventListener('abort', listeners[k]); + } + } + bench.end(n); +} diff --git a/benchmark/events/eventtarget-add-remove.js b/benchmark/events/eventtarget-add-remove.js index a3defce03cfb8d..62048086d5d0bf 100644 --- a/benchmark/events/eventtarget-add-remove.js +++ b/benchmark/events/eventtarget-add-remove.js @@ -2,8 +2,8 @@ const common = require('../common.js'); const bench = common.createBenchmark(main, { - n: [1e6], - nListener: [5, 10], + n: [1e5], + nListener: [1, 5, 10], }); function main({ n, nListener }) { diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index aa0203b930fbe8..99296b83f2e828 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -597,19 +597,40 @@ class EventTarget { if (arguments.length < 2) throw new ERR_MISSING_ARGS('type', 'listener'); - // We validateOptions before the validateListener check because the spec - // requires us to hit getters. - const { - once, - capture, - passive, - signal, - isNodeStyleListener, - weak, - resistStopPropagation, - } = validateEventListenerOptions(options); - - validateAbortSignal(signal, 'options.signal'); + let once = false; + let capture = false; + let passive = false; + let isNodeStyleListener = false; + let weak = false; + let resistStopPropagation = false; + + if (options !== kEmptyObject) { + // We validateOptions before the validateListener check because the spec + // requires us to hit getters. + options = validateEventListenerOptions(options); + + once = options.once; + capture = options.capture; + passive = options.passive; + isNodeStyleListener = options.isNodeStyleListener; + weak = options.weak; + resistStopPropagation = options.resistStopPropagation; + + const signal = options.signal; + + validateAbortSignal(signal, 'options.signal'); + + if (signal) { + if (signal.aborted) { + return; + } + // TODO(benjamingr) make this weak somehow? ideally the signal would + // not prevent the event target from GC. + signal.addEventListener('abort', () => { + this.removeEventListener(type, listener, options); + }, { __proto__: null, once: true, [kWeakHandler]: this, [kResistStopPropagation]: true }); + } + } if (!validateEventListener(listener)) { // The DOM silently allows passing undefined as a second argument @@ -623,18 +644,8 @@ class EventTarget { process.emitWarning(w); return; } - type = webidl.converters.DOMString(type); - if (signal) { - if (signal.aborted) { - return; - } - // TODO(benjamingr) make this weak somehow? ideally the signal would - // not prevent the event target from GC. - signal.addEventListener('abort', () => { - this.removeEventListener(type, listener, options); - }, { __proto__: null, once: true, [kWeakHandler]: this, [kResistStopPropagation]: true }); - } + type = webidl.converters.DOMString(type); let root = this[kEvents].get(type);