feat: improve performance by reworking caching and supply cache to NFT (

netlify/zip-it-and-ship-it#1244) * feat: improve performance by reworking caching and supply cache to NFT * chore: move cache into class * chore: comment about fileIOConcurrency * Update src/runtimes/node/bundlers/nft/es_modules.ts Co-authored-by: Eduardo Bouças <[email protected]> * chore: fix comment * chore: fix comment * chore: add ff Co-authored-by: Eduardo Bouças <[email protected]>
netlify · Nov 4, 2022 · 6f391d3 · 6f391d3
1 parent 9513e4f
commit 6f391d3
Show file tree

Hide file tree

Showing 25 changed files with 249 additions and 192 deletions.
diff --git a/packages/zip-it-and-ship-it/package-lock.json b/packages/zip-it-and-ship-it/package-lock.json
diff --git a/packages/zip-it-and-ship-it/package.json b/packages/zip-it-and-ship-it/package.json
@@ -77,7 +77,6 @@
     "p-map": "^4.0.0",
     "path-exists": "^5.0.0",
     "precinct": "^9.0.1",
-    "read-package-json-fast": "^2.0.2",
     "require-package-name": "^2.0.1",
     "resolve": "^2.0.0-next.1",
     "semver": "^7.0.0",

diff --git a/packages/zip-it-and-ship-it/src/feature_flags.ts b/packages/zip-it-and-ship-it/src/feature_flags.ts
@@ -21,6 +21,12 @@ export const defaultFlags: Record<string, boolean> = {
   // Load configuration from per-function JSON files.
   project_deploy_configuration_api_use_per_function_configuration_files: false,
 
+  // Enable runtime cache for NFT
+  zisi_nft_use_cache: false,
+
+  // Raise file IO limit for NFT
+  zisi_nft_higher_fileio_limit: false,
+
   // Provide banner to esbuild which allows requires in ESM output
   zisi_esbuild_require_banner: false,
 }

diff --git a/packages/zip-it-and-ship-it/src/main.ts b/packages/zip-it-and-ship-it/src/main.ts
@@ -6,6 +6,7 @@ import { FunctionSource } from './function.js'
 import { getFunctionFromPath, getFunctionsFromPaths } from './runtimes/index.js'
 import { findISCDeclarationsInPath, ISCValues } from './runtimes/node/in_source_config/index.js'
 import { GetSrcFilesFunction, RuntimeType } from './runtimes/runtime.js'
+import { RuntimeCache } from './utils/cache.js'
 import { listFunctionsDirectories, resolveFunctionsDirectories } from './utils/fs.js'
 
 export { zipFunction, zipFunctions } from './zip.js'
@@ -60,7 +61,8 @@ export const listFunctions = async function (
   const featureFlags = getFlags(inputFeatureFlags)
   const srcFolders = resolveFunctionsDirectories(relativeSrcFolders)
   const paths = await listFunctionsDirectories(srcFolders)
-  const functionsMap = await getFunctionsFromPaths(paths, { featureFlags, config })
+  const cache = new RuntimeCache()
+  const functionsMap = await getFunctionsFromPaths(paths, { cache, config, featureFlags })
   const functions = [...functionsMap.values()]
   const augmentedFunctions = parseISC ? await Promise.all(functions.map(augmentWithISC)) : functions
 
@@ -77,7 +79,8 @@ export const listFunction = async function (
   }: { featureFlags?: FeatureFlags; config?: Config; parseISC?: boolean } = {},
 ) {
   const featureFlags = getFlags(inputFeatureFlags)
-  const func = await getFunctionFromPath(path, { featureFlags, config })
+  const cache = new RuntimeCache()
+  const func = await getFunctionFromPath(path, { cache, config, featureFlags })
 
   if (!func) {
     return
@@ -96,7 +99,8 @@ export const listFunctionsFiles = async function (
   const featureFlags = getFlags(inputFeatureFlags)
   const srcFolders = resolveFunctionsDirectories(relativeSrcFolders)
   const paths = await listFunctionsDirectories(srcFolders)
-  const functionsMap = await getFunctionsFromPaths(paths, { config, featureFlags })
+  const cache = new RuntimeCache()
+  const functionsMap = await getFunctionsFromPaths(paths, { cache, config, featureFlags })
   const functions = [...functionsMap.values()]
   const augmentedFunctions = parseISC ? await Promise.all(functions.map(augmentWithISC)) : functions
   const listedFunctionsFiles = await Promise.all(

diff --git a/packages/zip-it-and-ship-it/src/runtimes/detect_runtime.ts b/packages/zip-it-and-ship-it/src/runtimes/detect_runtime.ts
@@ -1,9 +1,7 @@
-import type { Buffer } from 'buffer'
+import { readFile } from 'fs/promises'
 
 import { detect, Runtime as BinaryRuntime, Arch, Platform, BinaryInfo } from '@netlify/binary-info'
 
-import { cachedReadFile, FsCache } from '../utils/fs.js'
-
 import { RuntimeType } from './runtime.js'
 
 const isValidFunctionBinary = (info: BinaryInfo) => info.arch === Arch.Amd64 && info.platform === Platform.Linux
@@ -21,20 +19,10 @@ The binary needs to be built for Linux/Amd64, but it was built for ${Platform[bi
 }
 
 // Try to guess the runtime by inspecting the binary file.
-export const detectBinaryRuntime = async function ({
-  fsCache,
-  path,
-}: {
-  fsCache: FsCache
-  path: string
-}): Promise<RuntimeType | undefined> {
+export const detectBinaryRuntime = async function ({ path }: { path: string }): Promise<RuntimeType | undefined> {
   try {
-    const buffer = await cachedReadFile(fsCache, path)
-
-    // We're using the Type Assertion because the `cachedReadFile` abstraction
-    // loses part of the return type information. We can safely say it's a
-    // Buffer in this case because we're not specifying an encoding.
-    const binaryInfo = detect(buffer as Buffer)
+    const fileContents = await readFile(path)
+    const binaryInfo = detect(fileContents)
 
     if (!isValidFunctionBinary(binaryInfo)) {
       return warnIncompatibleBinary(path, binaryInfo)

diff --git a/packages/zip-it-and-ship-it/src/runtimes/go/index.ts b/packages/zip-it-and-ship-it/src/runtimes/go/index.ts
@@ -4,7 +4,8 @@ import { basename, dirname, extname, join } from 'path'
 import { copyFile } from 'cp-file'
 
 import { SourceFile } from '../../function.js'
-import { cachedLstat, cachedReaddir, FsCache } from '../../utils/fs.js'
+import type { RuntimeCache } from '../../utils/cache.js'
+import { cachedLstat, cachedReaddir } from '../../utils/fs.js'
 import { nonNullable } from '../../utils/non_nullable.js'
 import { zipBinary } from '../../zip_binary.js'
 import { detectBinaryRuntime } from '../detect_runtime.js'
@@ -23,18 +24,16 @@ interface GoBinary {
   stat: Stats
 }
 
-const detectGoFunction = async ({ fsCache, path }: { fsCache: FsCache; path: string }) => {
-  const stat = await cachedLstat(fsCache, path)
+const detectGoFunction = async ({ cache, path }: { cache: RuntimeCache; path: string }) => {
+  const stat = await cachedLstat(cache.lstatCache, path)
 
   if (!stat.isDirectory()) {
     return
   }
 
   const directoryName = basename(path)
 
-  // @ts-expect-error TODO: The `makeCachedFunction` abstraction is causing the
-  // return value of `readdir` to be incorrectly typed.
-  const files = (await cachedReaddir(fsCache, path)) as string[]
+  const files = await cachedReaddir(cache.readdirCache, path)
   const mainFileName = [`${directoryName}.go`, 'main.go'].find((name) => files.includes(name))
 
   if (mainFileName === undefined) {
@@ -44,28 +43,28 @@ const detectGoFunction = async ({ fsCache, path }: { fsCache: FsCache; path: str
   return mainFileName
 }
 
-const findFunctionsInPaths: FindFunctionsInPathsFunction = async function ({ featureFlags, fsCache, paths }) {
-  const functions = await Promise.all(paths.map((path) => findFunctionInPath({ featureFlags, fsCache, path })))
+const findFunctionsInPaths: FindFunctionsInPathsFunction = async function ({ cache, featureFlags, paths }) {
+  const functions = await Promise.all(paths.map((path) => findFunctionInPath({ cache, featureFlags, path })))
 
   return functions.filter(nonNullable)
 }
 
-const findFunctionInPath: FindFunctionInPathFunction = async function ({ fsCache, path }) {
-  const runtime = await detectBinaryRuntime({ fsCache, path })
+const findFunctionInPath: FindFunctionInPathFunction = async function ({ cache, path }) {
+  const runtime = await detectBinaryRuntime({ path })
 
   if (runtime === RuntimeType.GO) {
-    return processBinary({ fsCache, path })
+    return processBinary({ cache, path })
   }
 
-  const goSourceFile = await detectGoFunction({ fsCache, path })
+  const goSourceFile = await detectGoFunction({ cache, path })
 
   if (goSourceFile) {
-    return processSource({ fsCache, mainFile: goSourceFile, path })
+    return processSource({ cache, mainFile: goSourceFile, path })
   }
 }
 
-const processBinary = async ({ fsCache, path }: { fsCache: FsCache; path: string }): Promise<SourceFile> => {
-  const stat = (await cachedLstat(fsCache, path)) as Stats
+const processBinary = async ({ cache, path }: { cache: RuntimeCache; path: string }): Promise<SourceFile> => {
+  const stat = await cachedLstat(cache.lstatCache, path)
   const extension = extname(path)
   const filename = basename(path)
   const name = basename(path, extname(path))
@@ -82,19 +81,19 @@ const processBinary = async ({ fsCache, path }: { fsCache: FsCache; path: string
 }
 
 const processSource = async ({
-  fsCache,
+  cache,
   mainFile,
   path,
 }: {
-  fsCache: FsCache
+  cache: RuntimeCache
   mainFile: string
   path: string
 }): Promise<SourceFile> => {
   // TODO: This `stat` value is not going to be used, but we need it to satisfy
   // the `FunctionSource` interface. We should revisit whether `stat` should be
   // part of that interface in the first place, or whether we could compute it
   // downstream when needed (maybe using the FS cache as an optimisation).
-  const stat = (await cachedLstat(fsCache, path)) as Stats
+  const stat = (await cachedLstat(cache.lstatCache, path)) as Stats
   const filename = basename(path)
   const extension = extname(mainFile)
   const name = basename(path, extname(path))

diff --git a/packages/zip-it-and-ship-it/src/runtimes/index.ts b/packages/zip-it-and-ship-it/src/runtimes/index.ts
@@ -3,7 +3,7 @@ import { extname, basename } from 'path'
 import { Config, getConfigForFunction, FunctionWithoutConfig } from '../config.js'
 import { defaultFlags, FeatureFlags } from '../feature_flags.js'
 import { FunctionSource } from '../function.js'
-import { FsCache } from '../utils/fs.js'
+import type { RuntimeCache } from '../utils/cache.js'
 
 import goRuntime from './go/index.js'
 import jsRuntime from './node/index.js'
@@ -27,19 +27,19 @@ type FunctionTupleWithoutConfig = [string, FunctionWithoutConfig]
  * (`remainingPaths`).
  */
 const findFunctionsInRuntime = async function ({
+  cache,
   dedupe = false,
   featureFlags,
-  fsCache,
   paths,
   runtime,
 }: {
+  cache: RuntimeCache
   dedupe: boolean
   featureFlags: FeatureFlags
-  fsCache: FsCache
   paths: string[]
   runtime: Runtime
 }) {
-  const functions = await runtime.findFunctionsInPaths({ featureFlags, fsCache, paths })
+  const functions = await runtime.findFunctionsInPaths({ cache, featureFlags, paths })
 
   // If `dedupe` is true, we use the function name (`filename`) as the map key,
   // so that `function-1.js` will overwrite `function-1.go`. Otherwise, we use
@@ -62,11 +62,6 @@ const findFunctionsInRuntime = async function ({
   return { functions: augmentedFunctions, remainingPaths }
 }
 
-// An object to cache filesystem operations. This allows different functions
-// to perform IO operations on the same file (i.e. getting its stats or its
-// contents) without duplicating work.
-const makeFsCache = (): FsCache => ({})
-
 // The order of this array determines the priority of the runtimes. If a path
 // is used by the first time, it won't be made available to the subsequent
 // runtimes.
@@ -78,24 +73,29 @@ const RUNTIMES = [jsRuntime, goRuntime, rustRuntime]
 export const getFunctionsFromPaths = async (
   paths: string[],
   {
+    cache,
     config,
     configFileDirectories = [],
     dedupe = false,
     featureFlags = defaultFlags,
-  }: { config?: Config; configFileDirectories?: string[]; dedupe?: boolean; featureFlags?: FeatureFlags } = {},
+  }: {
+    cache: RuntimeCache
+    config?: Config
+    configFileDirectories?: string[]
+    dedupe?: boolean
+    featureFlags?: FeatureFlags
+  },
 ): Promise<FunctionMap> => {
-  const fsCache = makeFsCache()
-
   // We cycle through the ordered array of runtimes, passing each one of them
   // through `findFunctionsInRuntime`. For each iteration, we collect all the
   // functions found plus the list of paths that still need to be evaluated,
   // using them as the input for the next iteration until the last runtime.
   const { functions } = await RUNTIMES.reduce(async (aggregate, runtime) => {
     const { functions: aggregateFunctions, remainingPaths: aggregatePaths } = await aggregate
     const { functions: runtimeFunctions, remainingPaths: runtimePaths } = await findFunctionsInRuntime({
+      cache,
       dedupe,
       featureFlags,
-      fsCache,
       paths: aggregatePaths,
       runtime,
     })
@@ -121,12 +121,10 @@ export const getFunctionsFromPaths = async (
  */
 export const getFunctionFromPath = async (
   path: string,
-  { config, featureFlags = defaultFlags }: { config?: Config; featureFlags?: FeatureFlags } = {},
+  { cache, config, featureFlags = defaultFlags }: { cache: RuntimeCache; config?: Config; featureFlags?: FeatureFlags },
 ): Promise<FunctionSource | undefined> => {
-  const fsCache = makeFsCache()
-
   for (const runtime of RUNTIMES) {
-    const func = await runtime.findFunctionInPath({ path, fsCache, featureFlags })
+    const func = await runtime.findFunctionInPath({ path, cache, featureFlags })
 
     if (func) {
       const functionConfig = await getConfigForFunction({ config, func: { ...func, runtime }, featureFlags })

diff --git a/packages/zip-it-and-ship-it/src/runtimes/node/bundlers/esbuild/plugin_dynamic_imports.ts b/packages/zip-it-and-ship-it/src/runtimes/node/bundlers/esbuild/plugin_dynamic_imports.ts
@@ -3,9 +3,9 @@ import { basename, join, relative } from 'path'
 import type { Plugin } from '@netlify/esbuild'
 import { findUp, findUpStop, pathExists } from 'find-up'
 import normalizePath from 'normalize-path'
-import readPackageJson from 'read-package-json-fast'
 
 import { parseExpression } from '../../parser/index.js'
+import { readPackageJson } from '../../utils/package_json.js'
 
 type PackageCache = Map<string, Promise<string | undefined>>
 
@@ -102,11 +102,14 @@ const getPackageNameCached = ({
   resolveDir: string
   srcDir: string
 }) => {
-  if (!cache.has(resolveDir)) {
-    cache.set(resolveDir, getPackageName({ resolveDir, srcDir }))
+  let result = cache.get(resolveDir)
+
+  if (result === undefined) {
+    result = getPackageName({ resolveDir, srcDir })
+    cache.set(resolveDir, result)
   }
 
-  return cache.get(resolveDir)
+  return result
 }
 
 const getShimContents = ({