Skip to content

Commit

Permalink
Optimize next-app-loader resolving speed (vercel#50745)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
timneutkens authored and hydRAnger committed Jun 12, 2023
1 parent 4b1ecf6 commit cc22694
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 116 deletions.
4 changes: 2 additions & 2 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
PAGES_DIR_ALIAS,
INSTRUMENTATION_HOOK_FILENAME,
} from '../lib/constants'
import { fileExists } from '../lib/file-exists'
import { FileType, fileExists } from '../lib/file-exists'
import { findPagesDir } from '../lib/find-pages-dir'
import loadCustomRoutes, {
CustomRoutes,
Expand Down Expand Up @@ -588,7 +588,7 @@ export default async function build(
for (const page in mappedPages) {
const hasPublicPageFile = await fileExists(
path.join(publicDir, page === '/' ? '/index' : page),
'file'
FileType.File
)
if (hasPublicPageFile) {
conflictingPublicFiles.push(page)
Expand Down
41 changes: 11 additions & 30 deletions packages/next/src/build/webpack/loaders/metadata/discover.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type webpack from 'webpack'
import type {
CollectingMetadata,
PossibleStaticMetadataFileNameConvention,
Expand All @@ -7,6 +6,7 @@ import path from 'path'
import { stringify } from 'querystring'
import { STATIC_METADATA_IMAGES } from '../../../../lib/metadata/is-metadata-route'
import { WEBPACK_RESOURCE_QUERIES } from '../../../../lib/constants'
import { MetadataResolver } from '../next-app-loader'

const METADATA_TYPE = 'metadata'

Expand All @@ -16,16 +16,14 @@ async function enumMetadataFiles(
filename: string,
extensions: readonly string[],
{
resolvePath,
loaderContext,
metadataResolver,
// When set to true, possible filename without extension could: icon, icon0, ..., icon9
numericSuffix,
}: {
resolvePath: (pathname: string) => Promise<string>
loaderContext: webpack.LoaderContext<any>
metadataResolver: MetadataResolver
numericSuffix: boolean
}
) {
): Promise<string[]> {
const collectedFiles: string[] = []

const possibleFileNames = [filename].concat(
Expand All @@ -36,19 +34,9 @@ async function enumMetadataFiles(
: []
)
for (const name of possibleFileNames) {
for (const ext of extensions) {
const pathname = path.join(dir, `${name}.${ext}`)
try {
const resolved = await resolvePath(pathname)
loaderContext.addDependency(resolved)

collectedFiles.push(resolved)
} catch (err: any) {
if (!err.message.includes("Can't resolve")) {
throw err
}
loaderContext.addMissingDependency(pathname)
}
const resolved = await metadataResolver(path.join(dir, name), extensions)
if (resolved) {
collectedFiles.push(resolved)
}
}

Expand All @@ -59,16 +47,14 @@ export async function createStaticMetadataFromRoute(
resolvedDir: string,
{
segment,
resolvePath,
metadataResolver,
isRootLayoutOrRootPage,
loaderContext,
pageExtensions,
basePath,
}: {
segment: string
resolvePath: (pathname: string) => Promise<string>
metadataResolver: MetadataResolver
isRootLayoutOrRootPage: boolean
loaderContext: webpack.LoaderContext<any>
pageExtensions: string[]
basePath: string
}
Expand All @@ -82,11 +68,6 @@ export async function createStaticMetadataFromRoute(
manifest: undefined,
}

const opts = {
resolvePath,
loaderContext,
}

async function collectIconModuleIfExists(
type: PossibleStaticMetadataFileNameConvention
) {
Expand All @@ -96,7 +77,7 @@ export async function createStaticMetadataFromRoute(
resolvedDir,
'manifest',
staticManifestExtension.concat(pageExtensions),
{ ...opts, numericSuffix: false }
{ metadataResolver, numericSuffix: false }
)
if (manifestFile.length > 0) {
hasStaticMetadataFiles = true
Expand All @@ -116,7 +97,7 @@ export async function createStaticMetadataFromRoute(
...STATIC_METADATA_IMAGES[type].extensions,
...(type === 'favicon' ? [] : pageExtensions),
],
{ ...opts, numericSuffix: true }
{ metadataResolver, numericSuffix: true }
)
resolvedMetadataFiles
.sort((a, b) => a.localeCompare(b))
Expand Down

0 comments on commit cc22694

Please sign in to comment.