Skip to content
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.

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

[Fiber] Mark cascading updates #31866

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Dec 20, 2024

A common source of performance problems is due to cascading renders from calling setState in useLayoutEffect or useEffect. This marks the entry from the update to when we start the render as red and "Cascade" to highlight this.

Screenshot 2024-12-19 at 10 54 59 PM

In addition to this case, there's another case where you call setState multiple times in the same event causing multiple renders. This might be due to multiple flushSync, or spawned a microtasks from a useLayoutEffect. In theory it could also be from a microtask scheduled after the first setState. This one we can only detect if it's from an event that has a window.event since otherwise it's hard to know if we're still in the same event.

Screenshot 2024-12-19 at 11 38 44 PM

I decided against making a ping in a microtask considered a cascade. Because that should ideally be using the Suspense Optimization and so wouldn't be considered multi-pass.

Screenshot 2024-12-19 at 11 07 30 PM

We might consider making the whole render phase and maybe commit phase red but that should maybe reserved for actual errors. The "Blocked" phase really represents the setState and so will have the stack trace of the first update.

@sebmarkbage sebmarkbage requested a review from acdlite December 20, 2024 04:41
Copy link

vercel bot commented Dec 20, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 20, 2024 4:43am

@@ -229,13 +229,17 @@ function flushSyncWorkAcrossRoots_impl(
isFlushingWork = false;
}

function processRootScheduleInMicrotask() {
function processRootScheduleInImmediateTask() {
if (enableProfilerTimer && enableComponentPerformanceTrack) {
// Track the currently executing event if there is one so we can ignore this
// event when logging events.
trackSchedulerEvent();
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to restructure this a bit so that we have two separate callbacks depending on if we do the Scheduler_scheduleCallback pass or the microtask pass. Because we shouldn't call trackSchedulerEvent() if we're inside a microtask callback because that could mark a real event as if it was a scheduler event (and therefore hidden).

@react-sizebot
Copy link

Comparing: 518d06d...75b0af0

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js +0.03% 511.38 kB 511.55 kB +0.02% 91.38 kB 91.39 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.03% 516.17 kB 516.33 kB +0.02% 92.23 kB 92.24 kB
facebook-www/ReactDOM-prod.classic.js +0.03% 593.09 kB 593.25 kB +0.02% 104.45 kB 104.47 kB
facebook-www/ReactDOM-prod.modern.js +0.03% 583.35 kB 583.52 kB +0.02% 102.90 kB 102.93 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 3709ba4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants