perf: speed up all commands by 75+ ms by enabling V8 code cache when available (node>=22.8.0) (#7173)

* build(deps): use correct version of @types/node

Our minimum supported version is 18, so we need to rely on node 18 types.

* refactor: wrap main program in a main function

* chore(devDeps): remove unused is-ci dev dep

* refactor: allow calling `setAnalyticsPayload` earlier

The `preAction` hook was overwriting the whole payload, so any prior calls to
`setAnalyticsPayload` would have their data thrown away.

This didn't matter, but I'd like to introduce an early call.

* perf: enable V8 code cache when available

See https://nodejs.org/api/module.html#module-compile-cache.

In local benchmarking on various machines, this led to a 75-400 ms improvement to command
run time across the board, at the expense of a moderate increase in first-time run time.

See inline - to account for the common CI single-run use case, this is disabled when we
detect a CI environment.

* fix: log telemetry track errors in debug mode

* refactor: extract compile cache module

This also removes the child process handling.

* build: disable compile cache when running tests

* perf: defer more imports in bin/run

* docs: add another clarification to compile cache

Author: serhalp

bin/run.js

@@ -1,32 +1,41 @@
 #!/usr/bin/env node
 import { argv } from 'process'
 
-import updateNotifier from 'update-notifier'
-
-import { createMainCommand } from '../dist/commands/main.js'
-import { logError } from '../dist/utils/command-helpers.js'
-import getPackageJson from '../dist/utils/get-cli-package-json.js'
-import { runProgram } from '../dist/utils/run-program.js'
+import { maybeEnableCompileCache } from '../dist/utils/nodejs-compile-cache.js'
 
 // 12 hours
 const UPDATE_CHECK_INTERVAL = 432e5
-const pkg = await getPackageJson()
-
-try {
-  updateNotifier({
-    pkg,
-    updateCheckInterval: UPDATE_CHECK_INTERVAL,
-  }).notify()
-} catch (error) {
-  logError(`Error checking for updates: ${error?.toString()}`)
-}
 
-const program = createMainCommand()
+const main = async () => {
+  const { default: updateNotifier } = await import('update-notifier')
+  const { createMainCommand } = await import('../dist/commands/main.js')
+  const { logError } = await import('../dist/utils/command-helpers.js')
+  const { default: getPackageJson } = await import('../dist/utils/get-cli-package-json.js')
+  const { runProgram } = await import('../dist/utils/run-program.js')
+
+  const pkg = await getPackageJson()
+
+  try {
+    updateNotifier({
+      pkg,
+      updateCheckInterval: UPDATE_CHECK_INTERVAL,
+    }).notify()
+  } catch (error) {
+    logError(`Error checking for updates: ${error?.toString()}`)
+  }
 
-try {
-  await runProgram(program, argv)
+  const program = createMainCommand()
 
-  program.onEnd()
-} catch (error) {
-  program.onEnd(error)
+  try {
+    await runProgram(program, argv)
+
+    program.onEnd()
+  } catch (error) {
+    program.onEnd(error)
+  }
 }
+
+// Must come first, including before any imports
+maybeEnableCompileCache()
+
+await main()

eslint_temporary_suppressions.js

@@ -1441,7 +1441,6 @@ export default [
   {
     files: ['tests/integration/telemetry.test.ts'],
     rules: {
-      '@typescript-eslint/no-unused-vars': 'off',
       '@typescript-eslint/no-non-null-assertion': 'off',
       '@typescript-eslint/no-unsafe-assignment': 'off',
     },

package-lock.json

@@ -181,7 +181,7 @@
     "@types/jsonwebtoken": "9.0.9",
     "@types/lodash": "4.17.16",
     "@types/multiparty": "4.2.1",
-    "@types/node": "22.13.10",
+    "@types/node": "18.19.86",
     "@types/node-fetch": "2.6.12",
     "@types/parallel-transform": "1.1.4",
     "@types/parse-github-url": "1.0.3",
@@ -203,7 +203,6 @@
     "eslint-config-prettier": "^10.1.1",
     "eslint-plugin-n": "^17.16.1",
     "form-data": "4.0.2",
-    "is-ci": "4.1.0",
     "memfs": "^4.17.0",
     "nock": "14.0.1",
     "npm-run-all2": "^7.0.2",

package.json

@@ -152,6 +152,8 @@ async function getRepositoryRoot(cwd?: string): Promise<string | undefined> {
 export default class BaseCommand extends Command {
   /** The netlify object inside each command with the state */
   netlify!: NetlifyOptions
+  // TODO(serhalp) We set `startTime` here and then overwrite it in a `preAction` hook. This is
+  // just asking for latent bugs. Remove this one?
   analytics: Analytics = { startTime: process.hrtime.bigint() }
   project!: Project
 
@@ -213,7 +215,7 @@ export default class BaseCommand extends Command {
         process.env.DEBUG = '*'
       }
       debug(`${name}:preAction`)('start')
-      this.analytics = { startTime: process.hrtime.bigint() }
+      this.analytics.startTime = process.hrtime.bigint()
       await this.init(actionCommand as BaseCommand)
       debug(`${name}:preAction`)('end')
     })
@@ -378,7 +380,11 @@ export default class BaseCommand extends Command {
         duration,
         status,
       })
-    } catch {}
+    } catch (err) {
+      debug(`${this.name()}:onEnd`)(
+        `Command: ${command}. Telemetry tracking failed: ${err instanceof Error ? err.message : err?.toString()}`,
+      )
+    }
 
     if (error_ !== undefined) {
       logError(error_ instanceof Error ? error_ : format(error_))

src/commands/base-command.ts

@@ -19,6 +19,7 @@ import {
 import execa from '../utils/execa.js'
 import getGlobalConfigStore from '../utils/get-global-config-store.js'
 import getCLIPackageJson from '../utils/get-cli-package-json.js'
+import { didEnableCompileCache } from '../utils/nodejs-compile-cache.js'
 import { track, reportError } from '../utils/telemetry/index.js'
 
 import { createApiCommand } from './api/index.js'
@@ -211,11 +212,10 @@ const mainCommand = async function (options, command) {
 
 /**
  * Creates the `netlify-cli` command
- * Promise is needed as the envinfo is a promise
- * @returns {import('./base-command.js').default}
  */
-export const createMainCommand = () => {
+export const createMainCommand = (): BaseCommand => {
   const program = new BaseCommand('netlify')
+
   // register all the commands
   createApiCommand(program)
   createBlobsCommand(program)
@@ -239,6 +239,8 @@ export const createMainCommand = () => {
   createWatchCommand(program)
   createLogsCommand(program)
 
+  program.setAnalyticsPayload({ didEnableCompileCache })
+
   program
     .version(USER_AGENT, '-V')
     .showSuggestionAfterError(true)

src/commands/main.ts

@@ -74,7 +74,7 @@ const connectTunnel = function ({
   }
 
   const ps = execa(execPath, args, { stdio: 'inherit' })
-  ps.on('close', (code) => process.exit(code))
+  ps.on('close', (code) => process.exit(code ?? undefined))
   ps.on('SIGINT', process.exit)
   ps.on('SIGTERM', process.exit)
 }

src/utils/live-tunnel.ts

@@ -0,0 +1,47 @@
+import * as module from 'module'
+
+import { isCI } from 'ci-info'
+
+export let didEnableCompileCache = false
+
+/**
+ * This enables the Node.js compile cache (aka the V8 code cache or bytecode cache), if it is available and if we
+ * detect that we are not running in a CI environment.
+ *
+ * This feature is a performance optimization that allows the V8 JS engine to cache some (parse/compile) work from one
+ * execution on disk and reuse it on subsequent executions. There's a performance hit on the first (cold) run, but all
+ * subsequent (warm) runs get performance savings. As the CLI is generally run hundreds of times, it is worth a small
+ * overhead on the occasional cold run to shave tens to hundreds of milliseconds on several subsequent warm runs.
+ *
+ * Keep in mind that the cache is specific to a version of netlify-cli and a version of node.js and it is stored on the
+ * user's disk in a temp dir. If any of these changes or the temp dir is cleared, the next run is a cold run.
+ *
+ * The programmatic API to enable this (`enableCompileCache()`) was added in node 22.8.0, but we currently support
+ * >=18.14.0, hence the conditional below. (For completeness, note that support via the env var was added in 22.1.0.)
+ *
+ * The Netlify CLI is often used in CI workflows. In this context, we wouldn't want the overhead of the first run
+ * because we almost certainly would not get any benefits on "subsequent runs". Even if the user has configured caching
+ * of the CLI itself, there's no chance they've configured the V8 compile cache directory to be cached.
+ *
+ * @see https://nodejs.org/api/module.html#moduleenablecompilecachecachedir
+ */
+export const maybeEnableCompileCache = (): void => {
+  if (isCI) return
+  // The docs recommend turning this off when running tests to generate precise coverage
+  if (process.env.NODE_ENV === 'test') return
+
+  // eslint-disable-next-line n/no-unsupported-features/node-builtins
+  if ('enableCompileCache' in module && typeof module.enableCompileCache === 'function') {
+    // eslint-disable-next-line n/no-unsupported-features/node-builtins
+    const { directory } = (module.enableCompileCache as () => { directory: string | undefined })()
+
+    if (directory == null) return
+    didEnableCompileCache = true
+
+    // TODO(serhalp): Investigate enabling the compile cache for spawned subprocesses by passing
+    // NODE_COMPILE_CACHE=directory.
+
+    return
+  }
+  return
+}

src/utils/nodejs-compile-cache.ts

@@ -2,7 +2,6 @@ import { env as _env, version as nodejsVersion } from 'process'
 
 import type { Options } from 'execa'
 import execa from 'execa'
-import { version as uuidVersion } from 'uuid'
 import { expect, test } from 'vitest'
 
 import pkg from '../../package.json'
@@ -58,7 +57,7 @@ await withMockApi(routes, () => {
     expect(userAgent!.startsWith(`${pkg.name}/${pkg.version}`)).toBe(true)
   })
 
-  test<MockApiTestContext>('should send correct command on success', async ({ apiUrl, requests }) => {
+  test<MockApiTestContext>('should send invoked command on success', async ({ apiUrl, requests }) => {
     await callCli(['api', 'listSites'], getCLIOptions(apiUrl))
     const request = requests.find(({ path }) => path === '/api/v1/track')
     expect(request).toBeDefined()
@@ -71,14 +70,16 @@ await withMockApi(routes, () => {
       buildSystem: [],
       cliVersion: pkg.version,
       command: 'api',
+      // Varies depending on node.js version tested
+      didEnableCompileCache: expect.any(Boolean),
       monorepo: false,
       nodejsVersion,
       // TODO: this should be NPM however some CI tests are using pnpm which changes the value
       packageManager: expect.stringMatching(/npm|pnpm/),
     })
   })
 
-  test<MockApiTestContext>('should send correct command on failure', async ({ apiUrl, requests }) => {
+  test<MockApiTestContext>('should send invoked command on failure', async ({ apiUrl, requests }) => {
     await expect(callCli(['dev:exec', 'exit 1'], getCLIOptions(apiUrl))).rejects.toThrowError()
     const request = requests.find(({ path }) => path === '/api/v1/track')
     expect(request).toBeDefined()
@@ -91,6 +92,8 @@ await withMockApi(routes, () => {
       buildSystem: [],
       cliVersion: pkg.version,
       command: 'dev:exec',
+      // Varies depending on node.js version tested
+      didEnableCompileCache: expect.any(Boolean),
       monorepo: false,
       nodejsVersion,
       // TODO: this should be NPM however some CI tests are using pnpm which changes the value
@@ -110,20 +113,15 @@ await withMockApi(routes, () => {
       const request = t.requests.find(({ path }) => path === '/api/v1/track')
       expect(request).toBeDefined()
 
-      expect(request!.body).toHaveProperty('anonymousId', expect.any(String))
-      expect(request!.body).toHaveProperty('duration', expect.any(Number))
-      expect(request!.body).toHaveProperty('event', 'cli:command')
-      expect(request!.body).toHaveProperty('status', 'success')
-      expect(request!.body).toHaveProperty('properties', {
-        frameworks: ['next'],
-        buildSystem: [],
-        cliVersion: pkg.version,
-        command: 'api',
-        monorepo: false,
-        nodejsVersion,
-        // TODO: this should be NPM however some CI tests are using pnpm which changes the value
-        packageManager: expect.stringMatching(/npm|pnpm/),
-      })
+      expect(request!.body).toHaveProperty(
+        'properties',
+        expect.objectContaining({
+          frameworks: ['next'],
+          buildSystem: [],
+          // TODO: this should be NPM however some CI tests are using pnpm which changes the value
+          packageManager: expect.stringMatching(/npm|pnpm/),
+        }),
+      )
     })
   })
 })