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.

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

Optimize next-app-loader resolving speed #50745

Merged
merged 6 commits into from Jun 5, 2023

Conversation

timneutkens
Copy link
Member

@timneutkens timneutkens commented Jun 3, 2023

What?

We recently implemented an optimized resolving method for app in Turbopack, this ports some of the main changes in that resolving logic to optimize next-app-loader which during compilation resolves the tree structure that we use to render in app-render.tsx.

Here's the results for a page that is nested a few levels deep on vercel.com using App Router. These results only cover next-app-loader, not any modules compiled below it.

Before

CleanShot 2023-06-03 at 22 36 26@2x

After

CleanShot 2023-06-03 at 22 55 10@2x

Raw numbers

Before After Delta Delta (percent)
1.620 ms 76.39 ms -1.543.61 ms -95.2%

How?

Changed the resolving logic to use fileExists, looping over the provided pageExtensions.
For Turbopack we have a process that does only one pass for generating all trees. That also only reads directories instead of checking individual files, which is even better (<5ms for generating all possible trees) but this PR is a quick win that has a big impact already without refactoring the entire entries generation in webpack.

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team type: next labels Jun 3, 2023
@timneutkens
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@ijjk
Copy link
Member

ijjk commented Jun 4, 2023

Failing test suites

Commit: 08e0d63

pnpm testheadless test/development/basic/node-builtins.test.ts

  • node builtins > should have polyfilled node.js builtins for the browser correctly in client component
Expand output

● node builtins › should have polyfilled node.js builtins for the browser correctly in client component

page.waitForFunction: Timeout 60000ms exceeded.

  362 |   waitForCondition(condition, timeout) {
  363 |     return this.chain(() => {
> 364 |       return page.waitForFunction(condition, { timeout })
      |                   ^
  365 |     })
  366 |   }
  367 |

  at lib/browsers/playwright.ts:364:19

Read more about building and testing Next.js in contributing.md.

@kodiakhq kodiakhq bot merged commit dd71479 into canary Jun 5, 2023
28 of 29 checks passed
@kodiakhq kodiakhq bot deleted the 06-03-Optimize_next-app-loader_resolving_speed branch June 5, 2023 06:51
hydRAnger pushed a commit to hydRAnger/next.js that referenced this pull request Jun 12, 2023
## What?

We recently implemented an optimized resolving method for `app` in Turbopack, this ports some of the main changes in that resolving logic to optimize `next-app-loader` which during compilation resolves the tree structure that we use to render in `app-render.tsx`.

Here's the results for a page that is nested a few levels deep on vercel.com using App Router. These results only cover `next-app-loader`, not any modules compiled below it.

### Before

<img width="671" alt="CleanShot 2023-06-03 at 22 36 26@2x" src="https://github.com/vercel/next.js/assets/6324199/0edeb060-2460-4a7d-95a7-1c22ea26a065">

### After

<img width="673" alt="CleanShot 2023-06-03 at 22 55 10@2x" src="https://github.com/vercel/next.js/assets/6324199/f40964fc-b169-4d95-8711-73cbff3ec76a">


## Raw numbers

<table>
<tr>
 <td>Before</td>
 <td>After</td>
 <td>Delta</td>
 <td>Delta (percent)</td>
</tr>
<tr>
 <td>1.620 ms</td>
 <td>76.39 ms</td>
 <td>-1.543.61 ms</td>
  <td>-95.2%</td>
</tr>
</table>

## How?

Changed the resolving logic to use `fileExists`, looping over the provided pageExtensions.
For Turbopack we have a process that does only one pass for generating all trees. That also only reads directories instead of checking individual files, which is even better (<5ms for generating all possible trees) but this PR is a quick win that has a big impact already without refactoring the entire entries generation in webpack.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants