[Fiber] Try to give a stack trace to every entry in the Scheduler Performance Track #34123
Conversation
Ensures that any of them can be clicked to see the cause of the render.
github-actions
bot
added
the
React Core Team
…ndary that caused a Retry or Offscreen lane Suspense and Idle tracks don't have their own update causes. The component is the cause.
E.g. the <form> jsx callsite if it was a built-in action prop.
sebmarkbage
force-pushed
the
includecauseonrender
branch
from
4b7d35c to
6f2c7d7
Compare
|
For all of these, it really sucks that Chrome shows the enclosing line instead of the actual line for async stacks. |
These can cause it to jump to two rows but it should always be one row.
| if (endTime <= startTime) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
I see that you've added this check to the loggers, but what has changed so that you need to check this?
If I remember correctly, DevTools just won't show entries on a timeline, where endTime < startTime, but they would still be present in an exported json-file, which could be useful for debugging Perf tracks implementation.
And if endTime == startTime, are zero-width entries displayed on a timeline?
There was a problem hiding this comment.
This is unrelated to this PR but it's the same as the change I made here for some other ones:
The problem is that they get logged as zero-width entries but those stack on top of whatever other entry is at the same time. Which causes the Scheduler track row to turn into a two row track instead of one and there's this little single pixel lines on the second line.
Ideally that track shouldn't even need to be expanded. It should be clickable while on a single line from the start.
| ? 'Update Blocked' | ||
| : 'Update', | ||
| isPingedUpdate | ||
| ? 'Promise Resolved' |
There was a problem hiding this comment.
Not related to these changes, just thinking out loud.
"Promise Resolved" sounds like an instant event, just a timestamp, instead of a complete event, where you have start and end.
I can't really think of a better naming for this, just want to avoid users thinking that Scheduler was actually waiting for this Promise to be resolved and this was blocking work. Same can be said about Cascading Update, though.
What do you think about naming this something like "Idle... Promise Resolved" and "Idle... Cascading Update"?
There was a problem hiding this comment.
Yea I don't like it neither. I also don't like "resolved" because it implies that the promise completed successfully but it can also be that it rejected.
Originally this track was called "Blocked" since it represents the time from when you called the update until it started rendering. I later changed it to "Update Blocked" only if it's a longer period. If it's short anyway then "Update" is basically an instant.
We could do something like "Promise Resolved but Blocked" but a bit long to fit.
There was a problem hiding this comment.
Internally we just called it "ping" but we don't publicly use that term anywhere. "Ping Blocked"
…formance Track (#34123) For "render" and "commit" phases we don't give any specific stack atm. This tries to always provide something useful to say the cause of the render. For normal renders this will now show the same thing as the "Event" and "Update" entries already showed. We stash the task that was used for those and use them throughout the render and commit phases. For Suspense (Retry lane) and Idle (Offscreen lane), we don't have any updates. Instead for those there's a component that left work behind in previous passes. For those I use the debugTask of the `<Suspense>` or `<Activity>` boundary to indicate that this was the root of the render. Similarly when an Action is invoked on a `<form action={...}>` component using the built-in submit handler, there's no actionable stack in user space that called it. So we use the stack of the JSX for the form instead. DiffTrain build for [4c9c109](4c9c109)
…formance Track (#34123) For "render" and "commit" phases we don't give any specific stack atm. This tries to always provide something useful to say the cause of the render. For normal renders this will now show the same thing as the "Event" and "Update" entries already showed. We stash the task that was used for those and use them throughout the render and commit phases. For Suspense (Retry lane) and Idle (Offscreen lane), we don't have any updates. Instead for those there's a component that left work behind in previous passes. For those I use the debugTask of the `<Suspense>` or `<Activity>` boundary to indicate that this was the root of the render. Similarly when an Action is invoked on a `<form action={...}>` component using the built-in submit handler, there's no actionable stack in user space that called it. So we use the stack of the JSX for the form instead. DiffTrain build for [4c9c109](4c9c109)
2 participants
For "render" and "commit" phases we don't give any specific stack atm. This tries to always provide something useful to say the cause of the render.
For normal renders this will now show the same thing as the "Event" and "Update" entries already showed. We stash the task that was used for those and use them throughout the render and commit phases.
For Suspense (Retry lane) and Idle (Offscreen lane), we don't have any updates. Instead for those there's a component that left work behind in previous passes. For those I use the debugTask of the
<Suspense>or<Activity>boundary to indicate that this was the root of the render.Similarly when an Action is invoked on a
<form action={...}>component using the built-in submit handler, there's no actionable stack in user space that called it. So we use the stack of the JSX for the form instead.