Skip to content
This repository has been archived by the owner on May 22, 2024. It is now read-only.

Commit

Permalink
feat: improve performance by reworking caching and supply cache to NFT (
Browse files Browse the repository at this point in the history
#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 <mail@eduardoboucas.com>

* chore: fix comment

* chore: fix comment

* chore: add ff

Co-authored-by: Eduardo Bouças <mail@eduardoboucas.com>
  • Loading branch information
danez and eduardoboucas committed Nov 4, 2022
1 parent a4ec376 commit 22725d7
Show file tree
Hide file tree
Showing 25 changed files with 249 additions and 192 deletions.
38 changes: 4 additions & 34 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions src/feature_flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
10 changes: 7 additions & 3 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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(
Expand Down
20 changes: 4 additions & 16 deletions src/runtimes/detect_runtime.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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)
Expand Down
35 changes: 17 additions & 18 deletions src/runtimes/go/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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) {
Expand All @@ -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))
Expand All @@ -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))
Expand Down
32 changes: 15 additions & 17 deletions src/runtimes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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,
})
Expand All @@ -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 })
Expand Down
11 changes: 7 additions & 4 deletions src/runtimes/node/bundlers/esbuild/plugin_dynamic_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>>

Expand Down Expand Up @@ -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 = ({
Expand Down

1 comment on commit 22725d7

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⏱ Benchmark results

largeDepsEsbuild: 2.5s

largeDepsNft: 11.9s

largeDepsZisi: 18s

Please sign in to comment.