Skip to content

Commit feeee2e

Browse files
szuendDevtools-frontend LUCI CQ
authored andcommitted
[sources] Remove constant re-creation of 'children' array
The NavigatorTreeNode in the 'Sources' panel stashes the children in a Map internally but hands out an array for it's 'children' accessor. The implementation spreads the Map#values into an array. When debugging web apps with thousands of files that are deeply nested, this becomes noticeable and a performance trace where we spend 25 seconds executing JS in DevTools, we spend ~500ms just spreading child NavigatorTreeNodes. This CL fixes this by maintaining both a map and an array for a tree nodes' children. We also extract a new helper method 'updateId' so we can use private fields for both the map and the array. [email protected] Bug: None Change-Id: I42d1a7f6934dc05babbb055aaf9d19780004c759 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6190894 Commit-Queue: Simon Zünd <[email protected]> Reviewed-by: Philip Pfaffe <[email protected]>
1 parent bfbcc4c commit feeee2e

File tree

1 file changed

+26
-13
lines changed

1 file changed

+26
-13
lines changed

front_end/panels/sources/NavigatorView.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,7 @@ export class NavigatorView extends UI.Widget.VBox implements SDK.TargetManager.O
10241024
}
10251025

10261026
private async removeUISourceCodeFromProject(node: NavigatorTreeNode): Promise<void> {
1027-
node.children().forEach(async child => {
1027+
node.children().slice(0).forEach(async child => {
10281028
await this.removeUISourceCodeFromProject(child);
10291029
});
10301030

@@ -1526,19 +1526,20 @@ export class NavigatorTreeNode {
15261526
id: string;
15271527
protected navigatorView: NavigatorView;
15281528
type: string;
1529-
childrenInternal: Map<string, NavigatorTreeNode>;
15301529
private populated: boolean;
15311530
isMerged: boolean;
15321531
parent!: NavigatorTreeNode|null;
15331532
title!: string;
15341533
tooltip?: string;
15351534
recursiveProperties: NavigatorRecursiveTreeNodeProperties;
15361535

1536+
#children: NavigatorTreeNode[] = [];
1537+
readonly #childById = new Map<string, NavigatorTreeNode>();
1538+
15371539
constructor(navigatorView: NavigatorView, id: string, type: string, tooltip?: string) {
15381540
this.id = id;
15391541
this.navigatorView = navigatorView;
15401542
this.type = type;
1541-
this.childrenInternal = new Map();
15421543
this.tooltip = tooltip;
15431544

15441545
this.populated = false;
@@ -1625,32 +1626,46 @@ export class NavigatorTreeNode {
16251626
}
16261627

16271628
isEmpty(): boolean {
1628-
return !this.childrenInternal.size;
1629+
return !this.#children.length;
16291630
}
16301631

1631-
children(): NavigatorTreeNode[] {
1632-
return [...this.childrenInternal.values()];
1632+
children(): readonly NavigatorTreeNode[] {
1633+
return this.#children;
16331634
}
16341635

16351636
child(id: string): NavigatorTreeNode|null {
1636-
return this.childrenInternal.get(id) || null;
1637+
return this.#childById.get(id) ?? null;
16371638
}
16381639

16391640
appendChild(node: NavigatorTreeNode): void {
1640-
this.childrenInternal.set(node.id, node);
1641+
this.#children.push(node);
1642+
this.#childById.set(node.id, node);
16411643
node.parent = this;
16421644
this.didAddChild(node);
16431645
}
16441646

16451647
removeChild(node: NavigatorTreeNode): void {
16461648
this.willRemoveChild(node);
1647-
this.childrenInternal.delete(node.id);
1649+
const idx = this.#children.findIndex(n => n.id === node.id);
1650+
if (idx >= 0) {
1651+
this.#children.splice(idx, 1);
1652+
}
1653+
this.#childById.delete(node.id);
16481654
node.parent = null;
16491655
node.dispose();
16501656
}
16511657

16521658
reset(): void {
1653-
this.childrenInternal.clear();
1659+
this.#children = [];
1660+
this.#childById.clear();
1661+
}
1662+
1663+
updateId(newId: string): void {
1664+
if (this.parent) {
1665+
this.parent.#childById.delete(this.id);
1666+
this.parent.#childById.set(newId, this);
1667+
}
1668+
this.id = newId;
16541669
}
16551670
}
16561671

@@ -1748,9 +1763,7 @@ export class NavigatorUISourceCodeTreeNode extends NavigatorTreeNode {
17481763
this.treeElement.tooltip = tooltip;
17491764
this.treeElement.updateAccessibleName();
17501765

1751-
this.parent?.childrenInternal.delete(this.id);
1752-
this.id = 'UISourceCode:' + this.uiSourceCodeInternal.canononicalScriptId();
1753-
this.parent?.childrenInternal.set(this.id, this);
1766+
this.updateId('UISourceCode:' + this.uiSourceCodeInternal.canononicalScriptId());
17541767
}
17551768

17561769
override hasChildren(): boolean {

0 commit comments

Comments
 (0)