Skip to content

Commit 9238da7

Browse files
yurydelendikcodehag
authored andcommitted
Optimize addToTree function. (firefox-devtools#4263)
1 parent 9667292 commit 9238da7

9 files changed

Lines changed: 215 additions & 48 deletions

File tree

src/components/PrimaryPanes/SourcesTree.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import {
1919
createParentMap,
2020
isDirectory,
2121
addToTree,
22-
sortEntireTree,
2322
collapseTree,
2423
createTree,
2524
getDirectories
@@ -145,8 +144,7 @@ class SourcesTree extends Component {
145144
for (const source of newSet) {
146145
addToTree(uncollapsedTree, source, this.props.debuggeeUrl);
147146
}
148-
const unsortedTree = collapseTree(uncollapsedTree);
149-
sourceTree = sortEntireTree(unsortedTree, nextProps.debuggeeUrl);
147+
sourceTree = collapseTree(uncollapsedTree);
150148
}
151149

152150
this.setState({

src/utils/sources-tree/addToTree.js

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,29 @@ import {
77
partIsFile,
88
createNode
99
} from "./utils";
10+
import {
11+
createTreeNodeMatcher,
12+
findNodeInContents,
13+
getDomain
14+
} from "./treeOrder";
1015
import { getURL, getFilenameFromPath } from "./getURL";
1116

1217
import type { Node } from "./types";
1318
import type { SourceRecord } from "../../reducers/types";
1419

15-
function createNodeInTree(part: string, path: string, tree: Node) {
20+
function createNodeInTree(
21+
part: string,
22+
path: string,
23+
tree: Node,
24+
index: number
25+
) {
1626
const node = createNode(part, path, []);
27+
1728
// we are modifying the tree
18-
tree.contents = [...tree.contents, node];
29+
const contents = tree.contents.slice(0);
30+
contents.splice(index, 0, node);
31+
tree.contents = contents;
32+
1933
return node;
2034
}
2135

@@ -31,23 +45,28 @@ function findOrCreateNode(
3145
path: string,
3246
part: string,
3347
index: number,
34-
url: Object
48+
url: Object,
49+
debuggeeHost: ?string
3550
) {
36-
const child = subTree.contents.find(c => c.name === part);
51+
const addedPartIsFile = partIsFile(index, parts, url);
52+
const { found: childFound, index: childIndex } = findNodeInContents(
53+
subTree,
54+
createTreeNodeMatcher(part, !addedPartIsFile, debuggeeHost)
55+
);
3756

3857
// we create and enter the new node
39-
if (!child) {
40-
return createNodeInTree(part, path, subTree);
58+
if (!childFound) {
59+
return createNodeInTree(part, path, subTree, childIndex);
4160
}
4261

4362
// we found a path with the same name as the part. We need to determine
4463
// if this is the correct child, or if we have a naming conflict
45-
const addedPartIsFile = partIsFile(index, parts, url);
64+
const child = subTree.contents[childIndex];
4665
const childIsFile = !nodeHasChildren(child);
4766

4867
// if we have a naming conflict, we'll create a new node
4968
if ((childIsFile && !addedPartIsFile) || (!childIsFile && addedPartIsFile)) {
50-
return createNodeInTree(part, path, subTree);
69+
return createNodeInTree(part, path, subTree, childIndex);
5170
}
5271

5372
// if there is no naming conflict, we can traverse into the child
@@ -58,7 +77,7 @@ function findOrCreateNode(
5877
* walk the source tree to the final node for a given url,
5978
* adding new nodes along the way
6079
*/
61-
function traverseTree(url: Object, tree: Node) {
80+
function traverseTree(url: Object, tree: Node, debuggeeHost: ?string) {
6281
url.path = decodeURIComponent(url.path);
6382

6483
const parts = url.path.split("/").filter(p => p !== "");
@@ -67,7 +86,16 @@ function traverseTree(url: Object, tree: Node) {
6786
let path = "";
6887
return parts.reduce((subTree, part, index) => {
6988
path = `${path}/${part}`;
70-
return findOrCreateNode(parts, subTree, path, part, index, url);
89+
const debuggeeHostIfRoot = index === 0 ? debuggeeHost : null;
90+
return findOrCreateNode(
91+
parts,
92+
subTree,
93+
path,
94+
part,
95+
index,
96+
url,
97+
debuggeeHostIfRoot
98+
);
7199
}, tree);
72100
}
73101

@@ -84,18 +112,24 @@ function addSourceToNode(node: Node, url: Object, source: SourceRecord) {
84112
}
85113

86114
const name = getFilenameFromPath(url.path);
87-
const existingNode = node.contents.find(childNode => childNode.name === name);
115+
const { found: childFound, index: childIndex } = findNodeInContents(
116+
node,
117+
createTreeNodeMatcher(name, false, null)
118+
);
88119

89120
// if we are readding an existing file in the node, overwrite the existing
90121
// file and return the node's contents
91-
if (existingNode) {
122+
if (childFound) {
123+
const existingNode = node.contents[childIndex];
92124
existingNode.contents = source;
93125
return node.contents;
94126
}
95127

96128
// if this is a new file, add the new file;
97129
const newNode = createNode(name, source.get("url"), source);
98-
return [...node.contents, newNode];
130+
const contents = node.contents.slice(0);
131+
contents.splice(childIndex, 0, newNode);
132+
return contents;
99133
}
100134

101135
/**
@@ -108,11 +142,12 @@ export function addToTree(
108142
debuggeeUrl: string
109143
) {
110144
const url = getURL(source.get("url"), debuggeeUrl);
145+
const debuggeeHost = getDomain(debuggeeUrl);
111146

112147
if (isInvalidUrl(url, source)) {
113148
return;
114149
}
115150

116-
const finalNode = traverseTree(url, tree);
151+
const finalNode = traverseTree(url, tree, debuggeeHost);
117152
finalNode.contents = addSourceToNode(finalNode, url, source);
118153
}

src/utils/sources-tree/createTree.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import { createNode, createParentMap } from "./utils";
44
import { collapseTree } from "./collapseTree";
5-
import { sortEntireTree } from "./sortTree";
65
import { addToTree } from "./addToTree";
76

87
import type { SourcesMap } from "../../reducers/types";
@@ -13,7 +12,7 @@ export function createTree(sources: SourcesMap, debuggeeUrl: string) {
1312
addToTree(uncollapsedTree, source, debuggeeUrl);
1413
}
1514

16-
const sourceTree = sortEntireTree(collapseTree(uncollapsedTree), debuggeeUrl);
15+
const sourceTree = collapseTree(uncollapsedTree);
1716

1817
return {
1918
uncollapsedTree,

src/utils/sources-tree/tests/__snapshots__/addToTree.spec.js.snap

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,21 +45,21 @@ exports[`sources-tree addToTree excludes javascript: URLs from the tree 1`] = `
4545
exports[`sources-tree addToTree replaces a file with a directory 1`] = `
4646
" - root path=
4747
- unpkg.com path=/unpkg.com
48-
- codemirror@5.1 path=/unpkg.com/codemirror@5.1 source_id=server1.conn13.child1/37
4948
- codemirror@5.1 path=/unpkg.com/codemirror@5.1
5049
- mode path=/unpkg.com/codemirror@5.1/mode
5150
- xml path=/unpkg.com/codemirror@5.1/mode/xml
5251
- xml.js path=/unpkg.com/codemirror@5.1/mode/xml/xml.js source_id=server1.conn13.child1/39
52+
- codemirror@5.1 path=/unpkg.com/codemirror@5.1 source_id=server1.conn13.child1/37
5353
"
5454
`;
5555

5656
exports[`sources-tree addToTree uses debuggeeUrl as default 1`] = `
5757
" - root path=
5858
- localhost:4242 path=/localhost:4242
5959
- components path=/localhost:4242/components
60-
- TodoTextInput.js path=/localhost:4242/components/TodoTextInput.js source_id=undefined
6160
- Header.js path=/localhost:4242/components/Header.js source_id=undefined
6261
- TodoItem.js path=/localhost:4242/components/TodoItem.js source_id=undefined
62+
- TodoTextInput.js path=/localhost:4242/components/TodoTextInput.js source_id=undefined
6363
- reducers path=/localhost:4242/reducers
6464
- index.js path=/localhost:4242/reducers/index.js source_id=undefined
6565
- index.js path=/localhost:4242/index.js source_id=undefined

src/utils/sources-tree/tests/addToTree.spec.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe("sources-tree", () => {
5151
});
5252
const tree = createNode("root", "", []);
5353

54-
addToTree(tree, source1);
54+
addToTree(tree, source1, "http://example.com/");
5555
expect(tree.contents.length).toBe(1);
5656

5757
const base = tree.contents[0];
@@ -121,9 +121,9 @@ describe("sources-tree", () => {
121121
});
122122
const tree = createNode("root", "", []);
123123

124-
addToTree(tree, source1);
125-
addToTree(tree, source2);
126-
addToTree(tree, source3);
124+
addToTree(tree, source1, "http://example.com/");
125+
addToTree(tree, source2, "http://example.com/");
126+
addToTree(tree, source3, "http://example.com/");
127127

128128
const base = tree.contents[0];
129129
expect(tree.contents.length).toBe(1);
@@ -140,7 +140,7 @@ describe("sources-tree", () => {
140140
});
141141
const tree = createNode("root", "", []);
142142

143-
addToTree(tree, source);
143+
addToTree(tree, source, "file:///a/index.html");
144144
expect(tree.contents.length).toBe(1);
145145

146146
const base = tree.contents[0];
@@ -170,7 +170,7 @@ describe("sources-tree", () => {
170170

171171
const sources = createSourcesList(testData);
172172
const tree = createNode("root", "", []);
173-
sources.forEach(source => addToTree(tree, source));
173+
sources.forEach(source => addToTree(tree, source, "https://unpkg.com/"));
174174
expect(formatTree(tree)).toMatchSnapshot();
175175
});
176176

@@ -189,7 +189,7 @@ describe("sources-tree", () => {
189189

190190
const sources = createSourcesList(testData);
191191
const tree = createNode("root", "", []);
192-
sources.forEach(source => addToTree(tree, source));
192+
sources.forEach(source => addToTree(tree, source, "https://unpkg.com/"));
193193
expect(formatTree(tree)).toMatchSnapshot();
194194
});
195195

src/utils/sources-tree/tests/collapseTree.spec.js

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe("sources tree", () => {
2525
describe("collapseTree", () => {
2626
it("can collapse a single source", () => {
2727
const fullTree = createNode("root", "", []);
28-
addToTree(fullTree, abcSource);
28+
addToTree(fullTree, abcSource, "http://example.com/");
2929
expect(fullTree.contents.length).toBe(1);
3030
const tree = collapseTree(fullTree);
3131

@@ -45,8 +45,8 @@ describe("sources tree", () => {
4545

4646
it("correctly merges in a collapsed source with a deeper level", () => {
4747
const fullTree = createNode("root", "", []);
48-
addToTree(fullTree, abcSource);
49-
addToTree(fullTree, abcdeSource);
48+
addToTree(fullTree, abcSource, "http://example.com/");
49+
addToTree(fullTree, abcdeSource, "http://example.com/");
5050
const tree = collapseTree(fullTree);
5151

5252
sortEntireTree(tree);
@@ -73,8 +73,8 @@ describe("sources tree", () => {
7373

7474
it("correctly merges in a collapsed source with a shallower level", () => {
7575
const fullTree = createNode("root", "", []);
76-
addToTree(fullTree, abcSource);
77-
addToTree(fullTree, abxSource);
76+
addToTree(fullTree, abcSource, "http://example.com/");
77+
addToTree(fullTree, abxSource, "http://example.com/");
7878
const tree = collapseTree(fullTree);
7979

8080
expect(tree.contents.length).toBe(1);
@@ -97,8 +97,8 @@ describe("sources tree", () => {
9797

9898
it("correctly merges in a collapsed source with the same level", () => {
9999
const fullTree = createNode("root", "", []);
100-
addToTree(fullTree, abcdeSource);
101-
addToTree(fullTree, abcSource);
100+
addToTree(fullTree, abcdeSource, "http://example.com/");
101+
addToTree(fullTree, abcSource, "http://example.com/");
102102
const tree = collapseTree(fullTree);
103103

104104
expect(tree.contents.length).toBe(1);

src/utils/sources-tree/tests/sortTree.spec.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ describe("sources-tree", () => {
2121
});
2222
const _tree = createNode("root", "", []);
2323

24-
addToTree(_tree, source1);
25-
addToTree(_tree, source2);
26-
addToTree(_tree, source3);
24+
addToTree(_tree, source1, "http://example.com/");
25+
addToTree(_tree, source2, "http://example.com/");
26+
addToTree(_tree, source3, "http://example.com/");
2727
const tree = sortEntireTree(_tree);
2828

2929
const base = tree.contents[0];
@@ -71,7 +71,9 @@ describe("sources-tree", () => {
7171
];
7272

7373
const _tree = createNode("root", "", []);
74-
sources.forEach(source => addToTree(_tree, source));
74+
sources.forEach(source =>
75+
addToTree(_tree, source, "http://example.com/")
76+
);
7577
const tree = sortEntireTree(_tree);
7678
const domain = tree.contents[0];
7779

@@ -119,7 +121,9 @@ describe("sources-tree", () => {
119121
];
120122

121123
const _tree = createNode("root", "", []);
122-
sources.forEach(source => addToTree(_tree, source));
124+
sources.forEach(source =>
125+
addToTree(_tree, source, "http://example.com/")
126+
);
123127
const tree = sortEntireTree(_tree);
124128
const [
125129
bFolderNode,

src/utils/sources-tree/tests/utils.spec.js

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe("sources tree", () => {
5050
];
5151

5252
const tree = createNode("root", "", []);
53-
sources.forEach(source => addToTree(tree, source));
53+
sources.forEach(source => addToTree(tree, source, "http://example.com/"));
5454
sortEntireTree(tree);
5555
const [bFolderNode, aFileNode] = tree.contents[0].contents;
5656
const [cFolderNode] = bFolderNode.contents;
@@ -101,9 +101,9 @@ describe("sources tree", () => {
101101
});
102102

103103
const tree = createNode("root", "", []);
104-
addToTree(tree, source1);
105-
addToTree(tree, source2);
106-
addToTree(tree, source3);
104+
addToTree(tree, source1, "http://a/");
105+
addToTree(tree, source2, "http://a/");
106+
addToTree(tree, source3, "http://a/");
107107
const paths = getDirectories("http://a/b.js", tree);
108108

109109
expect(paths[1].path).toBe("/a");
@@ -122,8 +122,8 @@ describe("sources tree", () => {
122122
});
123123

124124
const tree = createNode("root", "", []);
125-
addToTree(tree, source1);
126-
addToTree(tree, source2);
125+
addToTree(tree, source1, "http://a/");
126+
addToTree(tree, source2, "http://a/");
127127
const paths = getDirectories("http://a/b.js?key=hi", tree);
128128

129129
expect(paths[1].path).toBe("/a");
@@ -142,8 +142,8 @@ describe("sources tree", () => {
142142
});
143143

144144
const tree = createNode("root", "", []);
145-
addToTree(tree, source1);
146-
addToTree(tree, source2);
145+
addToTree(tree, source1, "http://a/");
146+
addToTree(tree, source2, "http://a/");
147147
const paths = getDirectories("https://a/b.js", tree);
148148

149149
expect(paths[1].path).toBe("/a");

0 commit comments

Comments
 (0)