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
…#6002)

* perf(cli): avoid unnecessary module resolution when filtering by name

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.

* perf: extend partial resolution to action graph resolution

* chore(test): fix compile-time errors

Post-rebase fix

* chore: unify log format

Print class name as a prefix in all debug/silly messages.

* refactor: simplify for-loop

* fix: use correct task type in log messages

* chore: more detailed error message on available actions

* fix: throw if no actions found by pattern matching

* Revert "fix: throw if no actions found by pattern matching"

This reverts commit 4b86e05.

* fix: ugly hack to avoid changes in behaviour of build and deploy commands

---------

Co-authored-by: Vladimir Vagaytsev <vladimir.vagaitsev@gmail.com>
  • Loading branch information
edvald and vvagaytsev committed May 14, 2024
1 parent 32653f0 commit 86c885f
Show file tree
Hide file tree
Showing 24 changed files with 1,023 additions and 144 deletions.
15 changes: 13 additions & 2 deletions core/src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { deline } from "../util/string.js"
import { isBuildAction } from "../actions/build.js"
import { warnOnLinkedActions } from "../actions/helpers.js"
import { watchParameter, watchRemovedWarning } from "./util/watch-parameter.js"
import { gardenEnv } from "../constants.js"

const buildArgs = {
names: new StringsParameter({
Expand Down Expand Up @@ -82,8 +83,18 @@ 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 })
const getBuildsParams = gardenEnv.GARDEN_ENABLE_PARTIAL_RESOLUTION
? { includeNames: args.names }
: { names: args.names }
let actions = graph.getBuilds(getBuildsParams)

if (opts["with-dependants"]) {
// Then we include build dependants (recursively) in the list of modules to build.
Expand Down
16 changes: 14 additions & 2 deletions core/src/commands/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,20 @@ 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 })
const getDeploysParams = gardenEnv.GARDEN_ENABLE_PARTIAL_RESOLUTION
? { includeNames: args.names, includeDisabled: true }
: { names: args.names, includeDisabled: true }
let deployActions = graph.getDeploys(getDeploysParams)

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
14 changes: 10 additions & 4 deletions core/src/commands/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type { CommandParams } from "./base.js"
import type { ServeCommandOpts } from "./serve.js"
import { DevCommand } from "./dev.js"
import { styles } from "../logger/styles.js"
import { gardenEnv } from "../constants.js"

/**
* Runs a `dev` command and runs `commandName` with the args & opts provided in `params` as the first
Expand Down Expand Up @@ -61,10 +62,17 @@ function printField(name: string, value: string | null) {
return `${styles.primary(name)}: ${value || ""}`
}

const renderAvailableActions = (actions: { name: string }[]): string => {
if (gardenEnv.GARDEN_ENABLE_PARTIAL_RESOLUTION) {
return "<None> (action list is not available while partial graph resolution (i.e. when GARDEN_ENABLE_PARTIAL_RESOLUTION=true))"
}

return naturalList(actions.map((a) => a.name))
}

/**
* Throws if an action by name is not found.
* Logs a warning if no actions are found matching wildcard arguments.
*
*/
export const validateActionSearchResults = ({
log,
Expand All @@ -87,9 +95,7 @@ export const validateActionSearchResults = ({
names?.forEach((n) => {
if (!isGlob(n) && !actions.find((a) => a.name === n)) {
throw new ParameterError({
message: `${actionKind} action "${n}" was not found. Available actions: ${naturalList(
allActions.map((a) => a.name)
)}`,
message: `${actionKind} action "${n}" was not found. Available actions: ${renderAvailableActions(allActions)}`,
})
}
})
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
12 changes: 10 additions & 2 deletions 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 Expand Up @@ -202,7 +210,7 @@ export class RunCommand extends Command<Args, Opts> {

const results = await garden.processTasks({ tasks, log })

return handleProcessResults(garden, log, "test", results)
return handleProcessResults(garden, log, "run", results)
}
}

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(),
}
25 changes: 20 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 @@ -1066,10 +1081,9 @@ export class Garden {
moduleGraph,
actionModes,
linkedSources,
actionsFilter,
})

// 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 +2280,5 @@ export interface GetConfigGraphParams {
graphResults?: GraphResults
emit: boolean
actionModes?: ActionModeMap
actionsFilter?: string[]
}

0 comments on commit 86c885f

Please sign in to comment.