[Fiber] Optimize enableProfilerCommitHooks by Collecting Elapsed Effect Duration in Module Scope #30981
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Follows the pattern of the others.
This can use commitHookEffectListUnmount helper. The net effect is that all insertion effects unmount first and then all layout effects. That's the order that happens for updates and mounts and mirrors how generally different phase unmounts happen.
This wrapper adds the timings of layout effects.
When we enter or leaves a Profiler boundary we can collect the effect duration elapsed within that scope and add it to the counter. This avoids needing to search back up for every Fiber.
We infer if this was layout or passive duration when we propagate the number in the phase. We can also just use the same start timer as for render since they don't overlap.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
| // If we refactor to preserve the unmounted Fiber tree we could fix this. | ||
| // The current implementation would require too much extra overhead to track this. | ||
| expect(call[2]).toBe(0); // durations | ||
| expect(call[2]).toBe(10100); // durations |
rickhanlonii
approved these changes
Sep 17, 2024
| this.effectDuration = 0; | ||
| this.passiveEffectDuration = 0; | ||
| this.effectDuration = -0; | ||
| this.passiveEffectDuration = -0; |
github-actions bot
pushed a commit
that referenced
this pull request
Sep 17, 2024
…ct Duration in Module Scope (#30981) Stacked on #30979. The problem with the previous approach is that it recursively walked the tree up to propagate the resulting time from recording a layout effect. Instead, we keep a running count of the effect duration on the module scope. Then we reset it when entering a nested Profiler and then we add its elapsed count when we exit the Profiler. This also fixes a bug where we weren't previously including unmount times for some detached trees since they couldn't bubble up to find the profiler. DiffTrain build for commit 4549be0.
github-actions bot
pushed a commit
that referenced
this pull request
Sep 17, 2024
…ct Duration in Module Scope (#30981) Stacked on #30979. The problem with the previous approach is that it recursively walked the tree up to propagate the resulting time from recording a layout effect. Instead, we keep a running count of the effect duration on the module scope. Then we reset it when entering a nested Profiler and then we add its elapsed count when we exit the Profiler. This also fixes a bug where we weren't previously including unmount times for some detached trees since they couldn't bubble up to find the profiler. DiffTrain build for [4549be0](4549be0)
sebmarkbage
added a commit
that referenced
this pull request
Sep 17, 2024
Stacked on #30981. Same as #30967 but for effects. This logs a tree of components using `performance.measure()`. In addition to the previous render phase this logs one tree for each commit phase: - Mutation Phase - Layout Effect - Passive Unmounts - Passive Mounts I currently skip the Before Mutation phase since the snapshots are so unusual it's not worth creating trees for those. The mechanism is that I reuse the timings we track for `enableProfilerCommitHooks`. I track first and last effect timestamp within each component subtree. Then on the way up do we log the entry. This means that we don't include overhead to find our way down to a component and that we don't need to add any additional overhead by reading timestamps. To ensure that the entries get ordered correctly we need to ensure that the start time of each parent is slightly before the inner one.
github-actions bot
pushed a commit
that referenced
this pull request
Sep 17, 2024
Stacked on #30981. Same as #30967 but for effects. This logs a tree of components using `performance.measure()`. In addition to the previous render phase this logs one tree for each commit phase: - Mutation Phase - Layout Effect - Passive Unmounts - Passive Mounts I currently skip the Before Mutation phase since the snapshots are so unusual it's not worth creating trees for those. The mechanism is that I reuse the timings we track for `enableProfilerCommitHooks`. I track first and last effect timestamp within each component subtree. Then on the way up do we log the entry. This means that we don't include overhead to find our way down to a component and that we don't need to add any additional overhead by reading timestamps. To ensure that the entries get ordered correctly we need to ensure that the start time of each parent is slightly before the inner one. DiffTrain build for [e1c2090](e1c2090)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stacked on #30979.
The problem with the previous approach is that it recursively walked the tree up to propagate the resulting time from recording a layout effect.
Instead, we keep a running count of the effect duration on the module scope. Then we reset it when entering a nested Profiler and then we add its elapsed count when we exit the Profiler.
This also fixes a bug where we weren't previously including unmount times for some detached trees since they couldn't bubble up to find the profiler.