perf: improve graph resolve performance

core/src/commands/base.ts

@@ -41,6 +41,7 @@ import type { AnalyticsHandler } from "../analytics/analytics.js"
 import { withSessionContext } from "../util/open-telemetry/context.js"
 import { wrapActiveSpan } from "../util/open-telemetry/spans.js"
 import { styles } from "../logger/styles.js"
+import { clearVarfileCache } from "../config/base.js"
 
 export interface CommandConstructor {
   new (parent?: CommandGroup): Command
@@ -158,6 +159,7 @@ type DataCallback = (data: string) => void
 export type CommandArgsType<C extends Command> = C extends Command<infer Args, any> ? Args : never
 export type CommandOptionsType<C extends Command> = C extends Command<any, infer Opts> ? Opts : never
 export type CommandResultType<C extends Command> = C extends Command<any, any, infer R> ? R : never
+
 export abstract class Command<
   A extends ParameterObject = ParameterObject,
   O extends ParameterObject = ParameterObject,
@@ -375,6 +377,8 @@ export abstract class Command<
             // Clear the VCS handler's tree cache to make sure we pick up any changed sources.
             // FIXME: use file watching to be more surgical here, this is suboptimal
             garden.treeCache.invalidateDown(log, ["path"])
+            // also clear the cached varfiles
+            clearVarfileCache()
 
             log.silly(() => `Starting command '${this.getFullName()}' action`)
             result = await this.action({

core/src/config/base.ts

@@ -34,6 +34,7 @@ import { dedent, deline } from "../util/string.js"
 import { makeDocsLinkStyled } from "../docs/common.js"
 import { profile, profileAsync } from "../util/profiling.js"
 import { readFile } from "fs/promises"
+import { LRUCache } from "lru-cache"
 
 export const configTemplateKind = "ConfigTemplate"
 export const renderTemplateKind = "RenderTemplate"
@@ -560,6 +561,16 @@ const _loadYaml = profile(function _loadYaml(data: Buffer) {
   return load(data.toString()) as PrimitiveMap
 })
 
+const loadVarfileCache = new LRUCache<string, Promise<PrimitiveMap>>({
+  max: 10000,
+  ttl: 30000,
+  ttlAutopurge: true,
+})
+
+export function clearVarfileCache() {
+  loadVarfileCache.clear()
+}
+
 export const loadVarfile = profileAsync(async function loadVarfile({
   configRoot,
   path,
@@ -573,63 +584,81 @@ export const loadVarfile = profileAsync(async function loadVarfile({
   defaultPath: string | undefined
   optional?: boolean
   log?: Log
-}): Promise<PrimitiveMap> {
-  if (!path && !defaultPath) {
+}) {
+  const pathOrDefault = path || defaultPath
+  if (!pathOrDefault) {
     throw new ParameterError({
       message: `Neither a path nor a defaultPath was provided. Config root: ${configRoot}`,
     })
   }
-  const resolvedPath = resolve(configRoot, <string>(path || defaultPath))
+  const resolvedPath = resolve(configRoot, pathOrDefault)
 
-  try {
-    const data = await _readFile(resolvedPath)
-    log?.silly(() => `Loaded ${data.length} bytes from varfile ${resolvedPath}`)
-    const relPath = relative(configRoot, resolvedPath)
-    const filename = basename(resolvedPath.toLowerCase())
-
-    if (filename.endsWith(".json")) {
-      // JSON parser throws a JSON syntax error on completely empty input file,
-      // and returns an empty object for an empty JSON.
-      const parsed = JSON.parse(data.toString())
-      if (!isPlainObject(parsed)) {
-        throw new ConfigurationError({
-          message: `Configured variable file ${relPath} must be a valid plain JSON object. Got: ${typeof parsed}`,
-        })
+  let promise: Promise<PrimitiveMap> | undefined = loadVarfileCache.get(resolvedPath)
+  if (!promise) {
+    promise = loadVarfileInner()
+    loadVarfileCache.set(resolvedPath, promise)
+  }
+
+  return await promise
+
+  async function loadVarfileInner(): Promise<PrimitiveMap> {
+    try {
+      const data = await _readFile(resolvedPath)
+      log?.silly(() => `Loaded ${data.length} bytes from varfile ${resolvedPath}`)
+      const relPath = relative(configRoot, resolvedPath)
+      const filename = basename(resolvedPath.toLowerCase())
+
+      if (filename.endsWith(".json")) {
+        // JSON parser throws a JSON syntax error on completely empty input file,
+        // and returns an empty object for an empty JSON.
+        const parsed = JSON.parse(data.toString())
+        if (!isPlainObject(parsed)) {
+          throw new ConfigurationError({
+            message: `Configured variable file ${relPath} must be a valid plain JSON object. Got: ${typeof parsed}`,
+          })
+        }
+        return parsed as PrimitiveMap
+      } else if (filename.endsWith(".yml") || filename.endsWith(".yaml")) {
+        // YAML parser returns `undefined` for empty files, we interpret that as an empty object.
+        const parsed = _loadYaml(data) || {}
+        if (!isPlainObject(parsed)) {
+          throw new ConfigurationError({
+            message: `Configured variable file ${relPath} must be a single plain YAML mapping. Got: ${typeof parsed}`,
+          })
+        }
+        return parsed as PrimitiveMap
+      } else {
+        // Note: For backwards-compatibility we fall back on using .env as a default format,
+        // and don't specifically validate the extension for that.
+        // The dotenv parser returns an empty object for invalid or empty input file.
+        const parsed = dotenv.parse(data)
+        return parsed as PrimitiveMap
       }
-      return parsed as PrimitiveMap
-    } else if (filename.endsWith(".yml") || filename.endsWith(".yaml")) {
-      // YAML parser returns `undefined` for empty files, we interpret that as an empty object.
-      const parsed = _loadYaml(data) || {}
-      if (!isPlainObject(parsed)) {
-        throw new ConfigurationError({
-          message: `Configured variable file ${relPath} must be a single plain YAML mapping. Got: ${typeof parsed}`,
-        })
+    } catch (error) {
+      if (error instanceof ConfigurationError) {
+        throw error
       }
-      return parsed as PrimitiveMap
-    } else {
-      // Note: For backwards-compatibility we fall back on using .env as a default format,
-      // and don't specifically validate the extension for that.
-      // The dotenv parser returns an empty object for invalid or empty input file.
-      const parsed = dotenv.parse(data)
-      return parsed as PrimitiveMap
-    }
-  } catch (error) {
-    if (error instanceof ConfigurationError) {
-      throw error
-    }
 
-    if (isErrnoException(error) && error.code === "ENOENT") {
-      if (path && path !== defaultPath && !optional) {
-        throw new ConfigurationError({
-          message: `Could not find varfile at path '${path}'. Absolute path: ${resolvedPath}`,
-        })
-      } else {
-        return {}
+      if (isErrnoException(error) && error.code === "ENOENT") {
+        if (
+          // if path is defined, we are loading explicitly configured varfile.
+          path &&
+          // if the user explicitly declares default path (e.g. garden.env) then we do not throw.
+          path !== defaultPath &&
+          !optional
+        ) {
+          throw new ConfigurationError({
+            message: `Could not find varfile at path '${path}'. Absolute path: ${resolvedPath}`,
+          })
+        } else {
+          // The default var file did not exist. In that case we return empty object.
+          return {}
+        }
       }
-    }
 
-    throw new ConfigurationError({
-      message: `Unable to load varfile at '${path}': ${error}`,
-    })
+      throw new ConfigurationError({
+        message: `Unable to load varfile at '${path}': ${error}`,
+      })
+    }
   }
 })

core/src/template-string/template-string.ts

@@ -295,7 +295,8 @@ export const resolveTemplateStrings = profile(function resolveTemplateStrings<T
   } else if (Array.isArray(value)) {
     const output: unknown[] = []
 
-    value.forEach((v, i) => {
+    for (let i = 0; i < value.length; i++) {
+      const v = value[i]
       if (isPlainObject(v) && v[arrayConcatKey] !== undefined) {
         if (Object.keys(v).length > 1) {
           const extraKeys = naturalList(
@@ -337,7 +338,7 @@ export const resolveTemplateStrings = profile(function resolveTemplateStrings<T
       } else {
         output.push(resolveTemplateStrings({ value: v, context, contextOpts, source, path: path && [...path, i] }))
       }
-    })
+    }
 
     return <T>(<unknown>output)
   } else if (isPlainObject(value)) {
@@ -351,7 +352,8 @@ export const resolveTemplateStrings = profile(function resolveTemplateStrings<T
       // Resolve $merge keys, depth-first, leaves-first
       let output = {}
 
-      for (const [k, v] of Object.entries(value)) {
+      for (const k in value as Record<string, unknown>) {
+        const v = value[k]
         const resolved = resolveTemplateStrings({ value: v, context, contextOpts, source, path: path && [...path, k] })
 
         if (k === objectSpreadKey) {

core/test/unit/src/actions/action-configs-to-graph.ts

@@ -21,6 +21,7 @@ import {
   DEFAULT_TEST_TIMEOUT_SEC,
 } from "../../../../src/constants.js"
 import { getRemoteSourceLocalPath } from "../../../../src/util/ext-source-util.js"
+import { clearVarfileCache } from "../../../../src/config/base.js"
 
 describe("actionConfigsToGraph", () => {
   let tmpDir: TempDirectory
@@ -34,6 +35,11 @@ describe("actionConfigsToGraph", () => {
     log = garden.log
   })
 
+  afterEach(() => {
+    // Some tests re-use and re-write existing varfiles, so we need to clear the cache explicitly.
+    clearVarfileCache()
+  })
+
   it("resolves a Build action", async () => {
     const graph = await actionConfigsToGraph({
       garden,