From 47e490f0840feedcc075f3e4f9c24bf6f3b48a20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Mon, 27 Jan 2025 07:48:50 -0800 Subject: [PATCH] Make EventTarget compatible with the existing implementation of ReadOnlyNode (#48427) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/48427 Changelog: [internal] The `ReactNativeElement` class was refactored for performance reasons, and the current implementation does **NOT** call `super()`, and it inlines the parent constructor instead. When it eventually extends `EventTarget`, things won't work as expected because the existing `EventTarget` implementation has constructor dependencies. This refactors the current implementation of `EventTarget` to eliminate those constructor side-effects, and eliminates the constructor altogether. This breaks encapsulation, but it has some positive side-effects on performance: 1. Creating `EventTarget` instances is faster because it has no constructor logic. 2. Improves memory by not creating maps to hold the event listeners if no event listeners are ever added to the target (which is very common). 3. Improves the overall runtime performance of the methods in the class by migrating away from private methods (which are known to be slow on the babel transpiled version we're currently using). Extra: it also simplifies making window/the global scope implement the EventTarget interface :) ## Benchmark results Before: | Latency average (ns) | Latency median (ns) | Samples | Task name | Throughput average (ops/s) | Throughput median (ops/s) | | ---|--- |--- |--- |---|---| | 8234.22 ± 0.27% | 8132.00 | 121445 | dispatchEvent, no bubbling, no listeners | 122323 ± 0.02% | 122971 | | 9001.22 ± 0.41% | 8883.00 | 111097 | dispatchEvent, no bubbling, single listener | 111981 ± 0.02% | 112575 | | 51777.94 ± 0.58% | 51247.00 | 19314 | dispatchEvent, no bubbling, multiple listeners | 19393 ± 0.04% | 19513 | | 8256.65 ± 0.29% | 8152.00 | 121115 | dispatchEvent, bubbling, no listeners | 122031 ± 0.02% | 122669 | | 9064.32 ± 0.44% | 8933.00 | 110323 | dispatchEvent, bubbling, single listener per target | 111265 ± 0.02% | 111944 | | 51879.66 ± 0.27% | 51447.00 | 19276 | dispatchEvent, bubbling, multiple listeners per target | 19325 ± 0.04% | 19437 | After: | Latency average (ns) | Latency median (ns) | Samples | Task name | Throughput average (ops/s) | Throughput median (ops/s)| | ---------------------|---------------------|---------|--------------------------------------------------------|----------------------------|--------------------------| | 5664.62 ± 0.50% | 5588.00 | 176535 | dispatchEvent, no bubbling, no listeners | 178219 ± 0.02% | 178955 | | 7232.86 ± 0.50% | 7131.00 | 138258 | dispatchEvent, no bubbling, single listener | 139540 ± 0.02% | 140233 | | 50957.51 ± 0.71% | 50336.00 | 19625 | dispatchEvent, no bubbling, multiple listeners | 19751 ± 0.04% | 19866 | | 5692.36 ± 0.50% | 5618.00 | 175675 | dispatchEvent, bubbling, no listeners | 177315 ± 0.02% | 177999 | | 7277.82 ± 0.38% | 7181.00 | 137404 | dispatchEvent, bubbling, single listener per target | 138560 ± 0.02% | 139256 | | 50493.64 ± 0.28% | 50105.00 | 19805 | dispatchEvent, bubbling, multiple listeners per target | 19855 ± 0.04% | 19958 | Reviewed By: yungsters Differential Revision: D67758408 fbshipit-source-id: f8da1788251c9e21377de5ab730875bcc7610361 --- .../private/webapis/dom/events/EventTarget.js | 71 +++++++++++++------ 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/packages/react-native/src/private/webapis/dom/events/EventTarget.js b/packages/react-native/src/private/webapis/dom/events/EventTarget.js index a3b92b5923d..e7ae4438f8e 100644 --- a/packages/react-native/src/private/webapis/dom/events/EventTarget.js +++ b/packages/react-native/src/private/webapis/dom/events/EventTarget.js @@ -57,6 +57,11 @@ type EventListenerRegistration = { removed: boolean, }; +type ListenersMap = Map>; + +const CAPTURING_LISTENERS_KEY = Symbol('capturingListeners'); +const BUBBLING_LISTENERS_KEY = Symbol('bubblingListeners'); + function getDefaultPassiveValue( type: string, eventTarget: EventTarget, @@ -65,9 +70,6 @@ function getDefaultPassiveValue( } export default class EventTarget { - #listeners: Map> = new Map(); - #captureListeners: Map> = new Map(); - addEventListener( type: string, callback: EventListener | null, @@ -120,11 +122,15 @@ export default class EventTarget { return; } - const listenerMap = capture ? this.#captureListeners : this.#listeners; - let listenerList = listenerMap.get(processedType); + let listenersMap = this._getListenersMap(capture); + let listenerList = listenersMap?.get(processedType); if (listenerList == null) { + if (listenersMap == null) { + listenersMap = new Map(); + this._setListenersMap(capture, listenersMap); + } listenerList = []; - listenerMap.set(processedType, listenerList); + listenersMap.set(processedType, listenerList); } else { for (const listener of listenerList) { if (listener.callback === callback) { @@ -147,7 +153,7 @@ export default class EventTarget { signal.addEventListener( 'abort', () => { - this.#removeEventListenerRegistration(listener, nonNullListenerList); + this._removeEventListenerRegistration(listener, nonNullListenerList); }, { once: true, @@ -180,8 +186,8 @@ export default class EventTarget { ? optionsOrUseCapture : Boolean(optionsOrUseCapture.capture); - const listenerMap = capture ? this.#captureListeners : this.#listeners; - const listenerList = listenerMap.get(processedType); + const listenersMap = this._getListenersMap(capture); + const listenerList = listenersMap?.get(processedType); if (listenerList == null) { return; } @@ -212,7 +218,7 @@ export default class EventTarget { setIsTrusted(event, false); - this.#dispatch(event); + this._dispatch(event); return !event.defaultPrevented; } @@ -225,10 +231,10 @@ export default class EventTarget { * Implements the "event dispatch" concept * (see https://dom.spec.whatwg.org/#concept-event-dispatch). */ - #dispatch(event: Event): void { + _dispatch(event: Event): void { setEventDispatchFlag(event, true); - const eventPath = this.#getEventPath(event); + const eventPath = this._getEventPath(event); setComposedPath(event, eventPath); setTarget(event, this); @@ -242,7 +248,7 @@ export default class EventTarget { event, target === this ? Event.AT_TARGET : Event.CAPTURING_PHASE, ); - target.#invoke(event, Event.CAPTURING_PHASE); + target._invoke(event, Event.CAPTURING_PHASE); } for (const target of eventPath) { @@ -260,7 +266,7 @@ export default class EventTarget { event, target === this ? Event.AT_TARGET : Event.BUBBLING_PHASE, ); - target.#invoke(event, Event.BUBBLING_PHASE); + target._invoke(event, Event.BUBBLING_PHASE); } setEventPhase(event, Event.NONE); @@ -278,7 +284,7 @@ export default class EventTarget { * * The return value is also set as `composedPath` for the event. */ - #getEventPath(event: Event): $ReadOnlyArray { + _getEventPath(event: Event): $ReadOnlyArray { const path = []; // eslint-disable-next-line consistent-this let target: EventTarget | null = this; @@ -296,20 +302,21 @@ export default class EventTarget { * Implements the event listener invoke concept * (see https://dom.spec.whatwg.org/#concept-event-listener-invoke). */ - #invoke(event: Event, eventPhase: EventPhase) { - const listenerMap = - eventPhase === Event.CAPTURING_PHASE - ? this.#captureListeners - : this.#listeners; + _invoke(event: Event, eventPhase: EventPhase) { + const listenersMap = this._getListenersMap( + eventPhase === Event.CAPTURING_PHASE, + ); setCurrentTarget(event, this); // This is a copy so listeners added during dispatch are NOT executed. - const listenerList = listenerMap.get(event.type)?.slice(); + const listenerList = listenersMap?.get(event.type)?.slice(); if (listenerList == null) { return; } + setCurrentTarget(event, this); + for (const listener of listenerList) { if (listener.removed) { continue; @@ -356,7 +363,7 @@ export default class EventTarget { } } - #removeEventListenerRegistration( + _removeEventListenerRegistration( registration: EventListenerRegistration, listenerList: Array, ): void { @@ -371,6 +378,24 @@ export default class EventTarget { } } + _getListenersMap(isCapture: boolean): ?ListenersMap { + return isCapture + ? // $FlowExpectedError[prop-missing] + this[CAPTURING_LISTENERS_KEY] + : // $FlowExpectedError[prop-missing] + this[BUBBLING_LISTENERS_KEY]; + } + + _setListenersMap(isCapture: boolean, listenersMap: ListenersMap): void { + if (isCapture) { + // $FlowExpectedError[prop-missing] + this[CAPTURING_LISTENERS_KEY] = listenersMap; + } else { + // $FlowExpectedError[prop-missing] + this[BUBBLING_LISTENERS_KEY] = listenersMap; + } + } + /** * This a "protected" method to be overridden by a subclass to allow event * propagation. @@ -388,7 +413,7 @@ export default class EventTarget { */ // $FlowExpectedError[unsupported-syntax] [INTERNAL_DISPATCH_METHOD_KEY](event: Event): void { - this.#dispatch(event); + this._dispatch(event); } }