[Flight] Track the function name that was called for I/O entries #33392
Conversation
github-actions
bot
added
the
React Core Team
|
Comparing: 3fb17d1...fed17d5 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
sebmarkbage
force-pushed
the
ioname
branch
2 times, most recently
from
010e1b7 to
825da84
Compare
| if (ioNode.stack !== null) { | ||
| const fullStack = parseStackTrace(ioNode.stack, 1); | ||
| stack = filterStackTrace(request, fullStack); | ||
| name = findCalledFunctionNameFromStackTrace(request, fullStack); |
There was a problem hiding this comment.
We should have a fallback here if we don't find a named stackframe. Otherwise createTask will fail since it requires a non-empty string.
I know we have a fallback introduced upstack but we can be a bit more specific. And this is the PR where this starts failing.
I'm trying to figure out why CI was fine with it.
There was a problem hiding this comment.
We should be able to search for more things inside the I/O stack if we can't find it but there's some tradeoffs there, so it's complicated. It can be a follow up.
Regardless, we won't always find one or we may find <anonymous> which we also encode as empty. So the downstream needs to be resilient to this being empty.
There was a problem hiding this comment.
I pushed a fix to handle the empty name case on the client. Went with just "unknown" for now.
| name = findCalledFunctionNameFromStackTrace(request, fullStack); | ||
| // The name can include the object that this was called on but sometimes that's | ||
| // just unnecessary context. | ||
| if (name.startsWith('Window.')) { |
There was a problem hiding this comment.
Where is Window. coming from?
There was a problem hiding this comment.
window.constructor.name. It mainly happens due to JS DOM but it can really happen when calling anything on window or even globally when one of the special cases to ignore it as a "method" call doesn't kick in.
Since like setTimeout(...) is actually window.setTimeout(...) which is a method call on an object of the Window class.
The fact that we need this cache is really a V8 quirk of only allowing structured data once. It's technically possible that the implementation also doesn't need parsing and doesn't need a cache. It's also not technically correct to use a shared cache for two different requests since the filter is on the request. But mainly I'm doing this so that I can access the filtered frames.
filterStackTrace evolved into being both parsing and filtering. Split them so we can use the same parsed value twice without relying on caching.
The name of the function that is actually called into from first party code is not actually on the stack so we extract that separately.
Stacked on #33392. This adds another track to the Performance Track called `"Server Requests"`. <img width="1015" alt="Screenshot 2025-06-01 at 12 02 14 AM" src="https://github.com/user-attachments/assets/c4d164c4-cfdf-4e14-9a87-3f011f65fd20" /> This logs the flat list of I/O awaited on by Server Components. There will be other views that are more focused on what data blocks a specific Component or Suspense boundary but this is just the list of all the I/O basically so you can get an overview of those waterfalls without the noise of all the Component trees and rendering. It's similar to what the "Network" track is on the client. I've been going back and forth on what to call this track but I went with `"Server Requests"` for now. The idea is that the name should communicate that this is something that happens on the server and is a pairing with the `"Server Components"` track. Although we don't use that feature, since it's missing granularity, it's also similar to "Server Timings".
| } else if (name.startsWith('<anonymous>.')) { | ||
| name = name.slice(7); |
There was a problem hiding this comment.
| } else if (name.startsWith('<anonymous>.')) { | |
| name = name.slice(7); | |
| } else if (name.startsWith('<anonymous>.')) { | |
| name = name.slice(12); |
There was a problem hiding this comment.
Does GCC inline '<anonymous>.'.length? Ideally we'd just use that to spot mistakes more easily.
Stacked on #33390.
The stack trace doesn't include the thing you called when calling into ignore listed content. We consider the ignore listed content conceptually the abstraction that you called that's interesting.
This extracts the name of the first ignore listed function that was called from user space. For example
"fetch". So we can know what kind of request this is.This could be enhanced and tweaked with heuristics in the future. For example, when you create a Promise yourself and call I/O inside of it like my
delayexamples, then we use that Promise as the I/O node but its stack doesn't have the actual I/O performed. It might be better to use the inner I/O node in that case. E.g.setTimeout. Currently I pick the name from the first party code instead - in my exampledelay.Another case that could be improved is the case where your whole component is third-party. In that case we still log the I/O but it has no context about what kind of I/O since the whole stack is ignored it just gets the component name for example. We could for example look at the first name that is in a different package than the package name of the ignored listed component. So if
node_modules/my-component-library/index.jscalls intonode_modules/mysql/connection.jsthen we could use the name from the inner.