@samitouri / QOS-React / commits / 48a8574a68

Fixed edge case bug in Profiler (#24031)

Brian Vaughn committed Mar 10, 2022 at 10:35 UTC 48a8574a68066be0385bd90c4a24e3e620eafb92

3 files changed +220 -29

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

+197

@@ -2960,6 +2960,94 @@ Object {

2960 }

2961 `;

2962

2963 +exports[`ProfilingCache should collect data for each root (including ones added or mounted after profiling started): Data for root Parent 3`] = `

2964 +Object {

2965 + "commitData": Array [

2966 + Object {

2967 + "changeDescriptions": Map {},

2968 + "duration": 0,

2969 + "effectDuration": 0,

2970 + "fiberActualDurations": Map {},

2971 + "fiberSelfDurations": Map {},

2972 + "passiveEffectDuration": 0,

2973 + "priorityLevel": "Immediate",

2974 + "timestamp": 34,

2975 + "updaters": Array [

2976 + Object {

2977 + "displayName": "render()",

2978 + "hocDisplayNames": null,

2979 + "id": 7,

2980 + "key": null,

2981 + "type": 11,

2982 + },

2983 + ],

2984 + },

2985 + ],

2986 + "displayName": "Parent",

2987 + "initialTreeBaseDurations": Map {

2988 + 7 => 11,

2989 + 8 => 11,

2990 + 10 => 0,

2991 + 11 => 1,

2992 + },

2993 + "operations": Array [

2994 + Array [

2995 + 1,

2996 + 7,

2997 + 0,

2998 + 2,

2999 + 4,

3000 + 11,

3001 + 10,

3002 + 8,

3003 + 7,

3004 + ],

3005 + ],

3006 + "rootID": 7,

3007 + "snapshots": Map {

3008 + 7 => Object {

3009 + "children": Array [

3010 + 8,

3011 + ],

3012 + "displayName": null,

3013 + "hocDisplayNames": null,

3014 + "id": 7,

3015 + "key": null,

3016 + "type": 11,

3017 + },

3018 + 8 => Object {

3019 + "children": Array [

3020 + 10,

3021 + 11,

3022 + ],

3023 + "displayName": "Parent",

3024 + "hocDisplayNames": null,

3025 + "id": 8,

3026 + "key": null,

3027 + "type": 5,

3028 + },

3029 + 10 => Object {

3030 + "children": Array [],

3031 + "displayName": "Child",

3032 + "hocDisplayNames": null,

3033 + "id": 10,

3034 + "key": "0",

3035 + "type": 5,

3036 + },

3037 + 11 => Object {

3038 + "children": Array [],

3039 + "displayName": "Child",

3040 + "hocDisplayNames": Array [

3041 + "Memo",

3042 + ],

3043 + "id": 11,

3044 + "key": null,

3045 + "type": 8,

3046 + },

3047 + },

3048 +}

3049 +`;

3050 +

3051 exports[`ProfilingCache should collect data for each root (including ones added or mounted after profiling started): imported data 1`] = `

3052 Object {

3053 "dataForRoots": Array [

@@ -3504,6 +3592,115 @@ Object {

3592 "rootID": 13,

3593 "snapshots": Array [],

3594 },

3595 + Object {

3596 + "commitData": Array [

3597 + Object {

3598 + "changeDescriptions": Array [],

3599 + "duration": 0,

3600 + "effectDuration": 0,

3601 + "fiberActualDurations": Array [],

3602 + "fiberSelfDurations": Array [],

3603 + "passiveEffectDuration": 0,

3604 + "priorityLevel": "Immediate",

3605 + "timestamp": 34,

3606 + "updaters": Array [

3607 + Object {

3608 + "displayName": "render()",

3609 + "hocDisplayNames": null,

3610 + "id": 7,

3611 + "key": null,

3612 + "type": 11,

3613 + },

3614 + ],

3615 + },

3616 + ],

3617 + "displayName": "Parent",

3618 + "initialTreeBaseDurations": Array [

3619 + Array [

3620 + 7,

3621 + 11,

3622 + ],

3623 + Array [

3624 + 8,

3625 + 11,

3626 + ],

3627 + Array [

3628 + 10,

3629 + 0,

3630 + ],

3631 + Array [

3632 + 11,

3633 + 1,

3634 + ],

3635 + ],

3636 + "operations": Array [

3637 + Array [

3638 + 1,

3639 + 7,

3640 + 0,

3641 + 2,

3642 + 4,

3643 + 11,

3644 + 10,

3645 + 8,

3646 + 7,

3647 + ],

3648 + ],

3649 + "rootID": 7,

3650 + "snapshots": Array [

3651 + Array [

3652 + 7,

3653 + Object {

3654 + "children": Array [

3655 + 8,

3656 + ],

3657 + "displayName": null,

3658 + "hocDisplayNames": null,

3659 + "id": 7,

3660 + "key": null,

3661 + "type": 11,

3662 + },

3663 + ],

3664 + Array [

3665 + 8,

3666 + Object {

3667 + "children": Array [

3668 + 10,

3669 + 11,

3670 + ],

3671 + "displayName": "Parent",

3672 + "hocDisplayNames": null,

3673 + "id": 8,

3674 + "key": null,

3675 + "type": 5,

3676 + },

3677 + ],

3678 + Array [

3679 + 10,

3680 + Object {

3681 + "children": Array [],

3682 + "displayName": "Child",

3683 + "hocDisplayNames": null,

3684 + "id": 10,

3685 + "key": "0",

3686 + "type": 5,

3687 + },

3688 + ],

3689 + Array [

3690 + 11,

3691 + Object {

3692 + "children": Array [],

3693 + "displayName": "Child",

3694 + "hocDisplayNames": Array [

3695 + "Memo",

3696 + ],

3697 + "id": 11,

3698 + "key": null,

3699 + "type": 8,

3700 + },

3701 + ],

3702 + ],

3703 + },

3704 ],

3705 "timelineData": Array [

3706 Object {

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

+1 -4

@@ -110,10 +110,7 @@ describe('ProfilingCache', () => {

110 });

111 }

112

113 - // No profiling data gets logged for the 2nd root (container B)

114 - // because it doesn't render anything while profiling.

115 - // (Technically it unmounts but we don't profile root unmounts.)

116 - expect(allProfilingDataForRoots).toHaveLength(2);

113 + expect(allProfilingDataForRoots).toHaveLength(3);

114

115 utils.exportImportHelper(bridge, store);

116

packages/react-devtools-shared/src/backend/renderer.js

+22 -25

@@ -1624,20 +1624,29 @@ export function attach(

1624 pendingOperations.push(op);

1625 }

1626

1627 - function flushOrQueueOperations(operations: OperationsArray): void {

1628 - if (operations.length === 3) {

1629 - // This operations array is a no op: [renderer ID, root ID, string table size (0)]

1630 - // We can usually skip sending updates like this across the bridge, unless we're Profiling.

1631 - // In that case, even though the tree didn't change– some Fibers may have still rendered.

1627 + function shouldBailoutWithPendingOperations() {

1628 + if (isProfiling) {

1629 if (

1633 - !isProfiling ||

1634 - currentCommitProfilingMetadata == null ||

1635 - currentCommitProfilingMetadata.durations.length === 0

1630 + currentCommitProfilingMetadata != null &&

1631 + currentCommitProfilingMetadata.durations.length > 0

1632 ) {

1637 - return;

1633 + return false;

1634 }

1635 }

1636

1637 + return (

1638 + pendingOperations.length === 0 &&

1639 + pendingRealUnmountedIDs.length === 0 &&

1640 + pendingSimulatedUnmountedIDs.length === 0 &&

1641 + pendingUnmountedRootID === null

1642 + );

1643 + }

1644 +

1645 + function flushOrQueueOperations(operations: OperationsArray): void {

1646 + if (shouldBailoutWithPendingOperations()) {

1647 + return;

1648 + }

1649 +

1650 if (pendingOperationsQueue !== null) {

1651 pendingOperationsQueue.push(operations);

1652 } else {

@@ -1668,7 +1677,7 @@ export function attach(

1677

1678 recordPendingErrorsAndWarnings();

1679

1671 - if (pendingOperations.length === 0) {

1680 + if (shouldBailoutWithPendingOperations()) {

1681 // No warnings or errors to flush; we can bail out early here too.

1682 return;

1683 }

@@ -1791,12 +1800,7 @@ export function attach(

1800 // We do this just before flushing, so we can ignore errors for no-longer-mounted Fibers.

1801 recordPendingErrorsAndWarnings();

1802

1794 - if (

1795 - pendingOperations.length === 0 &&

1796 - pendingRealUnmountedIDs.length === 0 &&

1797 - pendingSimulatedUnmountedIDs.length === 0 &&

1798 - pendingUnmountedRootID === null

1799 - ) {

1803 + if (shouldBailoutWithPendingOperations()) {

1804 // If we aren't profiling, we can just bail out here.

1805 // No use sending an empty update over the bridge.

1806 //

@@ -1805,9 +1809,7 @@ export function attach(

1809 // (2) the operations array for each commit

1810 // Because of this, it's important that the operations and metadata arrays align,

1811 // So it's important not to omit even empty operations while profiling is active.

1808 - if (!isProfiling) {

1809 - return;

1810 - }

1812 + return;

1813 }

1814

1815 const numUnmountIDs =

@@ -2724,12 +2726,7 @@ export function attach(

2726 }

2727

2728 if (isProfiling && isProfilingSupported) {

2727 - // Make sure at least one Fiber performed work during this commit.

2728 - // If not, don't send it to the frontend; showing an empty commit in the Profiler is confusing.

2729 - if (

2730 - currentCommitProfilingMetadata != null &&

2731 - currentCommitProfilingMetadata.durations.length > 0

2732 - ) {

2729 + if (!shouldBailoutWithPendingOperations()) {

2730 const commitProfilingMetadata = ((rootToCommitProfilingMetadataMap: any): CommitProfilingMetadataMap).get(

2731 currentRootID,

2732 );