Skip to content

Effects list refactor: Move resetChildLanes into complete work #19440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Effects list refactor: Move resetChildLanes into complete work #19440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a789939
Moved resetChildLanes into complete work
Sep 8, 2020
c7c0203
Bubble profiler durations to parent (rather than walking children)
Sep 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions packages/react-reconciler/src/ReactFiber.new.js
View file
Open in desktop
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
Open in desktop
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
Loading