[Flight] Include I/O not awaited in user space#33715
Merged
sebmarkbage merged 2 commits intofacebook:mainfrom Jul 7, 2025
Merged
[Flight] Include I/O not awaited in user space#33715sebmarkbage merged 2 commits intofacebook:mainfrom
sebmarkbage merged 2 commits intofacebook:mainfrom
Conversation
|
Comparing: 9a645e1...28b7f3e Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
eps1lon
approved these changes
Jul 7, 2025
unstubbable
reviewed
Jul 7, 2025
| // that's part of a start up sequence or externally cached data. We exclude that information. | ||
| // The technique for debugging the effects of uncached data on the render is to simply uncache it. | ||
| return null; | ||
| return previousIONode; |
Collaborator
There was a problem hiding this comment.
The comment above makes we wonder whether this change may affect cached I/O. At least in the Flight fixture it doesn't seem to cause an issue with the cached third-party stream though.
Contributor
Author
There was a problem hiding this comment.
The previousIONode has its own check if it finished before this sequence so it’ll also be null.
We don’t really expect the future node to have finished before the previous. But for consistency I return it here.
This file contains hidden or 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
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.
If I/O is not awaited in user space in a "previous" path we used to just drop it on the floor. There's a few strategies we could apply here. My first commit just emits it without an await but that would mean we don't have an await stack when there's no I/O in a follow up.
I went with a strategy where the "previous" I/O is used only if the "next" didn't have I/O. This may still drop I/O on the floor if there's two back to back within internals for example. It would only log the first one even though the outer await may have started earlier.
It may also log deeper in the "next" path if that had user space stacks and then the outer await will appear as if it awaited after.
So it's not perfect.