@samitouri / QOS-React / commits / 09029acf00

Refactored owners list to use cached metadata (and added more tests)

Brian Vaughn committed Apr 25, 2019 at 12:16 UTC 09029acf009aeecdd10bbaeddfe3463568d23c0d
3 files changed +306 -34
src/__tests__/__snapshots__/storeOwners-test.js.snap
+103 -2
@@ -1,20 +1,121 @@
1 // Jest Snapshot v1, https://goo.gl/fbAQLP
2
3 +exports[`Store owners list should drill through interleaved intermediate components: 1: mount 1`] = `
4 +[root]
5 + ▾ <Root>
6 + ▾ <Intermediate key="intermediate">
7 + <Leaf key="leaf">
8 + ▾ <Wrapper key="wrapper">
9 + <Leaf>
10 + <Leaf key="leaf">
11 +`;
12 +
13 +exports[`Store owners list should drill through interleaved intermediate components: 2: components owned by <Root> 1`] = `
14 +" ▾ <Root>
15 + ▾ <Intermediate key=\\"intermediate\\">
16 + <Leaf>
17 + <Leaf key=\\"leaf\\">"
18 +`;
19 +
20 +exports[`Store owners list should drill through interleaved intermediate components: 3: components owned by <Intermediate> 1`] = `
21 +" ▾ <Intermediate key=\\"intermediate\\">
22 + <Leaf key=\\"leaf\\">
23 + ▾ <Wrapper key=\\"wrapper\\">"
24 +`;
25 +
26 exports[`Store owners list should drill through intermediate components: 1: mount 1`] = `
27 [root]
28 ▾ <Root>
29 ▾ <Intermediate>
30 ▾ <Wrapper>
8 - <Leaf key="children">
31 + <Leaf>
32 `;
33
34 exports[`Store owners list should drill through intermediate components: 2: components owned by <Root> 1`] = `
35 " ▾ <Root>
36 ▾ <Intermediate>
14 - <Leaf key=\\"children\\">"
37 + <Leaf>"
38 `;
39
40 exports[`Store owners list should drill through intermediate components: 3: components owned by <Intermediate> 1`] = `
41 " ▾ <Intermediate>
42 ▾ <Wrapper>"
43 `;
44 +
45 +exports[`Store owners list should show the proper owners list order and contents after insertions and deletions: 1: mount 1`] = `
46 +[root]
47 + ▾ <Root>
48 + ▾ <Intermediate>
49 + ▾ <Wrapper>
50 + <Leaf>
51 +`;
52 +
53 +exports[`Store owners list should show the proper owners list order and contents after insertions and deletions: 2: components owned by <Root> 1`] = `
54 +" ▾ <Root>
55 + ▾ <Intermediate>
56 + <Leaf>"
57 +`;
58 +
59 +exports[`Store owners list should show the proper owners list order and contents after insertions and deletions: 3: update to add direct 1`] = `
60 +[root]
61 + ▾ <Root>
62 + <Leaf>
63 + ▾ <Intermediate>
64 + ▾ <Wrapper>
65 + <Leaf>
66 +`;
67 +
68 +exports[`Store owners list should show the proper owners list order and contents after insertions and deletions: 4: components owned by <Root> 1`] = `
69 +" ▾ <Root>
70 + <Leaf>
71 + ▾ <Intermediate>
72 + <Leaf>"
73 +`;
74 +
75 +exports[`Store owners list should show the proper owners list order and contents after insertions and deletions: 5: update to remove indirect 1`] = `
76 +[root]
77 + ▾ <Root>
78 + <Leaf>
79 +`;
80 +
81 +exports[`Store owners list should show the proper owners list order and contents after insertions and deletions: 6: components owned by <Root> 1`] = `
82 +" ▾ <Root>
83 + <Leaf>"
84 +`;
85 +
86 +exports[`Store owners list should show the proper owners list order and contents after insertions and deletions: 7: update to remove both 1`] = `
87 +[root]
88 + <Root>
89 +`;
90 +
91 +exports[`Store owners list should show the proper owners list order and contents after insertions and deletions: 8: components owned by <Root> 1`] = `" <Root>"`;
92 +
93 +exports[`Store owners list should show the proper owners list ordering after reordered children: 1: mount (ascending) 1`] = `
94 +[root]
95 + ▾ <Root>
96 + <Leaf key="A">
97 + <Leaf key="B">
98 + <Leaf key="C">
99 +`;
100 +
101 +exports[`Store owners list should show the proper owners list ordering after reordered children: 2: components owned by <Root> 1`] = `
102 +" ▾ <Root>
103 + <Leaf key=\\"A\\">
104 + <Leaf key=\\"B\\">
105 + <Leaf key=\\"C\\">"
106 +`;
107 +
108 +exports[`Store owners list should show the proper owners list ordering after reordered children: 3: update (descending) 1`] = `
109 +[root]
110 + ▾ <Root>
111 + <Leaf key="C">
112 + <Leaf key="B">
113 + <Leaf key="A">
114 +`;
115 +
116 +exports[`Store owners list should show the proper owners list ordering after reordered children: 4: components owned by <Root> 1`] = `
117 +" ▾ <Root>
118 + <Leaf key=\\"C\\">
119 + <Leaf key=\\"B\\">
120 + <Leaf key=\\"A\\">"
121 +`;
src/__tests__/storeOwners-test.js
+122 -1
@@ -27,7 +27,9 @@ describe('Store owners list', () => {
27 it('should drill through intermediate components', () => {
28 const Root = () => (
29 <Intermediate>
30 - <Leaf key="children" />
30 + <div>
31 + <Leaf />
32 + </div>
33 </Intermediate>
34 );
35 const Wrapper = ({ children }) => children;
@@ -47,4 +49,123 @@ describe('Store owners list', () => {
49 printOwnersList(store.getOwnersListForElement(intermediateID))
50 ).toMatchSnapshot('3: components owned by <Intermediate>');
51 });
52 +
53 + it('should drill through interleaved intermediate components', () => {
54 + const Root = () => [
55 + <Intermediate key="intermediate">
56 + <Leaf />
57 + </Intermediate>,
58 + <Leaf key="leaf" />,
59 + ];
60 + const Wrapper = ({ children }) => children;
61 + const Leaf = () => <div>Leaf</div>;
62 + const Intermediate = ({ children }) => [
63 + <Leaf key="leaf" />,
64 + <Wrapper key="wrapper">{children}</Wrapper>,
65 + ];
66 +
67 + act(() => ReactDOM.render(<Root />, document.createElement('div')));
68 + expect(store).toMatchSnapshot('1: mount');
69 +
70 + const rootID = store.getElementIDAtIndex(0);
71 + expect(
72 + printOwnersList(store.getOwnersListForElement(rootID))
73 + ).toMatchSnapshot('2: components owned by <Root>');
74 +
75 + const intermediateID = store.getElementIDAtIndex(1);
76 + expect(
77 + printOwnersList(store.getOwnersListForElement(intermediateID))
78 + ).toMatchSnapshot('3: components owned by <Intermediate>');
79 + });
80 +
81 + it('should show the proper owners list order and contents after insertions and deletions', () => {
82 + const Root = ({ includeDirect, includeIndirect }) => (
83 + <div>
84 + {includeDirect ? <Leaf /> : null}
85 + {includeIndirect ? (
86 + <Intermediate>
87 + <Leaf />
88 + </Intermediate>
89 + ) : null}
90 + </div>
91 + );
92 + const Wrapper = ({ children }) => children;
93 + const Leaf = () => <div>Leaf</div>;
94 + const Intermediate = ({ children }) => <Wrapper>{children}</Wrapper>;
95 +
96 + const container = document.createElement('div');
97 +
98 + act(() =>
99 + ReactDOM.render(
100 + <Root includeDirect={false} includeIndirect={true} />,
101 + container
102 + )
103 + );
104 + expect(store).toMatchSnapshot('1: mount');
105 +
106 + const rootID = store.getElementIDAtIndex(0);
107 + expect(
108 + printOwnersList(store.getOwnersListForElement(rootID))
109 + ).toMatchSnapshot('2: components owned by <Root>');
110 +
111 + act(() =>
112 + ReactDOM.render(
113 + <Root includeDirect={true} includeIndirect={true} />,
114 + container
115 + )
116 + );
117 + expect(store).toMatchSnapshot('3: update to add direct');
118 +
119 + expect(
120 + printOwnersList(store.getOwnersListForElement(rootID))
121 + ).toMatchSnapshot('4: components owned by <Root>');
122 +
123 + act(() =>
124 + ReactDOM.render(
125 + <Root includeDirect={true} includeIndirect={false} />,
126 + container
127 + )
128 + );
129 + expect(store).toMatchSnapshot('5: update to remove indirect');
130 +
131 + expect(
132 + printOwnersList(store.getOwnersListForElement(rootID))
133 + ).toMatchSnapshot('6: components owned by <Root>');
134 +
135 + act(() =>
136 + ReactDOM.render(
137 + <Root includeDirect={false} includeIndirect={false} />,
138 + container
139 + )
140 + );
141 + expect(store).toMatchSnapshot('7: update to remove both');
142 +
143 + expect(
144 + printOwnersList(store.getOwnersListForElement(rootID))
145 + ).toMatchSnapshot('8: components owned by <Root>');
146 + });
147 +
148 + it('should show the proper owners list ordering after reordered children', () => {
149 + const Root = ({ ascending }) =>
150 + ascending
151 + ? [<Leaf key="A" />, <Leaf key="B" />, <Leaf key="C" />]
152 + : [<Leaf key="C" />, <Leaf key="B" />, <Leaf key="A" />];
153 + const Leaf = () => <div>Leaf</div>;
154 +
155 + const container = document.createElement('div');
156 + act(() => ReactDOM.render(<Root ascending={true} />, container));
157 + expect(store).toMatchSnapshot('1: mount (ascending)');
158 +
159 + const rootID = store.getElementIDAtIndex(0);
160 + expect(
161 + printOwnersList(store.getOwnersListForElement(rootID))
162 + ).toMatchSnapshot('2: components owned by <Root>');
163 +
164 + act(() => ReactDOM.render(<Root ascending={false} />, container));
165 + expect(store).toMatchSnapshot('3: update (descending)');
166 +
167 + expect(
168 + printOwnersList(store.getOwnersListForElement(rootID))
169 + ).toMatchSnapshot('4: components owned by <Root>');
170 + });
171 });
src/devtools/store.js
+81 -31
@@ -81,6 +81,10 @@ export default class Store extends EventEmitter {
81 // When profiling is in progress, operations are stored so that we can later reconstruct past commit trees.
82 _isProfiling: boolean = false;
83
84 + // Map of element (id) to the set of elements (ids) it owns.
85 + // This map enables getOwnersListForElement() to avoid traversing the entire tree.
86 + _ownersMap: Map<number, Set<number>> = new Map();
87 +
88 // Suspense cache for reading profiling data.
89 _profilingCache: ProfilingCache;
90
@@ -406,10 +410,65 @@ export default class Store extends EventEmitter {
410 return index;
411 }
412
409 - getOwnersListForElement(id: number): Array<Element> {
413 + getOwnersListForElement(ownerID: number): Array<Element> {
414 const list = [];
415 + let element = this._idToElement.get(ownerID);
416 + if (element != null) {
417 + list.push({
418 + ...element,
419 + depth: 0,
420 + });
421
412 - this._populateOwnersList(id, id, 0, list);
422 + const unsortedIDs = this._ownersMap.get(ownerID);
423 + if (unsortedIDs !== undefined) {
424 + const depthMap: Map<number, number> = new Map([[ownerID, 0]]);
425 +
426 + // Items in a set are ordered based on insertion.
427 + // This does not correlate with their order in the tree.
428 + // So first we need to order them.
429 + // I wish we could avoid this sorting operation; we could sort at insertion time,
430 + // but then we'd have to pay sorting costs even if the owners list was never used.
431 + // Seems better to defer the cost, since the set of ids is probably pretty small.
432 + const sortedIDs = Array.from(unsortedIDs).sort(
433 + (idA, idB) =>
434 + ((this.getIndexOfElementID(idA): any): number) -
435 + ((this.getIndexOfElementID(idB): any): number)
436 + );
437 +
438 + // Next we need to determine the appropriate depth for each element in the list.
439 + // The depth in the list may not correspond to the depth in the tree,
440 + // because the list has been filtered to remove intermediate components.
441 + // Perhaps the easiest way to do this is to walk up the tree until we reach either:
442 + // (1) another node that's already in the tree, or (2) the root (owner)
443 + // at which point, our depth is just the depth of that node plus one.
444 + sortedIDs.forEach(id => {
445 + const element = this._idToElement.get(id);
446 + if (element != null) {
447 + let parentID = element.parentID;
448 +
449 + let depth = 0;
450 + while (parentID > 0) {
451 + if (parentID === ownerID || unsortedIDs.has(parentID)) {
452 + depth = depthMap.get(parentID) + 1;
453 + depthMap.set(id, depth);
454 + break;
455 + }
456 + const parent = this._idToElement.get(parentID);
457 + if (parent == null) {
458 + break;
459 + }
460 + parentID = parent.parentID;
461 + }
462 +
463 + if (depth === 0) {
464 + throw Error('Invalid owners list');
465 + }
466 +
467 + list.push({ ...element, depth });
468 + }
469 + });
470 + }
471 + }
472
473 return list;
474 }
@@ -561,32 +620,6 @@ export default class Store extends EventEmitter {
620 THROTTLE_CAPTURE_SCREENSHOT_DURATION
621 );
622
564 - _populateOwnersList(
565 - id: number,
566 - ownerID: number,
567 - depth: number,
568 - list: Array<Element>
569 - ) {
570 - const element = this._idToElement.get(id);
571 - if (element != null) {
572 - const isInList = id === ownerID || element.ownerID === ownerID;
573 - if (isInList) {
574 - list.push({
575 - ...element,
576 - depth: depth,
577 - });
578 - }
579 - element.children.forEach(childID =>
580 - this._populateOwnersList(
581 - childID,
582 - ownerID,
583 - isInList ? depth + 1 : depth,
584 - list
585 - )
586 - );
587 - }
588 - }
589 -
623 _takeProfilingSnapshotRecursive = (id: number) => {
624 const element = this.getElementByID(id);
625 if (element !== null) {
@@ -779,6 +812,15 @@ export default class Store extends EventEmitter {
812 this._idToElement.set(id, element);
813 addedElementIDs.push(id);
814 this._adjustParentTreeWeight(parentElement, 1);
815 +
816 + if (ownerID > 0) {
817 + let set = this._ownersMap.get(ownerID);
818 + if (set === undefined) {
819 + set = new Set();
820 + this._ownersMap.set(ownerID, set);
821 + }
822 + set.add(id);
823 + }
824 }
825 break;
826 }
@@ -798,13 +840,13 @@ export default class Store extends EventEmitter {
840 i = i + 1;
841
842 const element = ((this._idToElement.get(id): any): Element);
801 - if (element.children.length > 0) {
843 + const { children, ownerID, parentID, weight } = element;
844 + if (children.length > 0) {
845 throw new Error(`Node ${id} was removed before its children.`);
846 }
847
848 this._idToElement.delete(id);
849
807 - const parentID = element.parentID;
850 let parentElement = null;
851 if (parentID === 0) {
852 if (__DEBUG__) {
@@ -830,8 +872,16 @@ export default class Store extends EventEmitter {
872 parentElement.children.splice(index, 1);
873 }
874
833 - this._adjustParentTreeWeight(parentElement, -element.weight);
875 + this._adjustParentTreeWeight(parentElement, -weight);
876 removedElementIDs.set(id, parentID);
877 +
878 + this._ownersMap.delete(id);
879 + if (ownerID > 0) {
880 + const set = this._ownersMap.get(ownerID);
881 + if (set !== undefined) {
882 + set.delete(id);
883 + }
884 + }
885 }
886 break;
887 }