Skip to content

Commit

Permalink
Optimize next-app-loader file resolution (#51924)
Browse files Browse the repository at this point in the history
This PR changes `fs.stat` calls (to check if a file exists or not) to be
aggregated as `fs.readdir` calls and cached during the entire loader
pass. This is because we have a huge amount of file to check, but they
are always in a small amount of directories.

For example, a route that's 5-directory deep (`/a/b/c/d/page.js`) can
have up to 4,000 special segments and metadata files to check. However
they're always under these 5 directories. So instead of 4,000 `fs.stat`
calls let's just do 5 `fs.readdir` calls.

Didn't measure it carefully but a quick local test shows a 20~30%
improvement.

<details>
<summary>Another idea for future improvements</summary>
Currently, we create a MissingModuleDependency for any combination of
possible metadata filename. But in reality, we only need to track them
incrementally by index. For example if `icon1.js` is missing, it's kinda
waste to still track `icon2.js`~`icon9.js` because `icon1.js` will be
added first. This change should at least reduce the number of file
watchers by 80% and the initial compilation time by 0.5~1s, depending on
the actual route.
</details>

---------

Co-authored-by: Tim Neutkens <[email protected]>
  • Loading branch information
shuding and timneutkens authored Jun 29, 2023
1 parent b236670 commit 922a458
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 20 deletions.
19 changes: 10 additions & 9 deletions packages/next/src/build/webpack/loaders/metadata/discover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ async function enumMetadataFiles(
: []
)
for (const name of possibleFileNames) {
const resolved = await metadataResolver(path.join(dir, name), extensions)
const resolved = await metadataResolver(dir, name, extensions)
if (resolved) {
collectedFiles.push(resolved)
}
Expand Down Expand Up @@ -123,14 +123,15 @@ export async function createStaticMetadataFromRoute(
})
}

await Promise.all([
collectIconModuleIfExists('icon'),
collectIconModuleIfExists('apple'),
collectIconModuleIfExists('openGraph'),
collectIconModuleIfExists('twitter'),
isRootLayoutOrRootPage && collectIconModuleIfExists('favicon'),
isRootLayoutOrRootPage && collectIconModuleIfExists('manifest'),
])
// Intentially make these serial to reuse directory access cache.
await collectIconModuleIfExists('icon')
await collectIconModuleIfExists('apple')
await collectIconModuleIfExists('openGraph')
await collectIconModuleIfExists('twitter')
if (isRootLayoutOrRootPage) {
await collectIconModuleIfExists('favicon')
await collectIconModuleIfExists('manifest')
}

return hasStaticMetadataFiles ? staticImagesMetadata : null
}
Expand Down
54 changes: 43 additions & 11 deletions packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { AppPathnameNormalizer } from '../../../server/future/normalizers/built/
import { RouteKind } from '../../../server/future/route-kind'
import { AppRouteRouteModuleOptions } from '../../../server/future/route-modules/app-route/module'
import { AppBundlePathNormalizer } from '../../../server/future/normalizers/built/app/app-bundle-path-normalizer'
import { FileType, fileExists } from '../../../lib/file-exists'
import { MiddlewareConfig } from '../../analysis/get-page-static-info'

export type AppLoaderOptions = {
Expand Down Expand Up @@ -59,7 +58,8 @@ type PathResolver = (
pathname: string
) => Promise<string | undefined> | string | undefined
export type MetadataResolver = (
pathname: string,
dir: string,
filename: string,
extensions: readonly string[]
) => Promise<string | undefined>

Expand Down Expand Up @@ -511,16 +511,45 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {
return createAbsolutePath(appDir, pathToResolve)
}

// Cached checker to see if a file exists in a given directory.
// This can be more efficient than checking them with `fs.stat` one by one
// because all the thousands of files are likely in a few possible directories.
// Note that it should only be cached for this compilation, not globally.
const filesInDir = new Map<string, Set<string>>()
const fileExistsInDirectory = async (dirname: string, fileName: string) => {
const existingFiles = filesInDir.get(dirname)
if (existingFiles) {
return existingFiles.has(fileName)
}
try {
const files = await fs.readdir(dirname, { withFileTypes: true })
const fileNames = new Set<string>()
for (const file of files) {
if (file.isFile()) {
fileNames.add(file.name)
}
}
filesInDir.set(dirname, fileNames)
return fileNames.has(fileName)
} catch (err) {
return false
}
}

const resolver: PathResolver = async (pathname) => {
const absolutePath = createAbsolutePath(appDir, pathname)

const filenameIndex = absolutePath.lastIndexOf(path.sep)
const dirname = absolutePath.slice(0, filenameIndex)
const filename = absolutePath.slice(filenameIndex + 1)

let result: string | undefined

for (const ext of extensions) {
const absolutePathWithExtension = `${absolutePath}${ext}`
if (
!result &&
(await fileExists(absolutePathWithExtension, FileType.File))
(await fileExistsInDirectory(dirname, `${filename}${ext}`))
) {
result = absolutePathWithExtension
}
Expand All @@ -532,18 +561,20 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {
return result
}

const metadataResolver: MetadataResolver = async (pathname, exts) => {
const absolutePath = createAbsolutePath(appDir, pathname)
const metadataResolver: MetadataResolver = async (
dirname,
filename,
exts
) => {
const absoluteDir = createAbsolutePath(appDir, dirname)

let result: string | undefined

for (const ext of exts) {
// Compared to `resolver` above the exts do not have the `.` included already, so it's added here.
const absolutePathWithExtension = `${absolutePath}.${ext}`
if (
!result &&
(await fileExists(absolutePathWithExtension, FileType.File))
) {
const filenameWithExt = `${filename}.${ext}`
const absolutePathWithExtension = `${absoluteDir}${path.sep}${filenameWithExt}`
if (!result && (await fileExistsInDirectory(dirname, filenameWithExt))) {
result = absolutePathWithExtension
}
// Call `addMissingDependency` for all files even if they didn't match,
Expand Down Expand Up @@ -611,7 +642,8 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {
throw new Error(message)
}

// Get the new result with the created root layout.
// Clear fs cache, get the new result with the created root layout.
filesInDir.clear()
treeCodeResult = await createTreeCodeFromPath(pagePath, {
resolveDir,
resolver,
Expand Down

0 comments on commit 922a458

Please sign in to comment.