Skip to content

Commit

Permalink
perf(cli): avoid unnecessary module resolution when filtering by name
Browse files Browse the repository at this point in the history
This applies to most common usages of the `build`, `deploy`, `run` and `test`
commands when one or more names are specified, as well as in the
`get modules` and `get actions` commands.

For now this is enabled specifically with by setting the
`GARDEN_ENABLE_PARTIAL_RESOLUTION=true` env variable.
  • Loading branch information
edvald committed May 6, 2024
1 parent b72589d commit 668abf1
Show file tree
Hide file tree
Showing 20 changed files with 892 additions and 99 deletions.
11 changes: 9 additions & 2 deletions core/src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,15 @@ export class BuildCommand extends Command<Args, Opts> {

await garden.clearBuilds()

const graph = await garden.getConfigGraph({ log, emit: true })
let actions = graph.getBuilds({ names: args.names })
let actionsFilter: string[] | undefined = undefined

// TODO: Support partial module resolution with --with-dependants
if (args.names && !opts["with-dependants"]) {
actionsFilter = args.names.map((name) => `build.${name}`)
}

const graph = await garden.getConfigGraph({ log, emit: true, actionsFilter })
let actions = graph.getBuilds({ includeNames: args.names })

if (opts["with-dependants"]) {
// Then we include build dependants (recursively) in the list of modules to build.
Expand Down
13 changes: 11 additions & 2 deletions core/src/commands/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,17 @@ export class DeployCommand extends Command<Args, Opts> {
sync: opts.sync?.length === 0 ? ["deploy.*"] : opts.sync?.map((s) => "deploy." + s),
}

const graph = await garden.getConfigGraph({ log, emit: true, actionModes })
let deployActions = graph.getDeploys({ names: args.names, includeDisabled: true })
let actionsFilter: string[] | undefined = undefined

// TODO: Optimize partial module resolution further when --skip-dependencies=true
// TODO: Optimize partial resolution further with --skip flag
// TODO: Support partial module resolution with --with-dependants
if (args.names && !opts["with-dependants"]) {
actionsFilter = args.names.map((name) => `deploy.${name}`)
}

const graph = await garden.getConfigGraph({ log, emit: true, actionModes, actionsFilter })
let deployActions = graph.getDeploys({ includeNames: args.names, includeDisabled: true })

const disabled = deployActions.filter((s) => s.isDisabled()).map((s) => s.name)

Expand Down
37 changes: 24 additions & 13 deletions core/src/commands/get/get-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,13 @@ export class GetActionsCommand extends Command {
Examples:
garden get actions # list all actions in the project
garden get actions --include-state # list all actions in the project with state in output
garden get actions --detail # list all actions in project with detailed info
garden get actions --kind deploy # only list the actions of kind 'Deploy'
garden get actions A B --kind build --sort type # list actions A and B of kind 'Build' sorted by type
garden get actions --include-state -o=json # get json output
garden get actions # list all actions in the project
garden get actions --include-state # list all actions in the project with state in output
garden get actions --detail # list all actions in project with detailed info
garden get actions --kind deploy # only list the actions of kind 'Deploy'
garden get actions a b --kind build --sort type # list actions 'a' and 'b' of kind 'Build' sorted by type
garden get actions build.a deploy.b # list actions 'build.a' and 'deploy.b'
garden get actions --include-state -o=json # get json output
`

override arguments = getActionsArgs
Expand All @@ -137,30 +138,40 @@ export class GetActionsCommand extends Command {
args,
opts,
}: CommandParams<Args, Opts>): Promise<CommandResult<GetActionsCommandResult>> {
const { names: keys } = args
const includeStateInOutput = opts["include-state"]
const isOutputDetailed = opts["detail"]
const router = await garden.getActionRouter()
const graph = await garden.getResolvedConfigGraph({ log, emit: false })

let actionsFilter: string[] | undefined = undefined

if (args.names && opts.kind) {
actionsFilter = args.names.map((name) => `${opts.kind}.${name}`)
} else if (args.names) {
actionsFilter = args.names
} else if (opts.kind) {
actionsFilter = [opts.kind + ".*"]
}

const graph = await garden.getResolvedConfigGraph({ log, emit: false, actionsFilter })

const kindOpt = opts["kind"]?.toLowerCase()
let actions: ResolvedActionWithState[] = []

switch (kindOpt) {
case "build":
actions = graph.getBuilds({ names: keys })
actions = graph.getBuilds({ includeNames: args.names })
break
case "deploy":
actions = graph.getDeploys({ names: keys })
actions = graph.getDeploys({ includeNames: args.names })
break
case "run":
actions = graph.getRuns({ names: keys })
actions = graph.getRuns({ includeNames: args.names })
break
case "test":
actions = graph.getTests({ names: keys })
actions = graph.getTests({ includeNames: args.names })
break
default:
actions = graph.getActions({ refs: keys })
actions = graph.getActions({ refs: args.names })
break
}

Expand Down
8 changes: 7 additions & 1 deletion core/src/commands/get/get-modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,13 @@ export class GetModulesCommand extends Command {
}

async action({ garden, log, args, opts }: CommandParams<Args, Opts>) {
const graph = await garden.getConfigGraph({ log, emit: false })
let actionsFilter: string[] | undefined = undefined

if (args.modules) {
actionsFilter = args.modules.map((name) => `build.${name}`)
}

const graph = await garden.getConfigGraph({ log, emit: false, actionsFilter })

const modules = sortBy(
graph.getModules({ names: args.modules, includeDisabled: !opts["exclude-disabled"] }),
Expand Down
2 changes: 1 addition & 1 deletion core/src/commands/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class PluginsCommand extends Command<Args> {
let graph = new ConfigGraph({
environmentName: garden.environmentName,
actions: [],
moduleGraph: new ModuleGraph([], {}),
moduleGraph: new ModuleGraph({ modules: [], moduleTypes: {} }),
groups: [],
})

Expand Down
10 changes: 9 additions & 1 deletion core/src/commands/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,15 @@ export class RunCommand extends Command<Args, Opts> {
await watchRemovedWarning(garden, log)
}

const graph = await garden.getConfigGraph({ log, emit: true })
let actionsFilter: string[] | undefined = undefined

// TODO: Optimize partial module resolution further when --skip-dependencies=true
// TODO: Optimize partial resolution further with --skip flag
if (args.names) {
actionsFilter = args.names.map((name) => `run.${name}`)
}

const graph = await garden.getConfigGraph({ log, emit: true, actionsFilter })

const force = opts.force
const skipRuntimeDependencies = opts["skip-dependencies"]
Expand Down
19 changes: 16 additions & 3 deletions core/src/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,13 +149,26 @@ export class TestCommand extends Command<Args, Opts> {
)
}

const graph = await garden.getConfigGraph({ log, emit: true })
let actionsFilter: string[] | undefined = undefined

// TODO: Optimize partial resolution further when --skip-dependencies=true
// TODO: Optimize partial resolution further with --skip flag
if (args.names) {
actionsFilter = args.names.map((name) => `test.${name}`)
}

if (opts.module) {
actionsFilter = [...(actionsFilter || []), `test.${opts.module}-*`]
}

const graph = await garden.getConfigGraph({ log, emit: true, actionsFilter })

let names: string[] | undefined = undefined
const nameArgs = [...(args.names || []), ...(opts.name || []).map((n) => `*-${n}`)]
const force = opts.force
const skipRuntimeDependencies = opts["skip-dependencies"]

let names: string[] | undefined = undefined
const nameArgs = [...(args.names || []), ...(opts.name || []).map((n) => `*-${n}`)]

if (nameArgs.length > 0) {
names = nameArgs
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/config/template-contexts/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ export class OutputConfigContext extends ProviderConfigContext {
.description("Retrieve information about modules that are defined in the project.")
.meta({ keyPlaceholder: "<module-name>" })
)
public modules: Map<string, ConfigContext>
public modules: Map<string, ModuleReferenceContext | ErrorContext>

@schema(
RuntimeConfigContext.getSchema().description(
Expand Down
1 change: 1 addition & 0 deletions core/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,5 @@ export const gardenEnv = {
// GARDEN_CLOUD_BUILDER will always override the config; That's why it doesn't have a default.
// FIXME: If the environment variable is not set, asBool returns undefined, unlike the type suggests. That's why we cast to `boolean | undefined`.
GARDEN_CLOUD_BUILDER: env.get("GARDEN_CLOUD_BUILDER").required(false).asBool() as boolean | undefined,
GARDEN_ENABLE_PARTIAL_RESOLUTION: env.get("GARDEN_ENABLE_PARTIAL_RESOLUTION").required(false).asBool(),
}
24 changes: 19 additions & 5 deletions core/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,22 @@ export class Garden {
@OtelTraced({
name: "getConfigGraph",
})
async getConfigGraph({ log, graphResults, emit, actionModes = {} }: GetConfigGraphParams): Promise<ConfigGraph> {
async getConfigGraph({
log,
graphResults,
emit,
actionModes = {},
/**
* If provided, this is used to perform partial module resolution.
* TODO: also limit action resolution (less important because it's faster and more done on-demand)
*/
actionsFilter,
}: GetConfigGraphParams): Promise<ConfigGraph> {
// Feature-flagging this for now
if (!gardenEnv.GARDEN_ENABLE_PARTIAL_RESOLUTION) {
actionsFilter = undefined
}

// TODO: split this out of the Garden class
await this.scanAndAddConfigs()

Expand All @@ -1002,12 +1017,12 @@ export class Garden {
graphResults,
})

const resolvedModules = await resolver.resolveAll()
const { resolvedModules, skipped } = await resolver.resolve({ actionsFilter })

// Validate the module dependency structure. This will throw on failure.
const router = await this.getActionRouter()
const moduleTypes = await this.getModuleTypes()
const moduleGraph = new ModuleGraph(resolvedModules, moduleTypes)
const moduleGraph = new ModuleGraph({ modules: resolvedModules, moduleTypes, skippedKeys: skipped })

// Require include/exclude on modules if their paths overlap
const overlaps = detectModuleOverlap({
Expand Down Expand Up @@ -1068,8 +1083,6 @@ export class Garden {
linkedSources,
})

// TODO-0.13.1: detect overlap on Build actions

// Walk through all plugins in dependency order, and allow them to augment the graph
const plugins = keyBy(await this.getAllPlugins(), "name")

Expand Down Expand Up @@ -2266,4 +2279,5 @@ export interface GetConfigGraphParams {
graphResults?: GraphResults
emit: boolean
actionModes?: ActionModeMap
actionsFilter?: string[]
}
19 changes: 19 additions & 0 deletions core/src/graph/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,25 @@ export class DependencyGraph<T> extends DepGraph<T> {
}
}

keys() {
return Object.keys(this["nodes"])
}

/**
* Returns a clone of the graph.
* Overriding base implementation to retain the same class type.
*/
override clone() {
const result = new DependencyGraph<T>()
const keys = Object.keys(this["nodes"])
for (const n of keys) {
result["nodes"][n] = this["nodes"][n]
result["outgoingEdges"][n] = [...this["outgoingEdges"][n]]
result["incomingEdges"][n] = [...this["incomingEdges"][n]]
}
return result
}

/**
* Returns an error if cycles were found.
*/
Expand Down
38 changes: 35 additions & 3 deletions core/src/graph/modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,15 @@ export class ModuleGraph {
[key: string]: EntityConfigEntry<"test", TestConfig>
}

constructor(modules: GardenModule[], moduleTypes: ModuleTypeMap) {
constructor({
modules,
moduleTypes,
skippedKeys,
}: {
modules: GardenModule[]
moduleTypes: ModuleTypeMap
skippedKeys?: Set<string>
}) {
this.dependencyGraph = {}
this.modules = {}
this.serviceConfigs = {}
Expand Down Expand Up @@ -130,6 +138,10 @@ export class ModuleGraph {
})
}

if (skippedKeys?.has(`deploy.${serviceName}`)) {
continue
}

this.serviceConfigs[serviceName] = { type: "service", moduleKey, config: serviceConfig }
}

Expand All @@ -151,11 +163,15 @@ export class ModuleGraph {
})
}

if (skippedKeys?.has(`run.${taskName}`)) {
continue
}

this.taskConfigs[taskName] = { type: "task", moduleKey, config: taskConfig }
}
}

detectMissingDependencies(Object.values(this.modules))
detectMissingDependencies(Object.values(this.modules), skippedKeys)

// Add relations between nodes
for (const module of modules) {
Expand Down Expand Up @@ -183,6 +199,10 @@ export class ModuleGraph {

// Service dependencies
for (const serviceConfig of module.serviceConfigs) {
if (skippedKeys?.has(`deploy.${serviceConfig.name}`)) {
continue
}

const serviceNode = this.getNode(
"deploy",
serviceConfig.name,
Expand Down Expand Up @@ -210,6 +230,10 @@ export class ModuleGraph {

// Task dependencies
for (const taskConfig of module.taskConfigs) {
if (skippedKeys?.has(`run.${taskConfig.name}`)) {
continue
}

const taskNode = this.getNode("run", taskConfig.name, moduleKey, module.disabled || taskConfig.disabled)

if (needsBuild) {
Expand All @@ -232,6 +256,10 @@ export class ModuleGraph {

// Test dependencies
for (const testConfig of module.testConfigs) {
if (skippedKeys?.has(`test.${module.name}-${testConfig.name}`)) {
continue
}

const testConfigName = module.name + "." + testConfig.name

this.testConfigs[testConfigName] = { type: "test", moduleKey, config: testConfig }
Expand Down Expand Up @@ -704,7 +732,7 @@ export class ModuleDependencyGraphNode {
* Looks for dependencies on non-existent modules, services or tasks, and throws a ConfigurationError
* if any were found.
*/
export function detectMissingDependencies(moduleConfigs: ModuleConfig[]) {
export function detectMissingDependencies(moduleConfigs: ModuleConfig[], skippedKeys?: Set<string>) {
const moduleNames: Set<string> = new Set(moduleConfigs.map((m) => m.name))
const serviceNames = moduleConfigs.flatMap((m) => m.serviceConfigs.map((s) => s.name))
const taskNames = moduleConfigs.flatMap((m) => m.taskConfigs.map((t) => t.name))
Expand All @@ -729,6 +757,10 @@ export function detectMissingDependencies(moduleConfigs: ModuleConfig[]) {
for (const [configKey, entityName] of runtimeDepTypes) {
for (const config of m[configKey]) {
for (const missingRuntimeDep of config.dependencies.filter((d: string) => !runtimeNames.has(d))) {
if (skippedKeys?.has(`deploy.${missingRuntimeDep}`) || skippedKeys?.has(`run.${missingRuntimeDep}`)) {
// Don't flag missing dependencies that are explicitly skipped during resolution
continue
}
missingDepDescriptions.push(deline`
${entityName} '${config.name}' (in module '${m.name}'): Unknown service or task '${missingRuntimeDep}'
referenced in dependencies.`)
Expand Down
Loading

0 comments on commit 668abf1

Please sign in to comment.