Skip to content

Commit

Permalink
File Reader Improvements (#54645)
Browse files Browse the repository at this point in the history
This replaces the existing recursive directory reading beheviour with a more efficient implementation. Rather than previously doing actual recursion with the function calls to go deeper into each directory, this has been rewritten to instead use a while loop and a stack. This should improve memory usage for projects with very deep directory structures. 

The updated design that performs directory scanning on all directories found at each layer, allowing more filesystem calls to be sent to the OS instead of waiting like the previous implementation did.

**Currently the new implementation is about 3x faster.**
  • Loading branch information
wyattjoh committed Aug 28, 2023
1 parent 989233e commit 0f82237
Show file tree
Hide file tree
Showing 21 changed files with 332 additions and 276 deletions.
1 change: 0 additions & 1 deletion bench/readdir/.gitignore

This file was deleted.

4 changes: 0 additions & 4 deletions bench/readdir/create-fixtures.sh

This file was deleted.

24 changes: 0 additions & 24 deletions bench/readdir/glob.js

This file was deleted.

21 changes: 0 additions & 21 deletions bench/readdir/recursive-readdir.js

This file was deleted.

20 changes: 9 additions & 11 deletions packages/next/src/build/index.ts
Expand Up @@ -448,11 +448,11 @@ export default async function build(

const pagesPaths =
!appDirOnly && pagesDir
? await nextBuildSpan
.traceChild('collect-pages')
.traceAsyncFn(() =>
recursiveReadDir(pagesDir, validFileMatcher.isPageFile)
)
? await nextBuildSpan.traceChild('collect-pages').traceAsyncFn(() =>
recursiveReadDir(pagesDir, {
pathnameFilter: validFileMatcher.isPageFile,
})
)
: []

const middlewareDetectionRegExp = new RegExp(
Expand Down Expand Up @@ -512,16 +512,14 @@ export default async function build(
const appPaths = await nextBuildSpan
.traceChild('collect-app-paths')
.traceAsyncFn(() =>
recursiveReadDir(
appDir,
(absolutePath) =>
recursiveReadDir(appDir, {
pathnameFilter: (absolutePath) =>
validFileMatcher.isAppRouterPage(absolutePath) ||
// For now we only collect the root /not-found page in the app
// directory as the 404 fallback
validFileMatcher.isRootNotFound(absolutePath),
undefined,
(part) => part.startsWith('_')
)
ignorePartFilter: (part) => part.startsWith('_'),
})
)

mappedAppPages = nextBuildSpan
Expand Down
221 changes: 173 additions & 48 deletions packages/next/src/lib/recursive-readdir.ts
@@ -1,59 +1,184 @@
import { Dirent, promises } from 'fs'
import { join } from 'path'
import fs from 'fs/promises'
import path from 'path'

type Filter = (pathname: string) => boolean

type Result = {
directories: string[]
pathnames: string[]
links: string[]
}

export type RecursiveReadDirOptions = {
/**
* Filter to ignore files with absolute pathnames, false to ignore.
*/
pathnameFilter?: Filter

/**
* Filter to ignore files and directories with absolute pathnames, false to
* ignore.
*/
ignoreFilter?: Filter

/**
* Filter to ignore files and directories with the pathname part, false to
* ignore.
*/
ignorePartFilter?: Filter

/**
* Whether to sort the results, true by default.
*/
sortPathnames?: boolean

/**
* Whether to return relative pathnames, true by default.
*/
relativePathnames?: boolean
}

/**
* Recursively read directory
* Returns array holding all relative paths
* Recursively reads a directory and returns the list of pathnames.
*
* @param rootDirectory the directory to read
* @param options options to control the behavior of the recursive read
* @returns the list of pathnames
*/
export async function recursiveReadDir(
/** Directory to read */
dir: string,
/** Filter for the file path */
filter: (absoluteFilePath: string) => boolean,
/** Filter for the file path */
ignore?: (absoluteFilePath: string) => boolean,
ignorePart?: (partName: string) => boolean,
/** This doesn't have to be provided, it's used for the recursion */
arr: string[] = [],
/** Used to replace the initial path, only the relative path is left, it's faster than path.relative. */
rootDir: string = dir
rootDirectory: string,
options: RecursiveReadDirOptions = {}
): Promise<string[]> {
const result = await promises.readdir(dir, { withFileTypes: true })

await Promise.all(
result.map(async (part: Dirent) => {
const absolutePath = join(dir, part.name)
const relativePath = absolutePath.replace(rootDir, '')
if (ignore && ignore(absolutePath)) return
if (ignorePart && ignorePart(part.name)) return

// readdir does not follow symbolic links
// if part is a symbolic link, follow it using stat
let isDirectory = part.isDirectory()
if (part.isSymbolicLink()) {
const stats = await promises.stat(absolutePath)
isDirectory = stats.isDirectory()
}
// Grab our options.
const {
pathnameFilter,
ignoreFilter,
ignorePartFilter,
sortPathnames = true,
relativePathnames = true,
} = options

if (isDirectory) {
await recursiveReadDir(
absolutePath,
filter,
ignore,
ignorePart,
arr,
rootDir
)
return
}
// The list of pathnames to return.
const pathnames: string[] = []

/**
* Coerces the pathname to be relative if requested.
*/
const coerce = relativePathnames
? (pathname: string) => pathname.replace(rootDirectory, '')
: (pathname: string) => pathname

// The queue of directories to scan.
let directories: string[] = [rootDirectory]

while (directories.length > 0) {
// Load all the files in each directory at the same time.
const results = await Promise.all(
directories.map(async (directory) => {
const result: Result = { directories: [], pathnames: [], links: [] }

try {
const dir = await fs.readdir(directory, { withFileTypes: true })
for (const file of dir) {
// If enabled, ignore the file if it matches the ignore filter.
if (ignorePartFilter && ignorePartFilter(file.name)) {
continue
}

// Handle each file.
const absolutePathname = path.join(directory, file.name)

// If enabled, ignore the file if it matches the ignore filter.
if (ignoreFilter && ignoreFilter(absolutePathname)) {
continue
}

// If the file is a directory, then add it to the list of directories,
// they'll be scanned on a later pass.
if (file.isDirectory()) {
result.directories.push(absolutePathname)
} else if (file.isSymbolicLink()) {
result.links.push(absolutePathname)
} else if (!pathnameFilter || pathnameFilter(absolutePathname)) {
result.pathnames.push(coerce(absolutePathname))
}
}
} catch (err: any) {
// This can only happen when the underlying directory was removed. If
// anything other than this error occurs, re-throw it.
// if (err.code !== 'ENOENT') throw err
if (err.code !== 'ENOENT' || directory === rootDirectory) throw err

// The error occurred, so abandon reading this directory.
return null
}

return result
})
)

// Empty the directories, we'll fill it later if some of the files are
// directories.
directories = []

// Keep track of any symbolic links we find, we'll resolve them later.
const links = []

// For each result of directory scans...
for (const result of results) {
// If the directory was removed, then skip it.
if (!result) continue

// Add any directories to the list of directories to scan.
directories.push(...result.directories)

// Add any symbolic links to the list of symbolic links to resolve.
links.push(...result.links)

// Add any file pathnames to the list of pathnames.
pathnames.push(...result.pathnames)
}

// Resolve all the symbolic links we found if any.
if (links.length > 0) {
const resolved = await Promise.all(
links.map(async (absolutePathname) => {
try {
return await fs.stat(absolutePathname)
} catch (err: any) {
// This can only happen when the underlying link was removed. If
// anything other than this error occurs, re-throw it.
if (err.code !== 'ENOENT') throw err

// The error occurred, so abandon reading this directory.
return null
}
})
)

for (let i = 0; i < links.length; i++) {
const stats = resolved[i]

// If the link was removed, then skip it.
if (!stats) continue

// We would have already ignored the file if it matched the ignore
// filter, so we don't need to check it again.
const absolutePathname = links[i]

if (!filter(absolutePath)) {
return
if (stats.isDirectory()) {
directories.push(absolutePathname)
} else if (!pathnameFilter || pathnameFilter(absolutePathname)) {
pathnames.push(coerce(absolutePathname))
}
}
}
}

arr.push(relativePath)
})
)
// Sort the pathnames in place if requested.
if (sortPathnames) {
pathnames.sort()
}

return arr.sort()
return pathnames
}
9 changes: 4 additions & 5 deletions packages/next/src/lib/typescript/getTypeScriptIntent.ts
Expand Up @@ -32,11 +32,10 @@ export async function getTypeScriptIntent(
const tsFilesRegex = /.*\.(ts|tsx)$/
const excludedRegex = /(node_modules|.*\.d\.ts$)/
for (const dir of intentDirs) {
const typescriptFiles = await recursiveReadDir(
dir,
(name) => tsFilesRegex.test(name),
(name) => excludedRegex.test(name)
)
const typescriptFiles = await recursiveReadDir(dir, {
pathnameFilter: (name) => tsFilesRegex.test(name),
ignoreFilter: (name) => excludedRegex.test(name),
})
if (typescriptFiles.length) {
return { firstTimeSetup: true }
}
Expand Down
8 changes: 8 additions & 0 deletions packages/next/src/lib/wait.ts
@@ -0,0 +1,8 @@
/**
* Wait for a given number of milliseconds and then resolve.
*
* @param ms the number of milliseconds to wait
*/
export async function wait(ms: number) {
return new Promise((resolve) => setTimeout(resolve, ms))
}
22 changes: 20 additions & 2 deletions packages/next/src/server/dev/next-dev-server.ts
Expand Up @@ -54,7 +54,7 @@ import { DevAppPageRouteMatcherProvider } from '../future/route-matcher-provider
import { DevAppRouteRouteMatcherProvider } from '../future/route-matcher-providers/dev/dev-app-route-route-matcher-provider'
import { PagesManifest } from '../../build/webpack/plugins/pages-manifest-plugin'
import { NodeManifestLoader } from '../future/route-matcher-providers/helpers/manifest-loaders/node-manifest-loader'
import { CachedFileReader } from '../future/route-matcher-providers/dev/helpers/file-reader/cached-file-reader'
import { BatchedFileReader } from '../future/route-matcher-providers/dev/helpers/file-reader/batched-file-reader'
import { DefaultFileReader } from '../future/route-matcher-providers/dev/helpers/file-reader/default-file-reader'
import { NextBuildContext } from '../../build/build-context'
import { IncrementalCache } from '../lib/incremental-cache'
Expand Down Expand Up @@ -208,10 +208,17 @@ export default class DevServer extends Server {
this.dir
)
const extensions = this.nextConfig.pageExtensions
const fileReader = new CachedFileReader(new DefaultFileReader())
const extensionsExpression = new RegExp(`\\.(?:${extensions.join('|')})$`)

// If the pages directory is available, then configure those matchers.
if (pagesDir) {
const fileReader = new BatchedFileReader(
new DefaultFileReader({
// Only allow files that have the correct extensions.
pathnameFilter: (pathname) => extensionsExpression.test(pathname),
})
)

matchers.push(
new DevPagesRouteMatcherProvider(
pagesDir,
Expand All @@ -231,6 +238,17 @@ export default class DevServer extends Server {
}

if (appDir) {
// We create a new file reader for the app directory because we don't want
// to include any folders or files starting with an underscore. This will
// prevent the reader from wasting time reading files that we know we
// don't care about.
const fileReader = new BatchedFileReader(
new DefaultFileReader({
// Ignore any directory prefixed with an underscore.
ignorePartFilter: (part) => part.startsWith('_'),
})
)

matchers.push(
new DevAppPageRouteMatcherProvider(appDir, extensions, fileReader)
)
Expand Down

0 comments on commit 0f82237

Please sign in to comment.