Fix unmounting performance regression in __DEV__ (#7463)
* Comment previous occurrences of this issue * Fix DEV performance regression in V8 * Extract try/catch into a separate function when calling hooks
Dan Abramov committed
Aug 10, 2016 at 20:14 UTC
afa27bc4d572b4855949e643d77f080b94528ab6
7 files changed
+108
-72
src/isomorphic/hooks/ReactComponentTreeHook.js
+48
-28
@@ -16,13 +16,33 @@ var ReactCurrentOwner = require('ReactCurrentOwner');
16
var invariant = require('invariant');
17
var warning = require('warning');
18
19
-var tree = {};
19
+var itemByKey = {};
20
var unmountedIDs = {};
21
var rootIDs = {};
22
23
-function updateTree(id, update) {
24
- if (!tree[id]) {
25
- tree[id] = {
23
+// Use non-numeric keys to prevent V8 performance issues:
24
+// https://github.com/facebook/react/pull/7232
25
+function getKeyFromID(id) {
26
+ return '.' + id;
27
+}
28
+function getIDFromKey(key) {
29
+ return parseInt(key.substr(1), 10);
30
+}
31
+
32
+function get(id) {
33
+ var key = getKeyFromID(id);
34
+ return itemByKey[key];
35
+}
36
+
37
+function remove(id) {
38
+ var key = getKeyFromID(id);
39
+ delete itemByKey[key];
40
+}
41
+
42
+function update(id, updater) {
43
+ var key = getKeyFromID(id);
44
+ if (!itemByKey[key]) {
45
+ itemByKey[key] = {
46
element: null,
47
parentID: null,
48
ownerID: null,
@@ -33,14 +53,14 @@ function updateTree(id, update) {
53
updateCount: 0,
54
};
55
}
36
- update(tree[id]);
56
+ updater(itemByKey[key]);
57
}
58
59
function purgeDeep(id) {
40
- var item = tree[id];
60
+ var item = get(id);
61
if (item) {
62
var {childIDs} = item;
43
- delete tree[id];
63
+ remove(id);
64
childIDs.forEach(purgeDeep);
65
}
66
}
@@ -75,15 +95,15 @@ function describeID(id) {
95
96
var ReactComponentTreeHook = {
97
onSetDisplayName(id, displayName) {
78
- updateTree(id, item => item.displayName = displayName);
98
+ update(id, item => item.displayName = displayName);
99
},
100
101
onSetChildren(id, nextChildIDs) {
82
- updateTree(id, item => {
102
+ update(id, item => {
103
item.childIDs = nextChildIDs;
104
105
nextChildIDs.forEach(nextChildID => {
86
- var nextChild = tree[nextChildID];
106
+ var nextChild = get(nextChildID);
107
invariant(
108
nextChild,
109
'Expected hook events to fire for the child ' +
@@ -123,27 +143,27 @@ var ReactComponentTreeHook = {
143
},
144
145
onSetOwner(id, ownerID) {
126
- updateTree(id, item => item.ownerID = ownerID);
146
+ update(id, item => item.ownerID = ownerID);
147
},
148
149
onSetParent(id, parentID) {
130
- updateTree(id, item => item.parentID = parentID);
150
+ update(id, item => item.parentID = parentID);
151
},
152
153
onSetText(id, text) {
134
- updateTree(id, item => item.text = text);
154
+ update(id, item => item.text = text);
155
},
156
157
onBeforeMountComponent(id, element) {
138
- updateTree(id, item => item.element = element);
158
+ update(id, item => item.element = element);
159
},
160
161
onBeforeUpdateComponent(id, element) {
142
- updateTree(id, item => item.element = element);
162
+ update(id, item => item.element = element);
163
},
164
165
onMountComponent(id) {
146
- updateTree(id, item => item.isMounted = true);
166
+ update(id, item => item.isMounted = true);
167
},
168
169
onMountRootComponent(id) {
@@ -151,11 +171,11 @@ var ReactComponentTreeHook = {
171
},
172
173
onUpdateComponent(id) {
154
- updateTree(id, item => item.updateCount++);
174
+ update(id, item => item.updateCount++);
175
},
176
177
onUnmountComponent(id) {
158
- updateTree(id, item => item.isMounted = false);
178
+ update(id, item => item.isMounted = false);
179
unmountedIDs[id] = true;
180
delete rootIDs[id];
181
},
@@ -173,7 +193,7 @@ var ReactComponentTreeHook = {
193
},
194
195
isMounted(id) {
176
- var item = tree[id];
196
+ var item = get(id);
197
return item ? item.isMounted : false;
198
},
199
@@ -209,44 +229,44 @@ var ReactComponentTreeHook = {
229
},
230
231
getChildIDs(id) {
212
- var item = tree[id];
232
+ var item = get(id);
233
return item ? item.childIDs : [];
234
},
235
236
getDisplayName(id) {
217
- var item = tree[id];
237
+ var item = get(id);
238
return item ? item.displayName : 'Unknown';
239
},
240
241
getElement(id) {
222
- var item = tree[id];
242
+ var item = get(id);
243
return item ? item.element : null;
244
},
245
246
getOwnerID(id) {
227
- var item = tree[id];
247
+ var item = get(id);
248
return item ? item.ownerID : null;
249
},
250
251
getParentID(id) {
232
- var item = tree[id];
252
+ var item = get(id);
253
return item ? item.parentID : null;
254
},
255
256
getSource(id) {
237
- var item = tree[id];
257
+ var item = get(id);
258
var element = item ? item.element : null;
259
var source = element != null ? element._source : null;
260
return source;
261
},
262
263
getText(id) {
244
- var item = tree[id];
264
+ var item = get(id);
265
return item ? item.text : null;
266
},
267
268
getUpdateCount(id) {
249
- var item = tree[id];
269
+ var item = get(id);
270
return item ? item.updateCount : 0;
271
},
272
@@ -255,7 +275,7 @@ var ReactComponentTreeHook = {
275
},
276
277
getRegisteredIDs() {
258
- return Object.keys(tree);
278
+ return Object.keys(itemByKey).map(getIDFromKey);
279
},
280
};
281
src/renderers/dom/client/eventPlugins/SimpleEventPlugin.js
+2
@@ -484,6 +484,8 @@ var ON_CLICK_KEY = keyOf({onClick: null});
484
var onClickListeners = {};
485
486
function getDictionaryKey(inst) {
487
+ // Prevents V8 performance issue:
488
+ // https://github.com/facebook/react/pull/7232
489
return '.' + inst._rootNodeID;
490
}
491
src/renderers/dom/shared/ReactDOMDebugTool.js
+27
-21
@@ -17,37 +17,43 @@ var ReactDebugTool = require('ReactDebugTool');
17
18
var warning = require('warning');
19
20
-var eventHandlers = [];
21
-var handlerDoesThrowForEvent = {};
20
+var hooks = [];
21
+var didHookThrowForEvent = {};
22
23
-function emitEvent(handlerFunctionName, arg1, arg2, arg3, arg4, arg5) {
24
- eventHandlers.forEach(function(handler) {
25
- try {
26
- if (handler[handlerFunctionName]) {
27
- handler[handlerFunctionName](arg1, arg2, arg3, arg4, arg5);
28
- }
29
- } catch (e) {
30
- warning(
31
- handlerDoesThrowForEvent[handlerFunctionName],
32
- 'exception thrown by hook while handling %s: %s',
33
- handlerFunctionName,
34
- e + '\n' + e.stack
35
- );
36
- handlerDoesThrowForEvent[handlerFunctionName] = true;
23
+function callHook(event, fn, context, arg1, arg2, arg3, arg4, arg5) {
24
+ try {
25
+ fn.call(context, arg1, arg2, arg3, arg4, arg5);
26
+ } catch (e) {
27
+ warning(
28
+ didHookThrowForEvent[event],
29
+ 'Exception thrown by hook while handling %s: %s',
30
+ event,
31
+ e + '\n' + e.stack
32
+ );
33
+ didHookThrowForEvent[event] = true;
34
+ }
35
+}
36
+
37
+function emitEvent(event, arg1, arg2, arg3, arg4, arg5) {
38
+ for (var i = 0; i < hooks.length; i++) {
39
+ var hook = hooks[i];
40
+ var fn = hook[event];
41
+ if (fn) {
42
+ callHook(event, fn, hook, arg1, arg2, arg3, arg4, arg5);
43
}
38
- });
44
+ }
45
}
46
47
var ReactDOMDebugTool = {
48
addHook(hook) {
49
ReactDebugTool.addHook(hook);
44
- eventHandlers.push(hook);
50
+ hooks.push(hook);
51
},
52
removeHook(hook) {
53
ReactDebugTool.removeHook(hook);
48
- for (var i = 0; i < eventHandlers.length; i++) {
49
- if (eventHandlers[i] === hook) {
50
- eventHandlers.splice(i, 1);
54
+ for (var i = 0; i < hooks.length; i++) {
55
+ if (hooks[i] === hook) {
56
+ hooks.splice(i, 1);
57
i--;
58
}
59
}
src/renderers/dom/shared/__tests__/ReactDOMDebugTool-test.js
+1
-1
@@ -65,7 +65,7 @@ describe('ReactDOMDebugTool', function() {
65
ReactDOMDebugTool.onTestEvent();
66
expect(console.error.calls.count()).toBe(1);
67
expect(console.error.calls.argsFor(0)[0]).toContain(
68
- 'exception thrown by hook while handling ' +
68
+ 'Exception thrown by hook while handling ' +
69
'onTestEvent: Error: Hi.'
70
);
71
src/renderers/shared/ReactDebugTool.js
+27
-21
@@ -20,25 +20,31 @@ var ExecutionEnvironment = require('ExecutionEnvironment');
20
var performanceNow = require('performanceNow');
21
var warning = require('warning');
22
23
-var eventHandlers = [];
24
-var handlerDoesThrowForEvent = {};
23
+var hooks = [];
24
+var didHookThrowForEvent = {};
25
26
-function emitEvent(handlerFunctionName, arg1, arg2, arg3, arg4, arg5) {
27
- eventHandlers.forEach(function(handler) {
28
- try {
29
- if (handler[handlerFunctionName]) {
30
- handler[handlerFunctionName](arg1, arg2, arg3, arg4, arg5);
31
- }
32
- } catch (e) {
33
- warning(
34
- handlerDoesThrowForEvent[handlerFunctionName],
35
- 'exception thrown by hook while handling %s: %s',
36
- handlerFunctionName,
37
- e + '\n' + e.stack
38
- );
39
- handlerDoesThrowForEvent[handlerFunctionName] = true;
26
+function callHook(event, fn, context, arg1, arg2, arg3, arg4, arg5) {
27
+ try {
28
+ fn.call(context, arg1, arg2, arg3, arg4, arg5);
29
+ } catch (e) {
30
+ warning(
31
+ didHookThrowForEvent[event],
32
+ 'Exception thrown by hook while handling %s: %s',
33
+ event,
34
+ e + '\n' + e.stack
35
+ );
36
+ didHookThrowForEvent[event] = true;
37
+ }
38
+}
39
+
40
+function emitEvent(event, arg1, arg2, arg3, arg4, arg5) {
41
+ for (var i = 0; i < hooks.length; i++) {
42
+ var hook = hooks[i];
43
+ var fn = hook[event];
44
+ if (fn) {
45
+ callHook(event, fn, hook, arg1, arg2, arg3, arg4, arg5);
46
}
41
- });
47
+ }
48
}
49
50
var isProfiling = false;
@@ -185,12 +191,12 @@ function resumeCurrentLifeCycleTimer() {
191
192
var ReactDebugTool = {
193
addHook(hook) {
188
- eventHandlers.push(hook);
194
+ hooks.push(hook);
195
},
196
removeHook(hook) {
191
- for (var i = 0; i < eventHandlers.length; i++) {
192
- if (eventHandlers[i] === hook) {
193
- eventHandlers.splice(i, 1);
197
+ for (var i = 0; i < hooks.length; i++) {
198
+ if (hooks[i] === hook) {
199
+ hooks.splice(i, 1);
200
i--;
201
}
202
}
src/renderers/shared/__tests__/ReactDebugTool-test.js
+1
-1
@@ -65,7 +65,7 @@ describe('ReactDebugTool', function() {
65
ReactDebugTool.onTestEvent();
66
expect(console.error.calls.count()).toBe(1);
67
expect(console.error.calls.argsFor(0)[0]).toContain(
68
- 'exception thrown by hook while handling ' +
68
+ 'Exception thrown by hook while handling ' +
69
'onTestEvent: Error: Hi.'
70
);
71
src/renderers/shared/stack/event/EventPluginHub.js
+2
@@ -54,6 +54,8 @@ var executeDispatchesAndReleaseTopLevel = function(e) {
54
};
55
56
var getDictionaryKey = function(inst) {
57
+ // Prevents V8 performance issue:
58
+ // https://github.com/facebook/react/pull/7232
59
return '.' + inst._rootNodeID;
60
};
61