perf: optimize action configs processing (#6547)

* chore: fix typos

* refactor: process action configs in batches

To limit concurrency and contention on I/O locking in large projects.

* perf(git): deduplicate paths when find minimal repo roots

* perf(git): calculate minimal roots in batches

* chore: log batch details on silly level

* refactor: remove unnecessary async declaration

Function `getAllProviderDependencyNames` does not perform any async operations.

* refactor: remove unnecessary promises around sync code

core/src/config/provider.ts

@@ -164,7 +164,7 @@ export function providerFromConfig({
  * Given a plugin and its provider config, return a list of dependency names based on declared dependencies,
  * as well as implicit dependencies based on template strings.
  */
-export async function getAllProviderDependencyNames(plugin: GardenPluginSpec, config: GenericProviderConfig) {
+export function getAllProviderDependencyNames(plugin: GardenPluginSpec, config: GenericProviderConfig) {
   return uniq([
     ...(plugin.dependencies || []).map((d) => d.name),
     ...(config.dependencies || []),

core/src/garden.ts

@@ -797,28 +797,26 @@ export class Garden {
       // Detect circular dependencies here
       const validationGraph = new DependencyGraph()
 
-      await Promise.all(
-        rawConfigs.map(async (config) => {
-          const plugin = plugins[config.name]
+      for (const config of rawConfigs) {
+        const plugin = plugins[config.name]
 
-          if (!plugin) {
-            throw new ConfigurationError({
-              message: dedent`
+        if (!plugin) {
+          throw new ConfigurationError({
+            message: dedent`
                 Configured provider '${config.name}' has not been registered.
 
                 Available plugins: ${Object.keys(plugins).join(", ")}
               `,
-            })
-          }
+          })
+        }
 
-          validationGraph.addNode(plugin.name)
+        validationGraph.addNode(plugin.name)
 
-          for (const dep of await getAllProviderDependencyNames(plugin!, config!)) {
-            validationGraph.addNode(dep)
-            validationGraph.addDependency(plugin.name, dep)
-          }
-        })
-      )
+        for (const dep of getAllProviderDependencyNames(plugin!, config!)) {
+          validationGraph.addNode(dep)
+          validationGraph.addDependency(plugin.name, dep)
+        }
+      }
 
       const cycles = validationGraph.detectCircularDependencies()
 
@@ -872,15 +870,13 @@ export class Garden {
       const allCached = providers.every((p) => p.status.cached)
       const someCached = providers.some((p) => p.status.cached)
 
-      await Promise.all(
-        providers.flatMap((provider) =>
-          provider.moduleConfigs.map(async (moduleConfig) => {
-            // Make sure module and all nested entities are scoped to the plugin
-            moduleConfig.plugin = provider.name
-            return this.addModuleConfig(moduleConfig)
-          })
-        )
-      )
+      for (const provider of providers) {
+        for (const moduleConfig of provider.moduleConfigs) {
+          // Make sure module and all nested entities are scoped to the plugin
+          moduleConfig.plugin = provider.name
+          this.addModuleConfig(moduleConfig)
+        }
+      }
 
       for (const provider of providers) {
         this.resolvedProviders[provider.name] = provider

core/src/graph/actions.ts

@@ -70,6 +70,19 @@ import { getSourcePath } from "../vcs/vcs.js"
 import { actionIsDisabled } from "../actions/base.js"
 import { styles } from "../logger/styles.js"
 
+function* sliceToBatches<T>(dict: Record<string, T>, batchSize: number) {
+  const entries = Object.entries(dict)
+
+  let position = 0
+
+  while (position < entries.length) {
+    yield entries.slice(position, position + batchSize)
+    position += batchSize
+  }
+}
+
+const processingBatchSize = 100
+
 function addActionConfig({
   garden,
   log,
@@ -156,29 +169,34 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG
   const computedActionModes: { [key: string]: ComputedActionMode } = {}
 
   const preprocessActions = async (predicate: (config: ActionConfig) => boolean = () => true) => {
-    return await Promise.all(
-      Object.entries(configsByKey).map(async ([key, config]) => {
-        if (!predicate(config)) {
-          return
-        }
+    let batchNo = 1
+    for (const batch of sliceToBatches(configsByKey, processingBatchSize)) {
+      log.silly(`Preprocessing actions batch #${batchNo} (${batch.length} items)`)
+      await Promise.all(
+        batch.map(async ([key, config]) => {
+          if (!predicate(config)) {
+            return
+          }
 
-        const { mode, explicitMode } = getActionMode(config, actionModes, log)
-        computedActionModes[key] = { mode, explicitMode }
-        const actionTypes = await garden.getActionTypes()
-        const definition = actionTypes[config.kind][config.type]?.spec
-        preprocessResults[key] = await preprocessActionConfig({
-          garden,
-          config,
-          configsByKey,
-          actionTypes,
-          definition,
-          router,
-          linkedSources,
-          log,
-          mode,
+          const { mode, explicitMode } = getActionMode(config, actionModes, log)
+          computedActionModes[key] = { mode, explicitMode }
+          const actionTypes = await garden.getActionTypes()
+          const definition = actionTypes[config.kind][config.type]?.spec
+          preprocessResults[key] = await preprocessActionConfig({
+            garden,
+            config,
+            configsByKey,
+            actionTypes,
+            definition,
+            router,
+            linkedSources,
+            log,
+            mode,
+          })
         })
-      })
-    )
+      )
+      batchNo++
+    }
   }
 
   // First preprocess only the Deploy actions, so we can infer the mode of Build actions that are used by them.
@@ -264,10 +282,14 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG
   log.debug(`Got ${preprocessedConfigs.length} action configs ${!!actionsFilter ? "with" : "without"} action filter`)
 
   // Optimize file scanning by avoiding unnecessarily broad scans when project is not in repo root.
-  const allPaths = preprocessedConfigs.map((c) => getSourcePath(c))
-  log.debug(`Finding minimal roots for ${allPaths.length} paths`)
+  const allPaths = new Set<string>()
+  for (const preprocessedConfig of preprocessedConfigs) {
+    const sourcePath = getSourcePath(preprocessedConfig)
+    allPaths.add(sourcePath)
+  }
+  log.debug(`Finding minimal roots for ${allPaths.size} paths`)
   const minimalRoots = await garden.vcs.getMinimalRoots(log, allPaths)
-  log.debug(`Finding minimal roots for ${allPaths.length} paths`)
+  log.debug(`Found minimal roots for ${allPaths.size} paths`)
 
   // TODO: Maybe we could optimize resolving tree versions, avoid parallel scanning of the same directory etc.
   const graph = new MutableConfigGraph({
@@ -277,51 +299,57 @@ export const actionConfigsToGraph = profileAsync(async function actionConfigsToG
     groups: groupConfigs,
   })
 
-  log.debug(`Processing ${Object.keys(preprocessResults).length} action configs...`)
-  await Promise.all(
-    Object.entries(preprocessResults).map(async ([key, res]) => {
-      const { config, linkedSource, remoteSourcePath, supportedModes, dependencies } = res
-      const { mode, explicitMode } = computedActionModes[key]
-
-      try {
-        const action = await processActionConfig({
-          garden,
-          graph,
-          config,
-          dependencies,
-          log,
-          mode,
-          linkedSource,
-          remoteSourcePath,
-          supportedModes,
-          scanRoot: minimalRoots[getSourcePath(config)],
-        })
+  const actionConfigCount = Object.keys(preprocessResults).length
+  log.debug(`Processing ${actionConfigCount} action configs...`)
+  let batchNo = 1
+  for (const batch of sliceToBatches(preprocessResults, 100)) {
+    log.silly(`Processing actions batch #${batchNo} (${batch.length} items)`)
+    await Promise.all(
+      batch.map(async ([key, res]) => {
+        const { config, linkedSource, remoteSourcePath, supportedModes, dependencies } = res
+        const { mode, explicitMode } = computedActionModes[key]
+
+        try {
+          const action = await processActionConfig({
+            garden,
+            graph,
+            config,
+            dependencies,
+            log,
+            mode,
+            linkedSource,
+            remoteSourcePath,
+            supportedModes,
+            scanRoot: minimalRoots[getSourcePath(config)],
+          })
+
+          if (!action.supportsMode(mode)) {
+            if (explicitMode) {
+              log.warn(`${action.longDescription()} is not configured for or does not support ${mode} mode`)
+            }
+          }
 
-        if (!action.supportsMode(mode)) {
-          if (explicitMode) {
-            log.warn(`${action.longDescription()} is not configured for or does not support ${mode} mode`)
+          graph.addAction(action)
+        } catch (error) {
+          if (!(error instanceof GardenError)) {
+            throw error
           }
-        }
 
-        graph.addAction(action)
-      } catch (error) {
-        if (!(error instanceof GardenError)) {
-          throw error
+          throw new ConfigurationError({
+            message:
+              styles.error(
+                `\nError processing config for ${styles.highlight(config.kind)} action ${styles.highlight(
+                  config.name
+                )}:\n`
+              ) + styles.error(error.message),
+            wrappedErrors: [error],
+          })
         }
-
-        throw new ConfigurationError({
-          message:
-            styles.error(
-              `\nError processing config for ${styles.highlight(config.kind)} action ${styles.highlight(
-                config.name
-              )}:\n`
-            ) + styles.error(error.message),
-          wrappedErrors: [error],
-        })
-      }
-    })
-  )
-  log.debug(`Processed ${Object.keys(preprocessResults).length} action configs`)
+      })
+    )
+    batchNo++
+  }
+  log.debug(`Processed ${actionConfigCount} action configs`)
 
   log.debug(`Validating the graph`)
   graph.validate()
@@ -958,7 +986,7 @@ function dependenciesFromActionConfig({
 
     if (!depConfig) {
       throw new ConfigurationError({
-        message: `${description} references depdendency ${depKey}, but no such action could be found`,
+        message: `${description} references dependency ${depKey}, but no such action could be found`,
       })
     }
 

core/src/resolve-module.ts

@@ -109,7 +109,10 @@ export class ModuleResolver {
     // remove nodes from as we complete the processing.
     const fullGraph = new DependencyGraph<string>()
     const rawConfigs = Object.values(this.rawConfigsByKey)
-    const allPaths: string[] = rawConfigs.map((c) => c.path)
+    const allPaths = new Set<string>()
+    for (const rawConfig of rawConfigs) {
+      allPaths.add(rawConfig.path)
+    }
 
     this.addModulesToGraph(fullGraph, rawConfigs)
 

core/src/util/util.ts

@@ -780,3 +780,12 @@ export async function userPrompt(params: {
 export function isValidDateInstance(d: any) {
   return !isNaN(d) && d instanceof Date
 }
+
+export function* sliceToBatches<T>(elements: T[], batchSize: number) {
+  let position = 0
+
+  while (position < elements.length) {
+    yield elements.slice(position, position + batchSize)
+    position += batchSize
+  }
+}

core/src/vcs/vcs.ts

@@ -36,6 +36,7 @@ import { getDefaultProfiler, Profile, type Profiler } from "../util/profiling.js
 import AsyncLock from "async-lock"
 import { makeDocsLinkStyled } from "../docs/common.js"
 import { RuntimeError } from "../exceptions.js"
+import { sliceToBatches } from "../util/util.js"
 
 const scanLock = new AsyncLock()
 
@@ -311,22 +312,25 @@ export abstract class VcsHandler {
    * reduces duplicate scanning of the same directories (since fewer unique roots mean
    * more tree cache hits).
    */
-  async getMinimalRoots(log: Log, paths: string[]) {
+  async getMinimalRoots(log: Log, paths: Set<string>) {
     const repoRoots: { [path: string]: string } = {}
     const outputs: { [path: string]: string } = {}
     const rootsToPaths: { [repoRoot: string]: string[] } = {}
 
-    await Promise.all(
-      paths.map(async (path) => {
-        const repoRoot = await this.getRepoRoot(log, path)
-        repoRoots[path] = repoRoot
-        if (rootsToPaths[repoRoot]) {
-          rootsToPaths[repoRoot].push(path)
-        } else {
-          rootsToPaths[repoRoot] = [path]
-        }
-      })
-    )
+    // Avoid too many concurrent git commands
+    for (const batch of sliceToBatches([...paths], 10)) {
+      await Promise.all(
+        batch.map(async (path) => {
+          const repoRoot = await this.getRepoRoot(log, path)
+          repoRoots[path] = repoRoot
+          if (rootsToPaths[repoRoot]) {
+            rootsToPaths[repoRoot].push(path)
+          } else {
+            rootsToPaths[repoRoot] = [path]
+          }
+        })
+      )
+    }
 
     for (const path of paths) {
       const repoRoot = repoRoots[path]

core/test/unit/src/config/provider.ts

@@ -23,7 +23,7 @@ describe("getProviderDependencies", () => {
       someKey: "${providers.other-provider.foo}",
       anotherKey: "foo-${providers.another-provider.bar}",
     }
-    expect(await getAllProviderDependencyNames(plugin, config)).to.eql(["another-provider", "other-provider"])
+    expect(getAllProviderDependencyNames(plugin, config)).to.eql(["another-provider", "other-provider"])
   })
 
   it("should ignore template strings that don't reference providers", async () => {
@@ -32,7 +32,7 @@ describe("getProviderDependencies", () => {
       someKey: "${providers.other-provider.foo}",
       anotherKey: "foo-${some.other.ref}",
     }
-    expect(await getAllProviderDependencyNames(plugin, config)).to.eql(["other-provider"])
+    expect(getAllProviderDependencyNames(plugin, config)).to.eql(["other-provider"])
   })
 
   it("should throw on provider-scoped template strings without a provider name", async () => {