8000 fix: avoid creating functions on each handleEvent() call · NativeScript/NativeScript@f08d122 · GitHub
[go: up one dir, main page]
Skip to content

Commit f08d122

Browse files
shirakabashirakaba
committed
fix: avoid creating functions on each handleEvent() call
1 parent 66f62fb commit f08d122

File tree

1 file changed

+37
-32
lines changed

1 file changed

+37
-32
lines changed

‎packages/core/data/dom-events/dom-event.ts

Lines changed: 37 additions & 32 deletions
Original file line numberDiff line numberDiff line change
< 8000 button class="Button Button--iconOnly Button--invisible ExpandableHunkHeaderDiffLine-module__expand-button-line--wZKjF ExpandableHunkHeaderDiffLine-module__expand-button-unified--Eae6C" aria-label="Expand file up from line 231" data-direction="up" aria-hidden="true" tabindex="-1">
@@ -231,6 +231,26 @@ export class DOMEvent implements Event {
231231
this.propagationState = EventPropagationState.stop;
232232
}
233233

234+
// During handleEvent(), we want to work on a copy of the listeners array,
235+
// as any callback could modify the original array during the loop.
236+
//
237+
// However, cloning arrays is expensive on this hot path, so we'll do it
238+
// lazily - i.e. only take a clone if a mutation is about to happen.
239+
// This optimisation is particularly worth doing as it's very rare that
240+
// an event listener callback will end up modifying the listeners array.
241+
private listenersLive: MutationSensitiveArray<ListenerEntry> = emptyArray;
242+
private listenersLazyCopy: ListenerEntry[] = emptyArray;
243+
244+
// Creating this upon class construction as an arrow function rather than as
245+
// an inline function bound afresh on each usage saves about 210 nanoseconds
246+
// per run of handleEvent().
247+
private onCurrentListenersMutation = () => {
248+
// Cloning the array via spread syntax is up to 180 nanoseconds
249+
// faster per run than using Array.prototype.slice().
250+
this.listenersLazyCopy = [...this.listenersLive];
251+
this.listenersLive.onMutation = null;
252+
};
253+
234254
/**
235255
* Dispatches a synthetic event event to target and returns true if either
236256
* event's cancelable attribute value is false or its preventDefault()
@@ -257,6 +277,8 @@ export class DOMEvent implements Event {
257277
this.target = null;
258278
this.eventPhase = this.NONE;
259279
this.propagationState = EventPropagationState.resume;
280+
this.listenersLive = emptyArray;
281+
this.listenersLazyCopy = emptyArray;
260282
};
261283

262284
// `Observable.removeEventListener` would likely suffice, but grabbing
@@ -282,10 +304,10 @@ export class DOMEvent implements Event {
282304
// event. This keeps behaviour as consistent with DOM Events as
283305
// possible.
284306

307+
this.listenersLazyCopy = this.listenersLive = getGlobalEventHandlersPreHandling?.() || emptyArray;
285308
this.handleEvent({
286309
data,
287310
isGlobal: true,
288-
getListenersForType: () => getGlobalEventHandlersPreHandling?.() ?? emptyArray,
289311
removeEventListener: removeGlobalEventListener,
290312
phase: this.CAPTURING_PHASE,
291313
});
@@ -297,13 +319,14 @@ export class DOMEvent implements Event {
297319
this.currentTarget = currentTarget;
298320
this.eventPhase = this.target === this.currentTarget ? this.AT_TARGET : this.CAPTURING_PHASE;
299321

322+
this.listenersLazyCopy = this.listenersLive = currentTarget.getEventList(this.type) || emptyArray;
300323
this.handleEvent({
301324
data,
302325
isGlobal: false,
303-
getListenersForType: () => currentTarget.getEventList(this.type) ?? emptyArray,
304326
removeEventListener: currentTarget.removeEventListener.bind(currentTarget) as Observable['removeEventListener'],
305327
phase: this.CAPTURING_PHASE,
306328
});
329+
307330
if (this.propagationState !== EventPropagationState.resume) {
308331
reset();
309332
return this.returnValue;
@@ -316,13 +339,14 @@ export class DOMEvent implements Event {
316339
const currentTarget = eventPath[i];
317340
this.eventPhase = this.target === this< 8000 span class=pl-kos>.currentTarget ? this.AT_TARGET : this.BUBBLING_PHASE;
318341

342+
this.listenersLazyCopy = this.listenersLive = currentTarget.getEventList(this.type) || emptyArray;
319343
this.handleEvent({
320344
data,
321345
isGlobal: false,
322-
getListenersForType: () => currentTarget.getEventList(this.type) ?? emptyArray,
323346
removeEventListener: currentTarget.removeEventListener.bind(currentTarget) as Observable['removeEventListener'],
324347
phase: this.BUBBLING_PHASE,
325348
});
349+
326350
if (this.propagationState !== EventPropagationState.resume) {
327351
reset();
328352
return this.returnValue;
@@ -341,10 +365,10 @@ export class DOMEvent implements Event {
341365
this.eventPhase = this.BUBBLING_PHASE;
342366
}
343367

368+
this.listenersLazyCopy = this.listenersLive = getGlobalEventHandlersPostHandling?.() || emptyArray;
344369
this.handleEvent({
345370
data,
346371
isGlobal: true,
347-
getListenersForType: () => getGlobalEventHandlersPostHandling?.() ?? emptyArray,
348372
removeEventListener: removeGlobalEventListener,
349373
phase: this.BUBBLING_PHASE,
350374
});
@@ -353,32 +377,12 @@ export class DOMEvent implements Event {
353377
return this.returnValue;
354378
}
355379

356-
private handleEvent({ data, isGlobal, getListenersForType, phase, removeEventListener }: { data: EventData; isGlobal: boolean; getListenersForType: () => MutationSensitiveArray<ListenerEntry>; phase: 0 | 1 | 2 | 3; removeEventListener: (eventName: string, callback?: any, thisArg?: any, capture?: boolean) => void }) {
357-
// We want to work on a copy of the array, as any callback could modify
358-
// the original array during the loop.
359-
//
360-
// However, cloning arrays is expensive on this hot path, so we'll do it
361-
// lazily - i.e. only take a clone if a mutation is about to happen.
362-
// This optimisation is particularly worth doing as it's very rare that
363-
// an event listener callback will end up modifying the listeners array.
364-
const listenersLive: MutationSensitiveArray<ListenerEntry> = getListenersForType();
365-
380+
private handleEvent({ data, isGlobal, phase, removeEventListener }: { data: EventData; isGlobal: boolean; phase: 0 | 1 | 2 | 3; removeEventListener: (eventName: string, callback?: any, thisArg?: any, capture?: boolean) => void }) {
366381
// Set a listener to clone the array just before any mutations.
367-
let listenersLazyCopy: ListenerEntry[] = listenersLive;
368-
listenersLive.onMutation = () => (mutation: string, payload?: unknown) => {
369-
console.log(`handleEvent "${data.eventName}": doLazyCopy due to "${mutation}"`, payload);
370-
// Cloning the array via spread syntax is up to 180 nanoseconds
371-
// faster per run than using Array.prototype.slice().
372-
listenersLazyCopy = [...listenersLive];
373-
listenersLive.onMutation = null;
374-
};
382+
this.listenersLive.onMutation = this.onCurrentListenersMutation;
375383

376-
// Make sure we clear the callback before we exit the function,
377-
// otherwise we may wastefully clone the array on future mutations.
378-
const cleanup = () => (listenersLive.onMutation = null);
379-
380-
for (let i = listenersLazyCopy.length - 1; i >= 0; i--) {
381-
const listener = listenersLazyCopy[i];
384+
for (let i = this.listenersLazyCopy.length - 1; i >= 0; i--) {
385+
const listener = this.listenersLazyCopy[i];
382386

383387
// Assigning variables this old-fashioned way is up to 50
384388
// nanoseconds faster per run than ESM destructuring syntax.
@@ -394,7 +398,7 @@ export class DOMEvent implements Event {
394398
// We simply use a strict equality check here because we trust that
395399
// the listeners provider will never allow two deeply-equal
396400
// listeners into the array.
397-
if (!listenersLive.includes(listener)) {
401+
if (!this.listenersLive.includes(listener)) {
398402
continue;
399403
}
400404

@@ -424,12 +428,13 @@ export class DOMEvent implements Event {
424428
}
425429

426430
if (this.propagationState === EventPropagationState.stopImmediate) {
427-
cleanup();
428-
return;
431+
break;
429432
}
430433
}
431434

432-
cleanup();
435+
// Make sure we clear the callback before we exit the function,
436+
// otherwise we may wastefully clone the array on future mutations.
437+
this.listenersLive.onMutation = null;
433438
}
434439
}
435440

0 commit comments

Comments
 (0)
0