Skip to content

Commit

Permalink
Bubble profiler durations to parent (rather than walking children)
Browse files Browse the repository at this point in the history 
Note this commit fails 5 tests. I am pushing it purely for sharing purposes.
  • Loading branch information
Brian Vaughn committed Sep 8, 2020
1 parent a789939 commit c7c0203
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 166 deletions.
21 changes: 10 additions & 11 deletions packages/react-reconciler/src/ReactFiber.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,10 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
// But works for yielding (the common case) and should support resuming.
workInProgress.actualDuration = 0;
workInProgress.actualStartTime = -1;

// Reset treeBaseDuration when cloning.
// As each child completes rendering, its base durations will bubble up to the parent.
workInProgress.treeBaseDuration = 0;
}
}

Expand Down Expand Up @@ -323,11 +327,6 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
workInProgress.index = current.index;
workInProgress.ref = current.ref;

if (enableProfilerTimer) {
workInProgress.selfBaseDuration = current.selfBaseDuration;
workInProgress.treeBaseDuration = current.treeBaseDuration;
}

if (__DEV__) {
workInProgress._debugNeedsRemount = current._debugNeedsRemount;
switch (workInProgress.tag) {
Expand Down Expand Up @@ -381,8 +380,8 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {
workInProgress.stateNode = null;

if (enableProfilerTimer) {
// Note: We don't reset the actualTime counts. It's useful to accumulate
// actual time across multiple render passes.
// Note: We don't reset the actualTime counts.
// It's useful to accumulate actual time across multiple render passes.
workInProgress.selfBaseDuration = 0;
workInProgress.treeBaseDuration = 0;
}
Expand Down Expand Up @@ -412,10 +411,10 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {
};

if (enableProfilerTimer) {
// Note: We don't reset the actualTime counts. It's useful to accumulate
// actual time across multiple render passes.
workInProgress.selfBaseDuration = current.selfBaseDuration;
workInProgress.treeBaseDuration = current.treeBaseDuration;
// Note: We don't reset the actualTime counts.
// It's useful to accumulate actual time across multiple render passes.
workInProgress.selfBaseDuration = 0;
workInProgress.treeBaseDuration = 0;
}
}

Expand Down
30 changes: 22 additions & 8 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1714,6 +1714,15 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
renderLanes,
)
) {
if (enableProfilerTimer) {
if ((workInProgress.mode & ProfileMode) !== NoMode) {
if (current !== null) {
// TODO (effects) Document
workInProgress.actualDuration = current.actualDuration;
}
}
}

// Something in this boundary's subtree already suspended. Switch to
// rendering the fallback children.
showFallback = true;
Expand Down Expand Up @@ -1990,10 +1999,9 @@ function mountSuspenseFallbackChildren(
primaryChildFragment.pendingProps = primaryChildProps;

if (enableProfilerTimer && workInProgress.mode & ProfileMode) {
// Reset the durations from the first pass so they aren't included in the
// final amounts. This seems counterintuitive, since we're intentionally
// not measuring part of the render phase, but this makes it match what we
// do in Concurrent Mode.
// Reset the durations from the first pass so they aren't included in the final amounts.
// This seems counterintuitive, since we're intentionally not measuring part of the render phase,
// but this makes it match what we do in Concurrent Mode.
primaryChildFragment.actualDuration = 0;
primaryChildFragment.actualStartTime = -1;
primaryChildFragment.selfBaseDuration = 0;
Expand Down Expand Up @@ -2111,10 +2119,9 @@ function updateSuspenseFallbackChildren(
primaryChildFragment.pendingProps = primaryChildProps;

if (enableProfilerTimer && workInProgress.mode & ProfileMode) {
// Reset the durations from the first pass so they aren't included in the
// final amounts. This seems counterintuitive, since we're intentionally
// not measuring part of the render phase, but this makes it match what we
// do in Concurrent Mode.
// Reset the durations from the first pass so they aren't included in the final amounts.
// This seems counterintuitive, since we're intentionally not measuring part of the render phase,
// but this makes it match what we do in Concurrent Mode.
primaryChildFragment.actualDuration = 0;
primaryChildFragment.actualStartTime = -1;
primaryChildFragment.selfBaseDuration =
Expand Down Expand Up @@ -2970,6 +2977,13 @@ function bailoutOnAlreadyFinishedWork(

// Check if the children have any pending work.
if (!includesSomeLane(renderLanes, workInProgress.childLanes)) {
if (enableProfilerTimer) {
if (current !== null) {
workInProgress.treeBaseDuration = current.treeBaseDuration;
workInProgress.selfBaseDuration = 0;
}
}

// The children don't have any work either. We can skip them.
// TODO: Once we add back resuming, we should check if the children are
// a work-in-progress set. If so, we need to transfer their effects.
Expand Down
171 changes: 78 additions & 93 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ import {
DidCapture,
Snapshot,
MutationMask,
PerformedWork,
StaticMask,
} from './ReactFiberFlags';
import invariant from 'shared/invariant';
Expand Down Expand Up @@ -155,7 +156,10 @@ import {
} from './ReactFiberLane';
import {resetChildFibers} from './ReactChildFiber.new';
import {createScopeInstance} from './ReactFiberScope.new';
import {transferActualDuration} from './ReactProfilerTimer.new';
import {
stopProfilerTimerIfRunning,
stopProfilerTimerIfRunningAndRecordDelta,
} from './ReactProfilerTimer.new';

function markUpdate(workInProgress: Fiber) {
// Tag the fiber with an update effect. This turns a Placement into
Expand Down Expand Up @@ -681,112 +685,99 @@ function cutOffTailIfNeeded(
}
}

function bubbleProperties(completedWork: Fiber) {
const didBailout =
completedWork.alternate !== null &&
completedWork.alternate.child === completedWork.child;
export function bubbleProfilerDurationsAfterError(erroredWork: Fiber): void {
if (enableProfilerTimer) {
if ((erroredWork.mode & ProfileMode) !== NoMode) {
const parent = erroredWork.return;
if (parent !== null) {
// TODO (effects) Document
parent.actualDuration += erroredWork.actualDuration;

let newChildLanes = NoLanes;
let subtreeFlags = NoFlags;
// TODO (effects) Document
parent.treeBaseDuration = 0;
}
}
}
}

if (!didBailout) {
// Bubble up the earliest expiration time.
if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) {
// In profiling mode, resetChildExpirationTime is also used to reset
// profiler durations.
let actualDuration = completedWork.actualDuration;
let treeBaseDuration = ((completedWork.selfBaseDuration: any): number);

let child = completedWork.child;
while (child !== null) {
newChildLanes = mergeLanes(
newChildLanes,
mergeLanes(child.lanes, child.childLanes),
);
// TODO (effects) Temorary method; replace with bubblePropertiesToParent
function bubbleProperties(completedWork: Fiber): void {
bubblePropertiesFromChildren(completedWork);
bubblePropertiesToParent(completedWork);
}

subtreeFlags |= child.subtreeFlags;
subtreeFlags |= child.flags;
function bubblePropertiesToParent(completedWork: Fiber): void {
const parent = completedWork.return;
if (parent !== null) {
const didBailout = (completeWork.flags & PerformedWork) !== NoFlags;
if (!didBailout) {
if (enableProfilerTimer) {
if ((completedWork.mode & ProfileMode) !== NoMode) {
stopProfilerTimerIfRunningAndRecordDelta(completedWork, true);

// When a fiber is cloned, its actualDuration is reset to 0. This value will
// only be updated if work is done on the fiber (i.e. it doesn't bailout).
// When work is done, it should bubble to the parent's actualDuration. If
// the fiber has not been cloned though, (meaning no work was done), then
// this value will reflect the amount of time spent working on a previous
// render. In that case it should not bubble. We determine whether it was
// cloned by comparing the child pointer.
actualDuration += child.actualDuration;
// At this point child base durations have already bubbled up to treeBaseDuration.
// Add our own base duration before bubbling further to the parent Fiber.
completedWork.treeBaseDuration += completedWork.selfBaseDuration;

treeBaseDuration += child.treeBaseDuration;
child = child.sibling;
// Bubble base durations to the parent.
parent.actualDuration += completedWork.actualDuration;
parent.treeBaseDuration += completedWork.treeBaseDuration;
}
}

completedWork.actualDuration = actualDuration;
completedWork.treeBaseDuration = treeBaseDuration;
} else {
let child = completedWork.child;
while (child !== null) {
newChildLanes = mergeLanes(
newChildLanes,
mergeLanes(child.lanes, child.childLanes),
);

subtreeFlags |= child.subtreeFlags;
subtreeFlags |= child.flags;
if (enableProfilerTimer) {
if ((completedWork.mode & ProfileMode) !== NoMode) {
stopProfilerTimerIfRunning(completedWork);

child = child.sibling;
parent.treeBaseDuration += completedWork.treeBaseDuration;
}
}
}
}
}

completedWork.subtreeFlags |= subtreeFlags;
} else {
// Bubble up the earliest expiration time.
if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) {
// In profiling mode, resetChildExpirationTime is also used to reset
// profiler durations.
let treeBaseDuration = ((completedWork.selfBaseDuration: any): number);

let child = completedWork.child;
while (child !== null) {
newChildLanes = mergeLanes(
newChildLanes,
mergeLanes(child.lanes, child.childLanes),
);
// TODO (effects) Move everything in this method into bubblePropertiesToParent
function bubblePropertiesFromChildren(completedWork: Fiber): void {
let newChildLanes = NoLanes;
let subtreeFlags = NoFlags;

// "Static" flags share the lifetime of the fiber/hook they belong to,
// so we should bubble those up even during a bailout. All the other
// flags have a lifetime only of a single render + commit, so we should
// ignore them.
subtreeFlags |= child.subtreeFlags & StaticMask;
subtreeFlags |= child.flags & StaticMask;
const didBailout =
completedWork.alternate !== null &&
completedWork.alternate.child === completedWork.child;
if (!didBailout) {
let child = completedWork.child;
while (child !== null) {
newChildLanes = mergeLanes(
newChildLanes,
mergeLanes(child.lanes, child.childLanes),
);

treeBaseDuration += child.treeBaseDuration;
child = child.sibling;
}
subtreeFlags |= child.subtreeFlags;
subtreeFlags |= child.flags;

completedWork.treeBaseDuration = treeBaseDuration;
} else {
let child = completedWork.child;
while (child !== null) {
newChildLanes = mergeLanes(
newChildLanes,
mergeLanes(child.lanes, child.childLanes),
);
child = child.sibling;
}
} else {
let child = completedWork.child;
while (child !== null) {
newChildLanes = mergeLanes(
newChildLanes,
mergeLanes(child.lanes, child.childLanes),
);

// "Static" flags share the lifetime of the fiber/hook they belong to,
// so we should bubble those up even during a bailout. All the other
// flags have a lifetime only of a single render + commit, so we should
// ignore them.
subtreeFlags |= child.subtreeFlags & StaticMask;
subtreeFlags |= child.flags & StaticMask;
// "Static" flags share the lifetime of the fiber/hook they belong to,
// so we should bubble those up even during a bailout. All the other
// flags have a lifetime only of a single render + commit, so we should
// ignore them.
subtreeFlags |= child.subtreeFlags & StaticMask;
subtreeFlags |= child.flags & StaticMask;

child = child.sibling;
}
child = child.sibling;
}

completedWork.subtreeFlags |= subtreeFlags;
}

completedWork.childLanes = newChildLanes;
completedWork.subtreeFlags |= subtreeFlags;
}

function completeWork(
Expand Down Expand Up @@ -1036,12 +1027,6 @@ function completeWork(
// Something suspended. Re-render with the fallback children.
workInProgress.lanes = renderLanes;
// Do not reset the effect list.
if (
enableProfilerTimer &&
(workInProgress.mode & ProfileMode) !== NoMode
) {
transferActualDuration(workInProgress);
}
return workInProgress;
}

Expand Down
19 changes: 1 addition & 18 deletions packages/react-reconciler/src/ReactFiberUnwindWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ import {
LegacyHiddenComponent,
} from './ReactWorkTags';
import {DidCapture, NoFlags, ShouldCapture} from './ReactFiberFlags';
import {NoMode, ProfileMode} from './ReactTypeOfMode';
import {
enableSuspenseServerRenderer,
enableProfilerTimer,
} from 'shared/ReactFeatureFlags';
import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags';

import {popHostContainer, popHostContext} from './ReactFiberHostContext.new';
import {popSuspenseContext} from './ReactFiberSuspenseContext.new';
Expand All @@ -40,7 +36,6 @@ import {
} from './ReactFiberContext.new';
import {popProvider} from './ReactFiberNewContext.new';
import {popRenderLanes} from './ReactFiberWorkLoop.new';
import {transferActualDuration} from './ReactProfilerTimer.new';

import invariant from 'shared/invariant';

Expand All @@ -54,12 +49,6 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
const flags = workInProgress.flags;
if (flags & ShouldCapture) {
workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
if (
enableProfilerTimer &&
(workInProgress.mode & ProfileMode) !== NoMode
) {
transferActualDuration(workInProgress);
}
return workInProgress;
}
return null;
Expand Down Expand Up @@ -100,12 +89,6 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
if (flags & ShouldCapture) {
workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
// Captured a suspense effect. Re-render the boundary.
if (
enableProfilerTimer &&
(workInProgress.mode & ProfileMode) !== NoMode
) {
transferActualDuration(workInProgress);
}
return workInProgress;
}
return null;
Expand Down
Loading

0 comments on commit c7c0203

Please sign in to comment.