Skip to content

Commit

Permalink
Fixed edge case bug in Profiler
Browse files Browse the repository at this point in the history
  • Loading branch information
Brian Vaughn committed Mar 8, 2022
1 parent d5f1b06 commit 47ba264
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2778,8 +2778,12 @@ Object {
1,
0,
2,
1,
5,
4,
11,
10,
8,
7,
],
],
"rootID": 1,
Expand Down Expand Up @@ -3279,8 +3283,12 @@ Object {
1,
0,
2,
1,
5,
4,
11,
10,
8,
7,
],
],
"rootID": 1,
Expand Down
47 changes: 21 additions & 26 deletions packages/react-devtools-shared/src/backend/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1624,18 +1624,25 @@ export function attach(
pendingOperations.push(op);
}

function flushOrQueueOperations(operations: OperationsArray): void {
if (operations.length === 3) {
// This operations array is a no op: [renderer ID, root ID, string table size (0)]
// We can usually skip sending updates like this across the bridge, unless we're Profiling.
// In that case, even though the tree didn't change– some Fibers may have still rendered.
if (
!isProfiling ||
function shouldBailoutWithPendingOperations() {
if (!isProfiling) {
return (
pendingOperations.length === 0 &&
pendingRealUnmountedIDs.length === 0 &&
pendingSimulatedUnmountedIDs.length === 0 &&
pendingUnmountedRootID === null
);
} else {
return (
currentCommitProfilingMetadata == null ||
currentCommitProfilingMetadata.durations.length === 0
) {
return;
}
);
}
}

function flushOrQueueOperations(operations: OperationsArray): void {
if (shouldBailoutWithPendingOperations()) {
return;
}

if (pendingOperationsQueue !== null) {
Expand Down Expand Up @@ -1668,7 +1675,7 @@ export function attach(

recordPendingErrorsAndWarnings();

if (pendingOperations.length === 0) {
if (shouldBailoutWithPendingOperations()) {
// No warnings or errors to flush; we can bail out early here too.
return;
}
Expand Down Expand Up @@ -1791,12 +1798,7 @@ export function attach(
// We do this just before flushing, so we can ignore errors for no-longer-mounted Fibers.
recordPendingErrorsAndWarnings();

if (
pendingOperations.length === 0 &&
pendingRealUnmountedIDs.length === 0 &&
pendingSimulatedUnmountedIDs.length === 0 &&
pendingUnmountedRootID === null
) {
if (shouldBailoutWithPendingOperations()) {
// If we aren't profiling, we can just bail out here.
// No use sending an empty update over the bridge.
//
Expand All @@ -1805,9 +1807,7 @@ export function attach(
// (2) the operations array for each commit
// Because of this, it's important that the operations and metadata arrays align,
// So it's important not to omit even empty operations while profiling is active.
if (!isProfiling) {
return;
}
return;
}

const numUnmountIDs =
Expand Down Expand Up @@ -2724,12 +2724,7 @@ export function attach(
}

if (isProfiling && isProfilingSupported) {
// Make sure at least one Fiber performed work during this commit.
// If not, don't send it to the frontend; showing an empty commit in the Profiler is confusing.
if (
currentCommitProfilingMetadata != null &&
currentCommitProfilingMetadata.durations.length > 0
) {
if (!shouldBailoutWithPendingOperations()) {
const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(
currentRootID,
);
Expand Down

0 comments on commit 47ba264

Please sign in to comment.