Skip to content

Commit 2035fca

Browse files
Connor ClarkDevtools-frontend LUCI CQ
authored andcommitted
[RPP] Improve performance of singleton overlays
Every call to `add` a new overlay would iterate over all existing overlays to manage singletons specially. For an extreme case of ~70k overlays, this resulted in ~22s of extra processing time on my M1 Mac. This CL reduces it down to 5ms by simply storing singleton overlays in a map for constant time lookup. Example trace w/ many overlays on 3p insight: https://trace.cafe/t/V7sS5bz7F0 Bug: 40278532 Change-Id: Ia8b76892e4f22f1a60215fffcc999fb4862121a0 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6376737 Commit-Queue: Connor Clark <[email protected]> Auto-Submit: Connor Clark <[email protected]> Reviewed-by: Paul Irish <[email protected]>
1 parent 166922f commit 2035fca

File tree

1 file changed

+37
-5
lines changed

1 file changed

+37
-5
lines changed

front_end/panels/timeline/overlays/OverlaysImpl.ts

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,11 @@ export interface TimelineOverlaySetOptions {
332332
*/
333333
type SingletonOverlay = EntrySelected|TimestampMarker;
334334
export function overlayIsSingleton(overlay: TimelineOverlay): overlay is SingletonOverlay {
335-
return overlay.type === 'TIMESTAMP_MARKER' || overlay.type === 'ENTRY_SELECTED';
335+
return overlayTypeIsSingleton(overlay.type);
336+
}
337+
338+
export function overlayTypeIsSingleton(type: TimelineOverlay['type']): type is SingletonOverlay['type'] {
339+
return type === 'TIMESTAMP_MARKER' || type === 'ENTRY_SELECTED';
336340
}
337341

338342
/**
@@ -454,6 +458,8 @@ export class Overlays extends EventTarget {
454458
*/
455459
#overlaysToElements = new Map<TimelineOverlay, HTMLElement|null>();
456460

461+
#singletonOverlays = new Map<SingletonOverlay['type'], TimelineOverlay>();
462+
457463
// When the Entries Link Annotation is created, the arrow needs to follow the mouse.
458464
// Update the mouse coordinates while it is being created.
459465
#lastMouseOffsetX: number|null = null;
@@ -581,10 +587,14 @@ export class Overlays extends EventTarget {
581587
* the existing one, rather than create a new one. This ensures you can only
582588
* ever have one instance of the overlay type.
583589
*/
584-
const existing = this.overlaysOfType<T>(newOverlay.type);
585-
if (overlayIsSingleton(newOverlay) && existing[0]) {
586-
this.updateExisting(existing[0], newOverlay);
587-
return existing[0];
590+
if (overlayIsSingleton(newOverlay)) {
591+
const existing = this.#singletonOverlays.get(newOverlay.type);
592+
if (existing) {
593+
this.updateExisting(existing, newOverlay);
594+
return existing as T; // The is a safe cast, thanks to `type` above.
595+
}
596+
597+
this.#singletonOverlays.set(newOverlay.type, newOverlay);
588598
}
589599

590600
// By setting the value to null, we ensure that on the next render that the
@@ -644,6 +654,16 @@ export class Overlays extends EventTarget {
644654
* @returns the number of overlays that were removed.
645655
*/
646656
removeOverlaysOfType(type: TimelineOverlay['type']): number {
657+
if (overlayTypeIsSingleton(type)) {
658+
const singleton = this.#singletonOverlays.get(type);
659+
if (singleton) {
660+
this.remove(singleton);
661+
return 1;
662+
}
663+
664+
return 0;
665+
}
666+
647667
const overlaysToRemove = Array.from(this.#overlaysToElements.keys()).filter(overlay => {
648668
return overlay.type === type;
649669
});
@@ -657,6 +677,15 @@ export class Overlays extends EventTarget {
657677
* @returns all overlays that match the provided type.
658678
*/
659679
overlaysOfType<T extends TimelineOverlay>(type: T['type']): Array<NoInfer<T>> {
680+
if (overlayTypeIsSingleton(type)) {
681+
const singleton = this.#singletonOverlays.get(type);
682+
if (singleton) {
683+
return [singleton as T];
684+
}
685+
686+
return [];
687+
}
688+
660689
const matches: T[] = [];
661690

662691
function overlayIsOfType(overlay: TimelineOverlay): overlay is T {
@@ -688,6 +717,9 @@ export class Overlays extends EventTarget {
688717
this.#overlaysContainer.removeChild(htmlElement);
689718
}
690719
this.#overlaysToElements.delete(overlay);
720+
if (overlayIsSingleton(overlay)) {
721+
this.#singletonOverlays.delete(overlay.type);
722+
}
691723
}
692724

693725
/**

0 commit comments

Comments
 (0)