Make ReactPerf.start() work during reconciliation (#7208)
* Add failing test demonstrating a ReactPerf warning * Make the failing ReactPerf test more precise * Make ReactPerf.start() work during reconciliation * Reorder lifecycle methods for greater clarity * Fix memory leak * Error boundaries should not break ReactPerf * Put onBeginFlush/onEndFlush into transaction wrappers This looks cleaner even though it is not strictly necessary. We still call them manually for unmounting because it doesn't have a transaction.
Dan Abramov committed
Jul 7, 2016 at 19:41 UTC
1a0e3a32150468223d6f9fd0125db0f8503b76d6
12 files changed
+100
-38
src/renderers/dom/client/ReactMount.js
-5
@@ -315,10 +315,6 @@ var ReactMount = {
315
shouldReuseMarkup,
316
context
317
) {
318
- if (__DEV__) {
319
- ReactInstrumentation.debugTool.onBeginFlush();
320
- }
321
-
318
// Various parts of our code (such as ReactCompositeComponent's
319
// _renderValidatedComponent) assume that calls to render aren't nested;
320
// verify that that's the case.
@@ -364,7 +360,6 @@ var ReactMount = {
360
ReactInstrumentation.debugTool.onMountRootComponent(
361
componentInstance._renderedComponent._debugID
362
);
367
- ReactInstrumentation.debugTool.onEndFlush();
363
}
364
365
return componentInstance;
src/renderers/dom/client/ReactReconcileTransaction.js
+8
@@ -15,6 +15,7 @@ var CallbackQueue = require('CallbackQueue');
15
var PooledClass = require('PooledClass');
16
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
17
var ReactInputSelection = require('ReactInputSelection');
18
+var ReactInstrumentation = require('ReactInstrumentation');
19
var Transaction = require('Transaction');
20
var ReactUpdateQueue = require('ReactUpdateQueue');
21
@@ -91,6 +92,13 @@ var TRANSACTION_WRAPPERS = [
92
ON_DOM_READY_QUEUEING,
93
];
94
95
+if (__DEV__) {
96
+ TRANSACTION_WRAPPERS.push({
97
+ initialize: ReactInstrumentation.debugTool.onBeginFlush,
98
+ close: ReactInstrumentation.debugTool.onEndFlush,
99
+ });
100
+}
101
+
102
/**
103
* Currently:
104
* - The order that these are listed in the transaction is critical:
src/renderers/dom/server/ReactServerRendering.js
-4
@@ -37,9 +37,6 @@ function renderToStringImpl(element, makeStaticMarkup) {
37
transaction = ReactServerRenderingTransaction.getPooled(makeStaticMarkup);
38
39
return transaction.perform(function() {
40
- if (__DEV__) {
41
- ReactInstrumentation.debugTool.onBeginFlush();
42
- }
40
var componentInstance = instantiateReactComponent(element, true);
41
var markup = ReactReconciler.mountComponent(
42
componentInstance,
@@ -52,7 +49,6 @@ function renderToStringImpl(element, makeStaticMarkup) {
49
ReactInstrumentation.debugTool.onUnmountComponent(
50
componentInstance._debugID
51
);
55
- ReactInstrumentation.debugTool.onEndFlush();
52
}
53
if (!makeStaticMarkup) {
54
markup = ReactMarkupChecksum.addChecksumToMarkup(markup);
src/renderers/dom/server/ReactServerRenderingTransaction.js
+8
@@ -13,6 +13,7 @@
13
14
var PooledClass = require('PooledClass');
15
var Transaction = require('Transaction');
16
+var ReactInstrumentation = require('ReactInstrumentation');
17
var ReactServerUpdateQueue = require('ReactServerUpdateQueue');
18
19
@@ -23,6 +24,13 @@ var ReactServerUpdateQueue = require('ReactServerUpdateQueue');
24
*/
25
var TRANSACTION_WRAPPERS = [];
26
27
+if (__DEV__) {
28
+ TRANSACTION_WRAPPERS.push({
29
+ initialize: ReactInstrumentation.debugTool.onBeginFlush,
30
+ close: ReactInstrumentation.debugTool.onEndFlush,
31
+ });
32
+}
33
+
34
var noopCallbackQueue = {
35
enqueue: function() {},
36
};
src/renderers/native/ReactNativeMount.js
-5
@@ -138,10 +138,6 @@ var ReactNativeMount = {
138
var instance = instantiateReactComponent(nextWrappedElement, false);
139
ReactNativeMount._instancesByContainerID[containerTag] = instance;
140
141
- if (__DEV__) {
142
- ReactInstrumentation.debugTool.onBeginFlush();
143
- }
144
-
141
// The initial render is synchronous but any updates that happen during
142
// rendering, in componentWillMount or componentDidMount, will be batched
143
// according to the current batching strategy.
@@ -156,7 +152,6 @@ var ReactNativeMount = {
152
ReactInstrumentation.debugTool.onMountRootComponent(
153
instance._renderedComponent._debugID
154
);
159
- ReactInstrumentation.debugTool.onEndFlush();
155
}
156
var component = instance.getPublicInstance();
157
if (callback) {
src/renderers/native/ReactNativeReconcileTransaction.js
+8
@@ -14,6 +14,7 @@
14
var CallbackQueue = require('CallbackQueue');
15
var PooledClass = require('PooledClass');
16
var Transaction = require('Transaction');
17
+var ReactInstrumentation = require('ReactInstrumentation');
18
var ReactUpdateQueue = require('ReactUpdateQueue');
19
20
/**
@@ -43,6 +44,13 @@ var ON_DOM_READY_QUEUEING = {
44
*/
45
var TRANSACTION_WRAPPERS = [ON_DOM_READY_QUEUEING];
46
47
+if (__DEV__) {
48
+ TRANSACTION_WRAPPERS.push({
49
+ initialize: ReactInstrumentation.debugTool.onBeginFlush,
50
+ close: ReactInstrumentation.debugTool.onEndFlush,
51
+ });
52
+}
53
+
54
/**
55
* Currently:
56
* - The order that these are listed in the transaction is critical:
src/renderers/shared/ReactDebugTool.js
+18
-9
@@ -79,7 +79,7 @@ function resetMeasurements() {
79
var previousMeasurements = currentFlushMeasurements || [];
80
var previousOperations = ReactHostOperationHistoryDevtool.getHistory();
81
82
- if (!isProfiling || currentFlushNesting === 0) {
82
+ if (currentFlushNesting === 0) {
83
currentFlushStartTime = null;
84
currentFlushMeasurements = null;
85
clearHistory();
@@ -106,7 +106,7 @@ function checkDebugID(debugID) {
106
}
107
108
function beginLifeCycleTimer(debugID, timerType) {
109
- if (!isProfiling || currentFlushNesting === 0) {
109
+ if (currentFlushNesting === 0) {
110
return;
111
}
112
warning(
@@ -125,7 +125,7 @@ function beginLifeCycleTimer(debugID, timerType) {
125
}
126
127
function endLifeCycleTimer(debugID, timerType) {
128
- if (!isProfiling || currentFlushNesting === 0) {
128
+ if (currentFlushNesting === 0) {
129
return;
130
}
131
warning(
@@ -137,11 +137,13 @@ function endLifeCycleTimer(debugID, timerType) {
137
currentTimerType || 'no',
138
(debugID === currentTimerDebugID) ? 'the same' : 'another'
139
);
140
- currentFlushMeasurements.push({
141
- timerType,
142
- instanceID: debugID,
143
- duration: performanceNow() - currentTimerStartTime - currentTimerNestedFlushDuration,
144
- });
140
+ if (isProfiling) {
141
+ currentFlushMeasurements.push({
142
+ timerType,
143
+ instanceID: debugID,
144
+ duration: performanceNow() - currentTimerStartTime - currentTimerNestedFlushDuration,
145
+ });
146
+ }
147
currentTimerStartTime = null;
148
currentTimerNestedFlushDuration = null;
149
currentTimerDebugID = null;
@@ -194,6 +196,7 @@ var ReactDebugTool = {
196
isProfiling = true;
197
flushHistory.length = 0;
198
resetMeasurements();
199
+ ReactDebugTool.addDevtool(ReactHostOperationHistoryDevtool);
200
},
201
endProfiling() {
202
if (!isProfiling) {
@@ -202,6 +205,7 @@ var ReactDebugTool = {
205
206
isProfiling = false;
207
resetMeasurements();
208
+ ReactDebugTool.removeDevtool(ReactHostOperationHistoryDevtool);
209
},
210
getFlushHistory() {
211
return flushHistory;
@@ -236,6 +240,12 @@ var ReactDebugTool = {
240
checkDebugID(debugID);
241
emitEvent('onEndReconcilerTimer', debugID, timerType);
242
},
243
+ onError(debugID) {
244
+ if (currentTimerDebugID != null) {
245
+ endLifeCycleTimer(currentTimerDebugID, currentTimerType);
246
+ }
247
+ emitEvent('onError', debugID);
248
+ },
249
onBeginProcessingChildContext() {
250
emitEvent('onBeginProcessingChildContext');
251
},
@@ -309,7 +319,6 @@ var ReactDebugTool = {
319
320
ReactDebugTool.addDevtool(ReactInvalidSetStateWarningDevTool);
321
ReactDebugTool.addDevtool(ReactComponentTreeDevtool);
312
-ReactDebugTool.addDevtool(ReactHostOperationHistoryDevtool);
322
ReactDebugTool.addDevtool(ReactChildrenMutationWarningDevtool);
323
var url = (ExecutionEnvironment.canUseDOM && window.location.href) || '';
324
if ((/[?&]react_perf\b/).test(url)) {
src/renderers/shared/__tests__/ReactPerf-test.js
+45
-1
@@ -467,5 +467,49 @@ describe('ReactPerf', function() {
467
expect(console.error.calls.count()).toBe(1);
468
469
__DEV__ = true;
470
- })
470
+ });
471
+
472
+ it('should work when measurement starts during reconciliation', () => {
473
+ // https://github.com/facebook/react/issues/6949#issuecomment-230371009
474
+ var Measurer = React.createClass({
475
+ componentWillMount() {
476
+ ReactPerf.start();
477
+ },
478
+ componentDidMount() {
479
+ ReactPerf.stop();
480
+ },
481
+ componentWillUpdate() {
482
+ ReactPerf.start();
483
+ },
484
+ componentDidUpdate() {
485
+ ReactPerf.stop();
486
+ },
487
+ render() {
488
+ // Force reconciliation despite constant element
489
+ return React.cloneElement(this.props.children);
490
+ },
491
+ });
492
+
493
+ var container = document.createElement('div');
494
+ ReactDOM.render(<Measurer><App /></Measurer>, container);
495
+ expect(ReactPerf.getWasted()).toEqual([]);
496
+
497
+ ReactDOM.render(<Measurer><App /></Measurer>, container);
498
+ expect(ReactPerf.getWasted()).toEqual([{
499
+ key: 'Measurer',
500
+ instanceCount: 1,
501
+ inclusiveRenderDuration: 4,
502
+ renderCount: 1,
503
+ }, {
504
+ key: 'App',
505
+ instanceCount: 1,
506
+ inclusiveRenderDuration: 3,
507
+ renderCount: 1,
508
+ }, {
509
+ key: 'App > Box',
510
+ instanceCount: 2,
511
+ inclusiveRenderDuration: 2,
512
+ renderCount: 2,
513
+ }]);
514
+ });
515
});
src/renderers/shared/devtools/__tests__/ReactHostOperationHistoryDevtool-test.js
+8
@@ -13,6 +13,7 @@
13
14
describe('ReactHostOperationHistoryDevtool', () => {
15
var React;
16
+ var ReactPerf;
17
var ReactDOM;
18
var ReactDOMComponentTree;
19
var ReactDOMFeatureFlags;
@@ -22,12 +23,19 @@ describe('ReactHostOperationHistoryDevtool', () => {
23
jest.resetModuleRegistry();
24
25
React = require('React');
26
+ ReactPerf = require('ReactPerf');
27
ReactDOM = require('ReactDOM');
28
ReactDOMComponentTree = require('ReactDOMComponentTree');
29
ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
30
ReactHostOperationHistoryDevtool = require('ReactHostOperationHistoryDevtool');
31
+
32
+ ReactPerf.start();
33
});
34
35
+ afterEach(() => {
36
+ ReactPerf.stop();
37
+ })
38
+
39
function assertHistoryMatches(expectedHistory) {
40
var actualHistory = ReactHostOperationHistoryDevtool.getHistory();
41
expect(actualHistory).toEqual(expectedHistory);
src/renderers/shared/stack/reconciler/ReactCompositeComponent.js
+5
@@ -467,6 +467,11 @@ var ReactCompositeComponentMixin = {
467
try {
468
markup = this.performInitialMount(renderedElement, hostParent, hostContainerInfo, transaction, context);
469
} catch (e) {
470
+ if (__DEV__) {
471
+ if (this._debugID !== 0) {
472
+ ReactInstrumentation.debugTool.onError();
473
+ }
474
+ }
475
// Roll back to checkpoint, handle error (which may add items to the transaction), and take a new checkpoint
476
transaction.rollback(checkpoint);
477
this._instance.unstable_handleError(e);
src/renderers/shared/stack/reconciler/ReactUpdates.js
-9
@@ -14,7 +14,6 @@
14
var CallbackQueue = require('CallbackQueue');
15
var PooledClass = require('PooledClass');
16
var ReactFeatureFlags = require('ReactFeatureFlags');
17
-var ReactInstrumentation = require('ReactInstrumentation');
17
var ReactReconciler = require('ReactReconciler');
18
var Transaction = require('Transaction');
19
@@ -193,10 +192,6 @@ function runBatchedUpdates(transaction) {
192
}
193
194
var flushBatchedUpdates = function() {
196
- if (__DEV__) {
197
- ReactInstrumentation.debugTool.onBeginFlush();
198
- }
199
-
195
// ReactUpdatesFlushTransaction's wrappers will clear the dirtyComponents
196
// array and perform any updates enqueued by mount-ready handlers (i.e.,
197
// componentDidUpdate) but we need to check here too in order to catch
@@ -216,10 +211,6 @@ var flushBatchedUpdates = function() {
211
CallbackQueue.release(queue);
212
}
213
}
219
-
220
- if (__DEV__) {
221
- ReactInstrumentation.debugTool.onEndFlush();
222
- }
214
};
215
216
/**
src/renderers/testing/ReactTestMount.js
-5
@@ -122,10 +122,6 @@ var ReactHostMount = {
122
123
var instance = instantiateReactComponent(nextWrappedElement, false);
124
125
- if (__DEV__) {
126
- ReactInstrumentation.debugTool.onBeginFlush();
127
- }
128
-
125
// The initial render is synchronous but any updates that happen during
126
// rendering, in componentWillMount or componentDidMount, will be batched
127
// according to the current batching strategy.
@@ -139,7 +135,6 @@ var ReactHostMount = {
135
ReactInstrumentation.debugTool.onMountRootComponent(
136
instance._renderedComponent._debugID
137
);
142
- ReactInstrumentation.debugTool.onEndFlush();
138
}
139
return new ReactTestInstance(instance);
140
},