Consolidate Server and Routing process into one process (#53523)

In the current version of Next.js there are 4 processes when running in production: - Server - Routing - Rendering Pages Router - Rendering App Router This setup was introduced in order to allow App Router and Pages Router to use different versions of React (i.e. Server Actions currently requires react@experimental to function). I wrote down more on why these processes exist in this comment: #49929 (comment) This PR combines the Server and Routing process into one handler, as the "Server" process was only proxying to the Routing process. In my testing this caused about ~2x the amount of memory per request as the response body was duplicated between the processes. This was especially visible in the case of that memory leak in Node.js 18.16 as it grew memory usage on both sides quickly. In the process of going through these changes I found a couple of other bugs like the propagation of values to the worker processes not being awaited ([link](https://github.com/vercel/next.js/pull/53523/files#diff-0ef09f360141930bb03263b378d37d71ad9432ac851679aeabc577923536df84R54)) and the dot syntax for propagation was not functioning. It also seemed there were a few cases where watchpack was used that would cause many more files to be watched than expected, for now I've removed those cases, specifically the "delete dir while running" and instrument.js hmr (instrument.js is experimental). Those tests have been skipped for now until we can revisit them to verfiy it I've also cleaned up the types a bit while I was looking into these changes. ### Improvement ⚠️ Important preface to this, measuring memory usage / peak usage is not super reliable especially when JS gets garbage collected. These numbers are just to show the rough percentage of less memory usage. #### Baseline Old: ``` next-server: 44.8MB next-router-worker: 57.5MB next-render-worker-app: 39,6MB next-render-worker-pages: 39,1MB ``` New: ``` next-server: Removed next-router-worker: 64.4MB next-render-worker-app: 43.1MB (Note: no changes here, this shows what I meant by rough numbers) next-render-worker-pages: 42.4MB (Note: no changes here, this shows what I meant by rough numbers) ``` Overall win: ~40MB (process is removed) #### Peak usage Old: ``` next-server: 118.6MB next-router-worker: 223.7MB next-render-worker-app: 32.8MB (I ran the test on a pages application in this case) next-render-worker-pages: 101.1MB ``` New: ``` next-server: Removed next-router-worker: 179.1MB next-render-worker-app: 33.4MB next-render-worker-pages: 117.5MB ``` Overall win: ~100MB (but it scales with requests so it was about ~50% of next-router-worker constantly) Related: #53523 --------- Co-authored-by: JJ Kasper <jj@jjsweb.site>
vercel · Aug 8, 2023 · c5a8e09 · c5a8e09
1 parent fef6f82
commit c5a8e09
Show file tree

Hide file tree

Showing 21 changed files with 452 additions and 972 deletions.
diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts
diff --git a/packages/next/src/cli/next-start.ts b/packages/next/src/cli/next-start.ts
@@ -5,9 +5,6 @@ import { startServer } from '../server/lib/start-server'
 import { getPort, printAndExit } from '../server/lib/utils'
 import { getProjectDir } from '../lib/get-project-dir'
 import { CliCommand } from '../lib/commands'
-import { resolve } from 'path'
-import { PHASE_PRODUCTION_SERVER } from '../shared/lib/constants'
-import loadConfig from '../server/config'
 import { getValidatedArgs } from '../lib/get-validated-args'
 
 const nextStart: CliCommand = async (argv) => {
@@ -66,22 +63,12 @@ const nextStart: CliCommand = async (argv) => {
     ? Math.ceil(keepAliveTimeoutArg)
     : undefined
 
-  const config = await loadConfig(
-    PHASE_PRODUCTION_SERVER,
-    resolve(dir || '.'),
-    undefined,
-    undefined,
-    true
-  )
-
   await startServer({
     dir,
-    nextConfig: config,
     isDev: false,
     hostname: host,
     port,
     keepAliveTimeout,
-    useWorkers: !!config.experimental.appDir,
   })
 }
 

diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts
@@ -468,6 +468,10 @@ export default abstract class Server<ServerOptions extends Options = Options> {
     this.responseCache = this.getResponseCache({ dev })
   }
 
+  protected reloadMatchers() {
+    return this.matchers.reload()
+  }
+
   protected async normalizeNextData(
     _req: BaseNextRequest,
     _res: BaseNextResponse,

diff --git a/packages/next/src/server/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts
@@ -87,7 +87,6 @@ export interface Options extends ServerOptions {
 export default class DevServer extends Server {
   private devReady: Promise<void>
   private setDevReady?: Function
-  private webpackWatcher?: any | null
   protected sortedRoutes?: string[]
   private pagesDir?: string
   private appDir?: string
@@ -247,15 +246,6 @@ export default class DevServer extends Server {
     return 'development'
   }
 
-  async stopWatcher(): Promise<void> {
-    if (!this.webpackWatcher) {
-      return
-    }
-
-    this.webpackWatcher.close()
-    this.webpackWatcher = null
-  }
-
   protected async prepareImpl(): Promise<void> {
     setGlobal('distDir', this.distDir)
     setGlobal('phase', PHASE_DEVELOPMENT_SERVER)

diff --git a/packages/next/src/server/lib/is-node-debugging.ts b/packages/next/src/server/lib/is-node-debugging.ts
@@ -0,0 +1,14 @@
+export function checkIsNodeDebugging() {
+  let isNodeDebugging: 'brk' | boolean = !!(
+    process.execArgv.some((localArg) => localArg.startsWith('--inspect')) ||
+    process.env.NODE_OPTIONS?.match?.(/--inspect(=\S+)?( |$)/)
+  )
+
+  if (
+    process.execArgv.some((localArg) => localArg.startsWith('--inspect-brk')) ||
+    process.env.NODE_OPTIONS?.match?.(/--inspect-brk(=\S+)?( |$)/)
+  ) {
+    isNodeDebugging = 'brk'
+  }
+  return isNodeDebugging
+}
diff --git a/packages/next/src/server/lib/render-server.ts b/packages/next/src/server/lib/render-server.ts
@@ -3,6 +3,7 @@ import type { RequestHandler } from '../next'
 // this must come first as it includes require hooks
 import { initializeServerWorker } from './setup-server-worker'
 import next from '../next'
+import { PropagateToWorkersField } from './router-utils/types'
 
 export const WORKER_SELF_EXIT_CODE = 77
 
@@ -39,26 +40,18 @@ export function deleteCache(filePaths: string[]) {
   }
 }
 
-export async function propagateServerField(field: string, value: any) {
+export async function propagateServerField(
+  field: PropagateToWorkersField,
+  value: any
+) {
   if (!app) {
     throw new Error('Invariant cant propagate server field, no app initialized')
   }
   let appField = (app as any).server
 
-  if (field.includes('.')) {
-    const parts = field.split('.')
-
-    for (let i = 0; i < parts.length - 1; i++) {
-      if (appField) {
-        appField = appField[parts[i]]
-      }
-    }
-    field = parts[parts.length - 1]
-  }
-
   if (appField) {
     if (typeof appField[field] === 'function') {
-      appField[field].apply(
+      await appField[field].apply(
         (app as any).server,
         Array.isArray(value) ? value : []
       )

diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts
@@ -1,7 +1,10 @@
 import type { IncomingMessage } from 'http'
 
 // this must come first as it includes require hooks
-import { initializeServerWorker } from './setup-server-worker'
+import type {
+  WorkerRequestHandler,
+  WorkerUpgradeHandler,
+} from './setup-server-worker'
 
 import url from 'url'
 import path from 'path'
@@ -24,6 +27,7 @@ import { getResolveRoutes } from './router-utils/resolve-routes'
 import { NextUrlWithParsedQuery, getRequestMeta } from '../request-meta'
 import { pathHasPrefix } from '../../shared/lib/router/utils/path-has-prefix'
 import { removePathPrefix } from '../../shared/lib/router/utils/remove-path-prefix'
+import setupCompression from 'next/dist/compiled/compression'
 
 import {
   PHASE_PRODUCTION_SERVER,
@@ -32,13 +36,6 @@ import {
 } from '../../shared/lib/constants'
 import { signalFromNodeResponse } from '../web/spec-extension/adapters/next-request'
 
-let initializeResult:
-  | undefined
-  | {
-      port: number
-      hostname: string
-    }
-
 const debug = setupDebug('next:router-server:main')
 
 export type RenderWorker = InstanceType<
@@ -51,6 +48,13 @@ export type RenderWorker = InstanceType<
   propagateServerField: typeof import('./render-server').propagateServerField
 }
 
+const nextDeleteCacheCallbacks: Array<(filePaths: string[]) => Promise<any>> =
+  []
+const nextDeleteAppClientCacheCallbacks: Array<() => Promise<any>> = []
+const nextClearModuleContextCallbacks: Array<
+  (targetPath: string) => Promise<any>
+> = []
+
 export async function initialize(opts: {
   dir: string
   port: number
@@ -61,10 +65,7 @@ export async function initialize(opts: {
   isNodeDebugging: boolean
   keepAliveTimeout?: number
   customServer?: boolean
-}): Promise<NonNullable<typeof initializeResult>> {
-  if (initializeResult) {
-    return initializeResult
-  }
+}): Promise<[WorkerRequestHandler, WorkerUpgradeHandler]> {
   process.title = 'next-router-worker'
 
   if (!process.env.NODE_ENV) {
@@ -80,6 +81,12 @@ export async function initialize(opts: {
     true
   )
 
+  let compress: ReturnType<typeof setupCompression> | undefined
+
+  if (config?.compress !== false) {
+    compress = setupCompression()
+  }
+
   const fsChecker = await setupFsCheck({
     dev: opts.dev,
     dir: opts.dir,
@@ -207,39 +214,57 @@ export async function initialize(opts: {
   )
 
   // pre-initialize workers
-  await renderWorkers.app?.initialize(renderWorkerOpts)
-  await renderWorkers.pages?.initialize(renderWorkerOpts)
+  const initialized = {
+    app: await renderWorkers.app?.initialize(renderWorkerOpts),
+    pages: await renderWorkers.pages?.initialize(renderWorkerOpts),
+  }
 
   if (devInstance) {
     Object.assign(devInstance.renderWorkers, renderWorkers)
+
+    nextDeleteCacheCallbacks.push((filePaths: string[]) =>
+      Promise.all([
+        renderWorkers.pages?.deleteCache(filePaths),
+        renderWorkers.app?.deleteCache(filePaths),
+      ])
+    )
+    nextDeleteAppClientCacheCallbacks.push(() =>
+      Promise.all([
+        renderWorkers.pages?.deleteAppClientCache(),
+        renderWorkers.app?.deleteAppClientCache(),
+      ])
+    )
+    nextClearModuleContextCallbacks.push((targetPath: string) =>
+      Promise.all([
+        renderWorkers.pages?.clearModuleContext(targetPath),
+        renderWorkers.app?.clearModuleContext(targetPath),
+      ])
+    )
     ;(global as any)._nextDeleteCache = async (filePaths: string[]) => {
-      try {
-        await Promise.all([
-          renderWorkers.pages?.deleteCache(filePaths),
-          renderWorkers.app?.deleteCache(filePaths),
-        ])
-      } catch (err) {
-        console.error(err)
+      for (const cb of nextDeleteCacheCallbacks) {
+        try {
+          await cb(filePaths)
+        } catch (err) {
+          console.error(err)
+        }
       }
     }
     ;(global as any)._nextDeleteAppClientCache = async () => {
-      try {
-        await Promise.all([
-          renderWorkers.pages?.deleteAppClientCache(),
-          renderWorkers.app?.deleteAppClientCache(),
-        ])
-      } catch (err) {
-        console.error(err)
+      for (const cb of nextDeleteAppClientCacheCallbacks) {
+        try {
+          await cb()
+        } catch (err) {
+          console.error(err)
+        }
       }
     }
     ;(global as any)._nextClearModuleContext = async (targetPath: string) => {
-      try {
-        await Promise.all([
-          renderWorkers.pages?.clearModuleContext(targetPath),
-          renderWorkers.app?.clearModuleContext(targetPath),
-        ])
-      } catch (err) {
-        console.error(err)
+      for (const cb of nextClearModuleContextCallbacks) {
+        try {
+          await cb(targetPath)
+        } catch (err) {
+          console.error(err)
+        }
       }
     }
   }
@@ -274,10 +299,11 @@ export async function initialize(opts: {
     devInstance?.ensureMiddleware
   )
 
-  const requestHandler: Parameters<typeof initializeServerWorker>[0] = async (
-    req,
-    res
-  ) => {
+  const requestHandler: WorkerRequestHandler = async (req, res) => {
+    if (compress) {
+      // @ts-expect-error not express req/res
+      compress(req, res, () => {})
+    }
     req.on('error', (_err) => {
       // TODO: log socket errors?
     })
@@ -319,8 +345,7 @@ export async function initialize(opts: {
         return null
       }
 
-      const curWorker = renderWorkers[type]
-      const workerResult = await curWorker?.initialize(renderWorkerOpts)
+      const workerResult = initialized[type]
 
       if (!workerResult) {
         throw new Error(`Failed to initialize render worker ${type}`)
@@ -690,11 +715,7 @@ export async function initialize(opts: {
     }
   }
 
-  const upgradeHandler: Parameters<typeof initializeServerWorker>[1] = async (
-    req,
-    socket,
-    head
-  ) => {
+  const upgradeHandler: WorkerUpgradeHandler = async (req, socket, head) => {
     try {
       req.on('error', (_err) => {
         // TODO: log socket errors?
@@ -735,16 +756,5 @@ export async function initialize(opts: {
     }
   }
 
-  const { port, hostname } = await initializeServerWorker(
-    requestHandler,
-    upgradeHandler,
-    opts
-  )
-
-  initializeResult = {
-    port,
-    hostname: hostname === '0.0.0.0' ? '127.0.0.1' : hostname,
-  }
-
-  return initializeResult
+  return [requestHandler, upgradeHandler]
 }
diff --git a/packages/next/src/server/lib/router-utils/setup-dev.ts b/packages/next/src/server/lib/router-utils/setup-dev.ts
@@ -17,7 +17,7 @@ import findUp from 'next/dist/compiled/find-up'
 import { buildCustomRoute } from './filesystem'
 import * as Log from '../../../build/output/log'
 import HotReloader, { matchNextPageBundleRequest } from '../../dev/hot-reloader'
-import { traceGlobals } from '../../../trace/shared'
+import { setGlobal } from '../../../trace/shared'
 import { Telemetry } from '../../../telemetry/storage'
 import { IncomingMessage, ServerResponse } from 'http'
 import loadJsConfig from '../../../build/load-jsconfig'
@@ -79,6 +79,7 @@ import { PagesManifest } from '../../../build/webpack/plugins/pages-manifest-plu
 import { AppBuildManifest } from '../../../build/webpack/plugins/app-build-manifest-plugin'
 import { PageNotFoundError } from '../../../shared/lib/utils'
 import { srcEmptySsgManifest } from '../../../build/webpack/plugins/build-manifest-plugin'
+import { PropagateToWorkersField } from './types'
 import { MiddlewareManifest } from '../../../build/webpack/plugins/middleware-plugin'
 
 type SetupOpts = {
@@ -120,8 +121,8 @@ async function startWatcher(opts: SetupOpts) {
 
   const distDir = path.join(opts.dir, opts.nextConfig.distDir)
 
-  traceGlobals.set('distDir', distDir)
-  traceGlobals.set('phase', PHASE_DEVELOPMENT_SERVER)
+  setGlobal('distDir', distDir)
+  setGlobal('phase', PHASE_DEVELOPMENT_SERVER)
 
   const validFileMatcher = createValidFileMatcher(
     nextConfig.pageExtensions,
@@ -133,7 +134,7 @@ async function startWatcher(opts: SetupOpts) {
     pages?: import('../router-server').RenderWorker
   } = {}
 
-  async function propagateToWorkers(field: string, args: any) {
+  async function propagateToWorkers(field: PropagateToWorkersField, args: any) {
     await renderWorkers.app?.propagateServerField(field, args)
     await renderWorkers.pages?.propagateServerField(field, args)
   }
@@ -1010,7 +1011,7 @@ async function startWatcher(opts: SetupOpts) {
           hotReloader.setHmrServerError(new Error(errorMessage))
         } else if (numConflicting === 0) {
           hotReloader.clearHmrServerError()
-          await propagateToWorkers('matchers.reload', undefined)
+          await propagateToWorkers('reloadMatchers', undefined)
         }
       }
 
@@ -1279,7 +1280,7 @@ async function startWatcher(opts: SetupOpts) {
       } finally {
         // Reload the matchers. The filesystem would have been written to,
         // and the matchers need to re-scan it to update the router.
-        await propagateToWorkers('middleware.reload', undefined)
+        await propagateToWorkers('reloadMatchers', undefined)
       }
     })
 

diff --git a/packages/next/src/server/lib/router-utils/types.ts b/packages/next/src/server/lib/router-utils/types.ts
@@ -0,0 +1,7 @@
+export type PropagateToWorkersField =
+  | 'actualMiddlewareFile'
+  | 'actualInstrumentationHookFile'
+  | 'reloadMatchers'
+  | 'loadEnvConfig'
+  | 'appPathRoutes'
+  | 'middleware'