[Flight] Start initial work immediately #30961
Merged
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
In a past update we made render and prerender have different work scheduling behavior because these methods are meant to be used in differeent environments with different performance tradeoffs in mind. For instance to prioritize streaming we want to allow as much IO to complete before triggering a round of work because we want to flush as few intermediate UI states. With Prerendering there will never be any intermediate UI states so we can more aggressively render tasks as they complete. One thing we've found is that even during render we should ideally kick off work immediately. This update normalizes the intitial work for render and prerender to start in a microtask. Choosing microtask over sync is somewhat arbitrary but there really isn't a reason to make them different between render/prerender so for now we'll unify them and keep it as a microtask for now. This change also updates pinging behavior. If the request is still in the initial task that spawned it then pings will schedule on the microtask queue. This allows immediately available async APIs to resolve right away. The concern with doing this for normal pings is that it might crowd out IO events but since this is the initial task there would be IO to already be scheduled.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
gnoff
commented
Sep 13, 2024
| @@ -1794,7 +1795,7 @@ function pingTask(request: Request, task: Task): void { | |||
| pingedTasks.push(task); | |||
| if (pingedTasks.length === 1) { | |||
| request.flushScheduled = request.destination !== null; | |||
| if (request.type === PRERENDER) { | |||
| if (request.type === PRERENDER || request.status === OPENING) { | |||
There was a problem hiding this comment.
Maybe we can unify the prerender path here and avoid the double conditional?
|
Comparing: 6774caa...77116a8 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) |
sebmarkbage
approved these changes
Sep 14, 2024
github-actions bot
pushed a commit
that referenced
this pull request
Sep 14, 2024
In a past update we made render and prerender have different work scheduling behavior because these methods are meant to be used in differeent environments with different performance tradeoffs in mind. For instance to prioritize streaming we want to allow as much IO to complete before triggering a round of work because we want to flush as few intermediate UI states. With Prerendering there will never be any intermediate UI states so we can more aggressively render tasks as they complete. One thing we've found is that even during render we should ideally kick off work immediately. This update normalizes the intitial work for render and prerender to start in a microtask. Choosing microtask over sync is somewhat arbitrary but there really isn't a reason to make them different between render/prerender so for now we'll unify them and keep it as a microtask for now. This change also updates pinging behavior. If the request is still in the initial task that spawned it then pings will schedule on the microtask queue. This allows immediately available async APIs to resolve right away. The concern with doing this for normal pings is that it might crowd out IO events but since this is the initial task there would be IO to already be scheduled. DiffTrain build for [fc5ef50](fc5ef50)
eps1lon
pushed a commit
to vercel/next.js
that referenced
this pull request
Sep 18, 2024
**breaking change for canary users: Bumps peer dependency of React from `19.0.0-rc-206df66e-20240912` to `19.0.0-rc-a99d8e8d-20240916`** [diff facebook/react@206df66e...a99d8e8d](facebook/react@206df66...a99d8e8) <details> <summary>React upstream changes</summary> - facebook/react#30977 - facebook/react#30971 - facebook/react#30922 - facebook/react#30917 - facebook/react#30902 - facebook/react#30912 - facebook/react#30970 - facebook/react#30969 - facebook/react#30967 - facebook/react#30966 - facebook/react#30960 - facebook/react#30968 - facebook/react#30961 - facebook/react#28255 - facebook/react#30957 - facebook/react#30958 - facebook/react#30959 - facebook/react#30951 - facebook/react#30954 - facebook/react#30920 - facebook/react#30942 </details>
gnoff
added a commit
that referenced
this pull request
Sep 26, 2024
In a recent update we make Flight start working immediately rather than waitin for a new task. This commit updates fizz to have similar mechanics. We start the render in the currently running task but we do so in a microtask to avoid reentrancy. This aligns Fizz with Flight. ref: #30961
github-actions bot
pushed a commit
that referenced
this pull request
Sep 26, 2024
In a recent update we make Flight start working immediately rather than waitin for a new task. This commit updates fizz to have similar mechanics. We start the render in the currently running task but we do so in a microtask to avoid reentrancy. This aligns Fizz with Flight. ref: #30961 DiffTrain build for [67fee58](67fee58)
github-actions bot
pushed a commit
that referenced
this pull request
Sep 26, 2024
In a recent update we make Flight start working immediately rather than waitin for a new task. This commit updates fizz to have similar mechanics. We start the render in the currently running task but we do so in a microtask to avoid reentrancy. This aligns Fizz with Flight. ref: #30961 DiffTrain build for [67fee58](67fee58)
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.
In a past update we made render and prerender have different work scheduling behavior because these methods are meant to be used in differeent environments with different performance tradeoffs in mind. For instance to prioritize streaming we want to allow as much IO to complete before triggering a round of work because we want to flush as few intermediate UI states. With Prerendering there will never be any intermediate UI states so we can more aggressively render tasks as they complete.
One thing we've found is that even during render we should ideally kick off work immediately. This update normalizes the intitial work for render and prerender to start in a microtask. Choosing microtask over sync is somewhat arbitrary but there really isn't a reason to make them different between render/prerender so for now we'll unify them and keep it as a microtask for now.
This change also updates pinging behavior. If the request is still in the initial task that spawned it then pings will schedule on the microtask queue. This allows immediately available async APIs to resolve right away. The concern with doing this for normal pings is that it might crowd out IO events but since this is the initial task there would be IO to already be scheduled.