Filter empty commits (all Fibers bailed out) from Profiler (#22745)
Brian Vaughn committed
Nov 11, 2021 at 10:35 UTC
a44a7a2a3f8d7b24ddf5d0c6efeb6127e6b395c3
4 files changed
+86
-208
packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap
-197
@@ -1700,94 +1700,6 @@ Object {
1700
}
1701
`;
1702
1703
-exports[`ProfilingCache should collect data for each root (including ones added or mounted after profiling started): Data for root Parent 3`] = `
1704
-Object {
1705
- "commitData": Array [
1706
- Object {
1707
- "changeDescriptions": Map {},
1708
- "duration": 0,
1709
- "effectDuration": 0,
1710
- "fiberActualDurations": Map {},
1711
- "fiberSelfDurations": Map {},
1712
- "passiveEffectDuration": 0,
1713
- "priorityLevel": "Immediate",
1714
- "timestamp": 34,
1715
- "updaters": Array [
1716
- Object {
1717
- "displayName": "render()",
1718
- "hocDisplayNames": null,
1719
- "id": 7,
1720
- "key": null,
1721
- "type": 11,
1722
- },
1723
- ],
1724
- },
1725
- ],
1726
- "displayName": "Parent",
1727
- "initialTreeBaseDurations": Map {
1728
- 7 => 11,
1729
- 8 => 11,
1730
- 10 => 0,
1731
- 11 => 1,
1732
- },
1733
- "operations": Array [
1734
- Array [
1735
- 1,
1736
- 7,
1737
- 0,
1738
- 2,
1739
- 4,
1740
- 11,
1741
- 10,
1742
- 8,
1743
- 7,
1744
- ],
1745
- ],
1746
- "rootID": 7,
1747
- "snapshots": Map {
1748
- 7 => Object {
1749
- "children": Array [
1750
- 8,
1751
- ],
1752
- "displayName": null,
1753
- "hocDisplayNames": null,
1754
- "id": 7,
1755
- "key": null,
1756
- "type": 11,
1757
- },
1758
- 8 => Object {
1759
- "children": Array [
1760
- 10,
1761
- 11,
1762
- ],
1763
- "displayName": "Parent",
1764
- "hocDisplayNames": null,
1765
- "id": 8,
1766
- "key": null,
1767
- "type": 5,
1768
- },
1769
- 10 => Object {
1770
- "children": Array [],
1771
- "displayName": "Child",
1772
- "hocDisplayNames": null,
1773
- "id": 10,
1774
- "key": "0",
1775
- "type": 5,
1776
- },
1777
- 11 => Object {
1778
- "children": Array [],
1779
- "displayName": "Child",
1780
- "hocDisplayNames": Array [
1781
- "Memo",
1782
- ],
1783
- "id": 11,
1784
- "key": null,
1785
- "type": 8,
1786
- },
1787
- },
1788
-}
1789
-`;
1790
-
1703
exports[`ProfilingCache should collect data for each root (including ones added or mounted after profiling started): imported data 1`] = `
1704
Object {
1705
"dataForRoots": Array [
@@ -2330,115 +2242,6 @@ Object {
2242
"rootID": 13,
2243
"snapshots": Array [],
2244
},
2333
- Object {
2334
- "commitData": Array [
2335
- Object {
2336
- "changeDescriptions": Array [],
2337
- "duration": 0,
2338
- "effectDuration": 0,
2339
- "fiberActualDurations": Array [],
2340
- "fiberSelfDurations": Array [],
2341
- "passiveEffectDuration": 0,
2342
- "priorityLevel": "Immediate",
2343
- "timestamp": 34,
2344
- "updaters": Array [
2345
- Object {
2346
- "displayName": "render()",
2347
- "hocDisplayNames": null,
2348
- "id": 7,
2349
- "key": null,
2350
- "type": 11,
2351
- },
2352
- ],
2353
- },
2354
- ],
2355
- "displayName": "Parent",
2356
- "initialTreeBaseDurations": Array [
2357
- Array [
2358
- 7,
2359
- 11,
2360
- ],
2361
- Array [
2362
- 8,
2363
- 11,
2364
- ],
2365
- Array [
2366
- 10,
2367
- 0,
2368
- ],
2369
- Array [
2370
- 11,
2371
- 1,
2372
- ],
2373
- ],
2374
- "operations": Array [
2375
- Array [
2376
- 1,
2377
- 7,
2378
- 0,
2379
- 2,
2380
- 4,
2381
- 11,
2382
- 10,
2383
- 8,
2384
- 7,
2385
- ],
2386
- ],
2387
- "rootID": 7,
2388
- "snapshots": Array [
2389
- Array [
2390
- 7,
2391
- Object {
2392
- "children": Array [
2393
- 8,
2394
- ],
2395
- "displayName": null,
2396
- "hocDisplayNames": null,
2397
- "id": 7,
2398
- "key": null,
2399
- "type": 11,
2400
- },
2401
- ],
2402
- Array [
2403
- 8,
2404
- Object {
2405
- "children": Array [
2406
- 10,
2407
- 11,
2408
- ],
2409
- "displayName": "Parent",
2410
- "hocDisplayNames": null,
2411
- "id": 8,
2412
- "key": null,
2413
- "type": 5,
2414
- },
2415
- ],
2416
- Array [
2417
- 10,
2418
- Object {
2419
- "children": Array [],
2420
- "displayName": "Child",
2421
- "hocDisplayNames": null,
2422
- "id": 10,
2423
- "key": "0",
2424
- "type": 5,
2425
- },
2426
- ],
2427
- Array [
2428
- 11,
2429
- Object {
2430
- "children": Array [],
2431
- "displayName": "Child",
2432
- "hocDisplayNames": Array [
2433
- "Memo",
2434
- ],
2435
- "id": 11,
2436
- "key": null,
2437
- "type": 8,
2438
- },
2439
- ],
2440
- ],
2441
- },
2245
],
2246
"version": 5,
2247
}
packages/react-devtools-shared/src/__tests__/profilerStore-test.js
+51
@@ -124,6 +124,57 @@ describe('ProfilerStore', () => {
124
expect(data.operations).toHaveLength(1);
125
});
126
127
+ it('should filter empty commits alt', () => {
128
+ let commitCount = 0;
129
+
130
+ const inputRef = React.createRef();
131
+ const Example = () => {
132
+ const [, setTouched] = React.useState(false);
133
+
134
+ const handleBlur = () => {
135
+ setTouched(true);
136
+ };
137
+
138
+ require('scheduler').unstable_advanceTime(1);
139
+
140
+ React.useLayoutEffect(() => {
141
+ commitCount++;
142
+ });
143
+
144
+ return <input ref={inputRef} onBlur={handleBlur} />;
145
+ };
146
+
147
+ const container = document.createElement('div');
148
+
149
+ // This element has to be in the <body> for the event system to work.
150
+ document.body.appendChild(container);
151
+
152
+ // It's important that this test uses legacy sync mode.
153
+ // The root API does not trigger this particular failing case.
154
+ legacyRender(<Example />, container);
155
+
156
+ expect(commitCount).toBe(1);
157
+ commitCount = 0;
158
+
159
+ utils.act(() => store.profilerStore.startProfiling());
160
+
161
+ // Focus and blur.
162
+ const target = inputRef.current;
163
+ target.focus();
164
+ target.blur();
165
+ target.focus();
166
+ target.blur();
167
+ expect(commitCount).toBe(1);
168
+
169
+ utils.act(() => store.profilerStore.stopProfiling());
170
+
171
+ // Only one commit should have been recorded (in response to the "change" event).
172
+ const root = store.roots[0];
173
+ const data = store.profilerStore.getDataForRoot(root);
174
+ expect(data.commitData).toHaveLength(1);
175
+ expect(data.operations).toHaveLength(1);
176
+ });
177
+
178
it('should throw if component filters are modified while profiling', () => {
179
utils.act(() => store.profilerStore.startProfiling());
180
packages/react-devtools-shared/src/__tests__/profilingCache-test.js
+4
-1
@@ -108,7 +108,10 @@ describe('ProfilingCache', () => {
108
});
109
}
110
111
- expect(allProfilingDataForRoots).toHaveLength(3);
111
+ // No profiling data gets logged for the 2nd root (container B)
112
+ // because it doesn't render anything while profiling.
113
+ // (Technically it unmounts but we don't profile root unmounts.)
114
+ expect(allProfilingDataForRoots).toHaveLength(2);
115
116
utils.exportImportHelper(bridge, store);
117
packages/react-devtools-shared/src/backend/renderer.js
+31
-10
@@ -1546,6 +1546,19 @@ export function attach(
1546
}
1547
1548
function flushOrQueueOperations(operations: OperationsArray): void {
1549
+ if (operations.length === 3) {
1550
+ // This operations array is a no op: [renderer ID, root ID, string table size (0)]
1551
+ // We can usually skip sending updates like this across the bridge, unless we're Profiling.
1552
+ // In that case, even though the tree didn't change– some Fibers may have still rendered.
1553
+ if (
1554
+ !isProfiling ||
1555
+ currentCommitProfilingMetadata == null ||
1556
+ currentCommitProfilingMetadata.durations.length === 0
1557
+ ) {
1558
+ return;
1559
+ }
1560
+ }
1561
+
1562
if (pendingOperationsQueue !== null) {
1563
pendingOperationsQueue.push(operations);
1564
} else {
@@ -2608,18 +2621,26 @@ export function attach(
2621
}
2622
2623
if (isProfiling && isProfilingSupported) {
2611
- const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(
2612
- currentRootID,
2613
- );
2614
- if (commitProfilingMetadata != null) {
2615
- commitProfilingMetadata.push(
2616
- ((currentCommitProfilingMetadata: any): CommitProfilingData),
2617
- );
2618
- } else {
2619
- ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).set(
2624
+ // Make sure at least one Fiber performed work during this commit.
2625
+ // If not, don't send it to the frontend; showing an empty commit in the Profiler is confusing.
2626
+ if (
2627
+ currentCommitProfilingMetadata != null &&
2628
+ currentCommitProfilingMetadata.durations.length > 0
2629
+ ) {
2630
+ const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(
2631
currentRootID,
2621
- [((currentCommitProfilingMetadata: any): CommitProfilingData)],
2632
);
2633
+
2634
+ if (commitProfilingMetadata != null) {
2635
+ commitProfilingMetadata.push(
2636
+ ((currentCommitProfilingMetadata: any): CommitProfilingData),
2637
+ );
2638
+ } else {
2639
+ ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).set(
2640
+ currentRootID,
2641
+ [((currentCommitProfilingMetadata: any): CommitProfilingData)],
2642
+ );
2643
+ }
2644
}
2645
}
2646