Skip to content

Commit 69e732a

Browse files
author
Brian Vaughn
authored
Disable Profiler commit filtering (facebook#18862)
* Disable Profiler commit filtering We used to filter "empty" DevTools commits, but it was error prone (see facebook#18798). A commit may appear to be empty (no actual durations) because of component filters, but filtering these empty commits causes interaction commit indices to be off by N. This not only corrupts the resulting data, but also potentially causes runtime errors. For that matter, hiding "empty" commits might cause confusion too. A commit *did happen* even if none of the components the Profiler is showing were involved. * Restart flaky CI
1 parent fb3f0ac commit 69e732a

File tree

2 files changed

+74
-35
lines changed

2 files changed

+74
-35
lines changed

packages/react-devtools-shared/src/__tests__/__snapshots__/profilingCache-test.js.snap

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,7 +1500,17 @@ Object {
15001500

15011501
exports[`ProfilingCache should collect data for each root (including ones added or mounted after profiling started): Data for root Parent 3`] = `
15021502
Object {
1503-
"commitData": Array [],
1503+
"commitData": Array [
1504+
Object {
1505+
"changeDescriptions": Map {},
1506+
"duration": 0,
1507+
"fiberActualDurations": Map {},
1508+
"fiberSelfDurations": Map {},
1509+
"interactionIDs": Array [],
1510+
"priorityLevel": "Immediate",
1511+
"timestamp": 34,
1512+
},
1513+
],
15041514
"displayName": "Parent",
15051515
"initialTreeBaseDurations": Map {
15061516
6 => 11,
@@ -1510,7 +1520,19 @@ Object {
15101520
},
15111521
"interactionCommits": Map {},
15121522
"interactions": Map {},
1513-
"operations": Array [],
1523+
"operations": Array [
1524+
Array [
1525+
1,
1526+
6,
1527+
0,
1528+
2,
1529+
4,
1530+
9,
1531+
8,
1532+
7,
1533+
6,
1534+
],
1535+
],
15141536
"rootID": 6,
15151537
"snapshots": Map {
15161538
6 => Object {
@@ -2044,7 +2066,17 @@ Object {
20442066
"snapshots": Array [],
20452067
},
20462068
Object {
2047-
"commitData": Array [],
2069+
"commitData": Array [
2070+
Object {
2071+
"changeDescriptions": Array [],
2072+
"duration": 0,
2073+
"fiberActualDurations": Array [],
2074+
"fiberSelfDurations": Array [],
2075+
"interactionIDs": Array [],
2076+
"priorityLevel": "Immediate",
2077+
"timestamp": 34,
2078+
},
2079+
],
20482080
"displayName": "Parent",
20492081
"initialTreeBaseDurations": Array [
20502082
Array [
@@ -2066,7 +2098,19 @@ Object {
20662098
],
20672099
"interactionCommits": Array [],
20682100
"interactions": Array [],
2069-
"operations": Array [],
2101+
"operations": Array [
2102+
Array [
2103+
1,
2104+
6,
2105+
0,
2106+
2,
2107+
4,
2108+
9,
2109+
8,
2110+
7,
2111+
6,
2112+
],
2113+
],
20702114
"rootID": 6,
20712115
"snapshots": Array [
20722116
Array [

packages/react-devtools-shared/src/devtools/views/Profiler/utils.js

Lines changed: 26 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -60,43 +60,38 @@ export function prepareProfilingDataFrontendFromBackendAndStore(
6060
throw Error(`Could not find profiling snapshots for root ${rootID}`);
6161
}
6262

63-
const filteredCommitData = [];
64-
const filteredOperations = [];
65-
66-
// Filter empty commits from the profiler data.
67-
// It is very important to keep operations and commit data arrays perfect in sync.
68-
// So we must use the same criteria to filter both.
69-
// If these two arrays were to get out of sync, the profiler would runtime error.
70-
// We choose to filter on commit metadata, rather than the operations array,
71-
// because the latter may have false positives,
72-
// (e.g. a commit that re-rendered a component with the same treeBaseDuration as before).
73-
commitData.forEach((commitDataBackend, commitIndex) => {
74-
if (commitDataBackend.fiberActualDurations.length > 0) {
75-
filteredCommitData.push({
76-
changeDescriptions:
77-
commitDataBackend.changeDescriptions != null
78-
? new Map(commitDataBackend.changeDescriptions)
79-
: null,
80-
duration: commitDataBackend.duration,
81-
fiberActualDurations: new Map(
82-
commitDataBackend.fiberActualDurations,
83-
),
84-
fiberSelfDurations: new Map(commitDataBackend.fiberSelfDurations),
85-
interactionIDs: commitDataBackend.interactionIDs,
86-
priorityLevel: commitDataBackend.priorityLevel,
87-
timestamp: commitDataBackend.timestamp,
88-
});
89-
filteredOperations.push(operations[commitIndex]);
90-
}
91-
});
63+
// Do not filter empty commits from the profiler data!
64+
// We used to do this, but it was error prone (see #18798).
65+
// A commit may appear to be empty (no actual durations) because of component filters,
66+
// but filtering these empty commits causes interaction commit indices to be off by N.
67+
// This not only corrupts the resulting data, but also potentially causes runtime errors.
68+
//
69+
// For that matter, hiding "empty" commits might cause confusion too.
70+
// A commit *did happen* even if none of the components the Profiler is showing were involved.
71+
const convertedCommitData = commitData.map(
72+
(commitDataBackend, commitIndex) => ({
73+
changeDescriptions:
74+
commitDataBackend.changeDescriptions != null
75+
? new Map(commitDataBackend.changeDescriptions)
76+
: null,
77+
duration: commitDataBackend.duration,
78+
fiberActualDurations: new Map(
79+
commitDataBackend.fiberActualDurations,
80+
),
81+
fiberSelfDurations: new Map(commitDataBackend.fiberSelfDurations),
82+
interactionIDs: commitDataBackend.interactionIDs,
83+
priorityLevel: commitDataBackend.priorityLevel,
84+
timestamp: commitDataBackend.timestamp,
85+
}),
86+
);
9287

9388
dataForRoots.set(rootID, {
94-
commitData: filteredCommitData,
89+
commitData: convertedCommitData,
9590
displayName,
9691
initialTreeBaseDurations: new Map(initialTreeBaseDurations),
9792
interactionCommits: new Map(interactionCommits),
9893
interactions: new Map(interactions),
99-
operations: filteredOperations,
94+
operations,
10095
rootID,
10196
snapshots,
10297
});

0 commit comments

Comments
 (0)