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.

Sign up for GitHub

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

Jump to bottom

Improve rendering performance #64408

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Improve rendering performance #64408

merged 1 commit into from
Apr 12, 2024
+47 −21

Conversation

shuding
Copy link
Member

@shuding shuding commented Apr 12, 2024 •
edited
Loading

This PR improves the server rendering performance a bit, especially for the App Router. It has mainly 2 changes.

A rough benchmark test with a 300kB Lorem Ipsum page, SSR is about 18% faster.

Improve getServerInsertedHTML

  • Avoid an extra renderToReadableStream if we already know the content will be empty
  • Avoid await stream.allReady for better parallelism with streamToString()
  • Increase the progressiveChunkSize

Improve createHeadInsertionTransformStream

  • Only do chunk splitting and enqueuing if the inserted string is not empty

@shuding
improve rendering performance
Loading
Loading status checks…
a940f55
@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Apr 12, 2024
feedthejim
feedthejim reviewed Apr 12, 2024
View reviewed changes
packages/next/src/server/app-render/make-get-server-inserted-html.tsx
{
// Larger chunk because this isn't sent over the network.
// Let's set it to 1MB.
progressiveChunkSize: 1024 * 1024,
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting

feedthejim
feedthejim reviewed Apr 12, 2024
View reviewed changes
packages/next/src/build/index.ts
@@ -2983,12 +2983,13 @@ export default async function build(
if (i18n) {
if (additionalSsgFile) return

const localeExt = page === '/' ? path.extname(file) : ''
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this lol

Copy link
Member Author

Choose a reason for hiding this comment

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

The path.extname() doesn't have to be called inside the loop as it doesn't change

feedthejim
feedthejim reviewed Apr 12, 2024
View reviewed changes
packages/next/src/server/app-render/make-get-server-inserted-html.tsx

// There's no need to wait for the stream to be ready
// e.g. calling `await stream.allReady` because `streamToString` will
// wait and decode the stream progressively with better parallelism.
return streamToString(stream)
Copy link
Contributor

Choose a reason for hiding this comment

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

Another low hanging fruit there is that maybe we can just call renderToString in some places where we know there won't be anything that needs to be streamed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that would require a larger change but this is definitely worth it (blocked by our edge React server not having renderToString exported). It will make this insertion function sync, and all the underlying stream methods sync too.

@ijjk
Copy link
Member

ijjk commented Apr 12, 2024

Stats from current PR

Default Build
General Overall increase ⚠️
vercel/next.js canary vercel/next.js shu/fs6b Change
buildDuration 14.2s 14s N/A
buildDurationCached 7.6s 6.3s N/A
nodeModulesSize 199 MB 199 MB ⚠️ +24.4 kB
nextStartRea..uration (ms) 400ms 400ms ✓
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js shu/fs6b Change
2453-HASH.js gzip 31.4 kB 31.4 kB N/A
3304.HASH.js gzip 181 B 181 B ✓
3f784ff6-HASH.js gzip 53.7 kB 53.7 kB ✓
8299-HASH.js gzip 5.1 kB 5.1 kB N/A
framework-HASH.js gzip 45.2 kB 45.2 kB ✓
main-app-HASH.js gzip 242 B 241 B N/A
main-HASH.js gzip 32.2 kB 32.2 kB N/A
webpack-HASH.js gzip 1.68 kB 1.68 kB N/A
Overall change 99 kB 99 kB ✓
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js shu/fs6b Change
polyfills-HASH.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary vercel/next.js shu/fs6b Change
_app-HASH.js gzip 196 B 197 B N/A
_error-HASH.js gzip 184 B 184 B ✓
amp-HASH.js gzip 505 B 505 B ✓
css-HASH.js gzip 324 B 325 B N/A
dynamic-HASH.js gzip 2.5 kB 2.5 kB N/A
edge-ssr-HASH.js gzip 258 B 258 B ✓
head-HASH.js gzip 352 B 352 B ✓
hooks-HASH.js gzip 370 B 371 B N/A
image-HASH.js gzip 4.27 kB 4.27 kB ✓
index-HASH.js gzip 259 B 259 B ✓
link-HASH.js gzip 2.67 kB 2.67 kB N/A
routerDirect..HASH.js gzip 314 B 312 B N/A
script-HASH.js gzip 386 B 386 B ✓
withRouter-HASH.js gzip 309 B 309 B ✓
1afbb74e6ecf..834.css gzip 106 B 106 B ✓
Overall change 6.63 kB 6.63 kB ✓
Client Build Manifests
vercel/next.js canary vercel/next.js shu/fs6b Change
_buildManifest.js gzip 483 B 485 B N/A
Overall change 0 B 0 B ✓
Rendered Page Sizes
vercel/next.js canary vercel/next.js shu/fs6b Change
index.html gzip 530 B 528 B N/A
link.html gzip 542 B 540 B N/A
withRouter.html gzip 525 B 523 B N/A
Overall change 0 B 0 B ✓
Edge SSR bundle Size
vercel/next.js canary vercel/next.js shu/fs6b Change
edge-ssr.js gzip 95.6 kB 95.6 kB N/A
page.js gzip 3.06 kB 3.06 kB N/A
Overall change 0 B 0 B ✓
Middleware size
vercel/next.js canary vercel/next.js shu/fs6b Change
middleware-b..fest.js gzip 624 B 626 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 25.5 kB 25.5 kB N/A
edge-runtime..pack.js gzip 839 B 839 B ✓
Overall change 839 B 839 B ✓
Next Runtimes
vercel/next.js canary vercel/next.js shu/fs6b Change
app-page-exp...dev.js gzip 171 kB 171 kB N/A
app-page-exp..prod.js gzip 97.4 kB 97.5 kB N/A
app-page-tur..prod.js gzip 99.2 kB 99.2 kB N/A
app-page-tur..prod.js gzip 93.4 kB 93.5 kB N/A
app-page.run...dev.js gzip 144 kB 145 kB N/A
app-page.run..prod.js gzip 91.9 kB 92 kB N/A
app-route-ex...dev.js gzip 21.4 kB 21.4 kB ✓
app-route-ex..prod.js gzip 15.2 kB 15.2 kB ✓
app-route-tu..prod.js gzip 15.2 kB 15.2 kB ✓
app-route-tu..prod.js gzip 14.9 kB 14.9 kB ✓
app-route.ru...dev.js gzip 21.1 kB 21.1 kB ✓
app-route.ru..prod.js gzip 14.9 kB 14.9 kB ✓
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB ✓
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB ✓
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB ✓
pages-turbo...prod.js gzip 22.5 kB 22.5 kB N/A
pages.runtim...dev.js gzip 23.1 kB 23.1 kB N/A
pages.runtim..prod.js gzip 22.5 kB 22.5 kB N/A
server.runti..prod.js gzip 51.3 kB 51.3 kB ✓
Overall change 183 kB 183 kB ✓
build cache
vercel/next.js canary vercel/next.js shu/fs6b Change
0.pack gzip 1.58 MB 1.58 MB N/A
index.pack gzip 107 kB 107 kB N/A
Overall change 0 B 0 B ✓
Diff details
Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Diff for app-page.runtime.prod.js

Diff too large to display

Diff for pages-turbo...time.prod.js

Diff too large to display

Diff for pages.runtime.dev.js

Diff too large to display

Diff for pages.runtime.prod.js

Diff too large to display

Commit: a940f55

@ijjk
Copy link
Member

ijjk commented Apr 12, 2024 •
edited
Loading

Tests Passed

@shuding shuding merged commit 52ae1f8 into canary Apr 12, 2024
80 checks passed
@shuding shuding deleted the shu/fs6b branch April 12, 2024 22:03
ztanner pushed a commit that referenced this pull request Apr 17, 2024
@shuding @ztanner
Improve rendering performance (#64408)

Verified

This commit was signed with the committer’s verified signature.
rickhanlonii Ricky
GPG key ID: DF41506EFF2FAF6A
Verified
Learn about vigilant mode
c262f38 
This PR improves the server rendering performance a bit, especially for
the App Router. It has mainly 2 changes.

A rough benchmark test with a 300kB Lorem Ipsum page, SSR is about 18%
faster.

### Improve `getServerInsertedHTML`

- Avoid an extra `renderToReadableStream` if we already know the content
will be empty
- Avoid `await stream.allReady` for better parallelism with
`streamToString()`
- Increase the `progressiveChunkSize`

### Improve `createHeadInsertionTransformStream`

- Only do chunk splitting and enqueuing if the inserted string is not
empty
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@feedthejim feedthejim

@huozhi huozhi

@timneutkens timneutkens

@ijjk ijjk

@ztanner ztanner

@wyattjoh wyattjoh

Assignees
No one assigned
Labels
created-by: Next.js team PRs by the Next.js team. locked type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@shuding @ijjk @huozhi @feedthejim