Reverted optimization to avoid re-sending inspected fiber unless it committed
Brian Vaughn committed
Apr 23, 2019 at 18:04 UTC
7262a30cf38e6980e0b3f2245cc65f2b9510c071
3 files changed
+51
-35
src/devtools/cache.js
+11
-11
@@ -1,7 +1,6 @@
1
// @flow
2
3
import React, { createContext } from 'react';
4
-import LRU from 'lru-cache';
4
5
// Cache implementation was forked from the React repo:
6
// https://github.com/facebook/react/blob/master/packages/react-cache/src/ReactCache.js
@@ -68,18 +67,23 @@ function readContext(Context, observedBits) {
67
const CacheContext = createContext(null);
68
69
type Config = {
71
- useLRU?: boolean,
70
+ useWeakMap?: boolean,
71
};
72
74
-const entries: Map<Resource<any, any, any>, Map<any, any> | LRU> = new Map();
73
+const entries: Map<
74
+ Resource<any, any, any>,
75
+ Map<any, any> | WeakMap<any, any>
76
+> = new Map();
77
const resourceConfigs: Map<Resource<any, any, any>, Config> = new Map();
78
77
-function getEntriesForResource(resource: any): Map<any, any> | LRU {
79
+function getEntriesForResource(
80
+ resource: any
81
+): Map<any, any> | WeakMap<any, any> {
82
let entriesForResource = ((entries.get(resource): any): Map<any, any>);
83
if (entriesForResource === undefined) {
84
const config = resourceConfigs.get(resource);
85
entriesForResource =
82
- config !== undefined && config.useLRU ? new LRU({ max: 10 }) : new Map();
86
+ config !== undefined && config.useWeakMap ? new WeakMap() : new Map();
87
entries.set(resource, entriesForResource);
88
}
89
return entriesForResource;
@@ -122,7 +126,7 @@ function accessResult<Input, Key, Value>(
126
}
127
}
128
125
-export function createResource<Input, Key: string | number, Value>(
129
+export function createResource<Input, Key, Value>(
130
fetch: Input => Thenable<Value>,
131
hashInput: Input => Key,
132
config?: Config = {}
@@ -134,11 +138,7 @@ export function createResource<Input, Key: string | number, Value>(
138
139
invalidate(key: Key): void {
140
const entriesForResource = getEntriesForResource(resource);
137
- if (entriesForResource instanceof Map) {
138
- entriesForResource.delete(key);
139
- } else {
140
- entriesForResource.set(key, undefined);
141
- }
141
+ entriesForResource.delete(key);
142
},
143
144
read(input: Input): Value {
src/devtools/store.js
+3
-2
@@ -68,8 +68,9 @@ export default class Store extends EventEmitter {
68
// At least one of the injected renderers contains (DEV only) owner metadata.
69
_hasOwnerMetadata: boolean = false;
70
71
- // Map of ID to Element.
72
- // Elements are mutable (for now) to avoid excessive cloning during tree updates.
71
+ // Map of ID to (mutable) Element.
72
+ // Elements are mutated to avoid excessive cloning during tree updates.
73
+ // The InspectedElementContext also relies on this mutability for its WeakMap usage.
74
_idToElement: Map<number, Element> = new Map();
75
76
// The user has imported a previously exported profiling session.
src/devtools/views/Components/InspectedElementContext.js
+37
-22
@@ -2,6 +2,7 @@
2
3
import React, {
4
createContext,
5
+ useCallback,
6
useContext,
7
useEffect,
8
useMemo,
@@ -14,6 +15,7 @@ import { TreeStateContext } from './TreeContext';
15
16
import type {
17
DehydratedData,
18
+ Element,
19
InspectedElement,
20
} from 'src/devtools/views/Components/types';
21
import type { Resource, Thenable } from '../../cache';
@@ -31,10 +33,10 @@ type InProgressRequest = {|
33
resolveFn: ResolveFn,
34
|};
35
34
-const inProgressRequests: Map<number, InProgressRequest> = new Map();
35
-const resource: Resource<number, number, InspectedElement> = createResource(
36
- (id: number) => {
37
- let request = inProgressRequests.get(id);
36
+const inProgressRequests: WeakMap<Element, InProgressRequest> = new WeakMap();
37
+const resource: Resource<Element, Element, InspectedElement> = createResource(
38
+ (element: Element) => {
39
+ let request = inProgressRequests.get(element);
40
if (request != null) {
41
return request.promise;
42
}
@@ -44,12 +46,12 @@ const resource: Resource<number, number, InspectedElement> = createResource(
46
resolveFn = resolve;
47
});
48
47
- inProgressRequests.set(id, { promise, resolveFn });
49
+ inProgressRequests.set(element, { promise, resolveFn });
50
51
return promise;
52
},
51
- (id: number) => id,
52
- { useLRU: true }
53
+ (element: Element) => element,
54
+ { useWeakMap: true }
55
);
56
57
type Props = {|
@@ -60,6 +62,18 @@ function InspectedElementContextController({ children }: Props) {
62
const bridge = useContext(BridgeContext);
63
const store = useContext(StoreContext);
64
65
+ const read = useCallback(
66
+ (id: number) => {
67
+ const element = store.getElementByID(id);
68
+ if (element !== null) {
69
+ return resource.read(element);
70
+ } else {
71
+ return null;
72
+ }
73
+ },
74
+ [store]
75
+ );
76
+
77
// It's very important that this context consumes selectedElementID and not inspectedElementID.
78
// Otherwise the effect that sends the "inspect" message across the bridge-
79
// would itself be blocked by the same render that suspends (waiting for the data).
@@ -81,16 +95,19 @@ function InspectedElementContextController({ children }: Props) {
95
state: hydrateHelper(inspectedElement.state),
96
}: any): InspectedElement);
97
84
- const request = inProgressRequests.get(id);
85
- if (request != null) {
86
- inProgressRequests.delete(id);
87
- request.resolveFn(inspectedElement);
88
- } else {
89
- resource.write(id, inspectedElement);
90
-
91
- // Schedule update with React if the curently-selected element has been invalidated.
92
- if (id === selectedElementID) {
93
- setCount(count => count + 1);
98
+ const element = store.getElementByID(id);
99
+ if (element !== null) {
100
+ const request = inProgressRequests.get(element);
101
+ if (request != null) {
102
+ inProgressRequests.delete(element);
103
+ request.resolveFn(inspectedElement);
104
+ } else {
105
+ resource.write(element, inspectedElement);
106
+
107
+ // Schedule update with React if the curently-selected element has been invalidated.
108
+ if (id === selectedElementID) {
109
+ setCount(count => count + 1);
110
+ }
111
}
112
}
113
}
@@ -98,7 +115,7 @@ function InspectedElementContextController({ children }: Props) {
115
116
bridge.addListener('inspectedElement', onInspectedElement);
117
return () => bridge.removeListener('inspectedElement', onInspectedElement);
101
- }, [bridge, selectedElementID]);
118
+ }, [bridge, selectedElementID, store]);
119
120
// This effect handler polls for updates on the currently selected element.
121
useEffect(() => {
@@ -145,12 +162,10 @@ function InspectedElementContextController({ children }: Props) {
162
}, [bridge, selectedElementID, store]);
163
164
const value = useMemo(
148
- () => ({
149
- read: resource.read,
150
- }),
165
+ () => ({ read }),
166
// Count is used to invalidate the cache and schedule an update with React.
167
// eslint-disable-next-line react-hooks/exhaustive-deps
153
- [count]
168
+ [count, read]
169
);
170
171
return (