Multi-renderer profiling improvements
Add support for mixed v15/v16 renderers that previously caused profiling to fail with 'profiling not supported by this renderer' type errors
Brian Vaughn committed
Aug 16, 2019 at 11:25 UTC
454157dd660f7ed70684a459fff0482bee1d87d1
4 files changed
+22
-7
src/__tests__/profilerContext-test.js
+1
-1
@@ -135,7 +135,7 @@ describe('ProfilerContext', () => {
135
expect(context.didRecordCommits).toBe(false);
136
expect(context.isProcessingData).toBe(false);
137
expect(context.isProfiling).toBe(false);
138
- expect(context.profilingData).not.toBe(null);
138
+ expect(context.profilingData).toBe(null);
139
140
done();
141
});
src/backend/legacy/renderer.js
+2
-2
@@ -875,10 +875,10 @@ export function attach(
875
throw new Error('setInHook not supported by this renderer');
876
};
877
const startProfiling = () => {
878
- throw new Error('startProfiling not supported by this renderer');
878
+ // Do not throw, since this would break a multi-root scenario where v15 and v16 were both present.
879
};
880
const stopProfiling = () => {
881
- throw new Error('stopProfiling not supported by this renderer');
881
+ // Do not throw, since this would break a multi-root scenario where v15 and v16 were both present.
882
};
883
884
function getBestMatchForTrackedPath(): PathMatch | null {
src/backend/renderer.js
+9
-3
@@ -1614,7 +1614,9 @@ export function attach(
1614
currentRootID = getFiberID(getPrimaryFiber(root.current));
1615
setRootPseudoKey(currentRootID, root.current);
1616
1617
- if (isProfiling) {
1617
+ // Checking root.memoizedInteractions handles multi-renderer edge-case-
1618
+ // where some v16 renderers support profiling and others don't.
1619
+ if (isProfiling && root.memoizedInteractions != null) {
1620
// If profiling is active, store commit time and duration, and the current interactions.
1621
// The frontend may request this information after profiling has stopped.
1622
currentCommitProfilingMetadata = {
@@ -1658,7 +1660,11 @@ export function attach(
1660
mightBeOnTrackedPath = true;
1661
}
1662
1661
- if (isProfiling) {
1663
+ // Checking root.memoizedInteractions handles multi-renderer edge-case-
1664
+ // where some v16 renderers support profiling and others don't.
1665
+ const isProfilingSupported = root.memoizedInteractions != null;
1666
+
1667
+ if (isProfiling && isProfilingSupported) {
1668
// If profiling is active, store commit time and duration, and the current interactions.
1669
// The frontend may request this information after profiling has stopped.
1670
currentCommitProfilingMetadata = {
@@ -1702,7 +1708,7 @@ export function attach(
1708
mountFiberRecursively(current, null);
1709
}
1710
1705
- if (isProfiling) {
1711
+ if (isProfiling && isProfilingSupported) {
1712
const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(
1713
currentRootID
1714
);
src/devtools/ProfilerStore.js
+10
-1
@@ -62,6 +62,9 @@ export default class ProfilerStore extends EventEmitter<{|
62
// When profiling is in progress, operations are stored so that we can later reconstruct past commit trees.
63
_isProfiling: boolean = false;
64
65
+ // Tracks whether a specific renderer logged any profiling data during the most recent session.
66
+ _rendererIDsThatReportedProfilingData: Set<number> = new Set();
67
+
68
// After profiling, data is requested from each attached renderer using this queue.
69
// So long as this queue is not empty, the store is retrieving and processing profiling data from the backend.
70
_rendererQueue: Set<number> = new Set();
@@ -233,6 +236,8 @@ export default class ProfilerStore extends EventEmitter<{|
236
if (!this._initialSnapshotsByRootID.has(rootID)) {
237
this._initialSnapshotsByRootID.set(rootID, new Map());
238
}
239
+
240
+ this._rendererIDsThatReportedProfilingData.add(rendererID);
241
}
242
};
243
@@ -280,6 +285,7 @@ export default class ProfilerStore extends EventEmitter<{|
285
this._initialRendererIDs.clear();
286
this._initialSnapshotsByRootID.clear();
287
this._inProgressOperationsByRootID.clear();
288
+ this._rendererIDsThatReportedProfilingData.clear();
289
this._rendererQueue.clear();
290
291
// Record all renderer IDs initially too (in case of unmount)
@@ -315,7 +321,10 @@ export default class ProfilerStore extends EventEmitter<{|
321
this._dataBackends.splice(0);
322
this._rendererQueue.clear();
323
318
- this._initialRendererIDs.forEach(rendererID => {
324
+ // Only request data from renderers that actually logged it.
325
+ // This avoids unnecessary bridge requests and also avoids edge case mixed renderer bugs.
326
+ // (e.g. when v15 and v16 are both present)
327
+ this._rendererIDsThatReportedProfilingData.forEach(rendererID => {
328
if (!this._rendererQueue.has(rendererID)) {
329
this._rendererQueue.add(rendererID);
330