Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
May 14, 2024
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}-*`]
}
Comment on lines +160 to +162
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like garden run command also support --module option. Should we implement the same logic for the Run actions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, didn't notice that. I'd say not strictly necessary for that one, feels more relevant for tests (since it's more natural to run all tests for a module than to run all tasks).


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[]
}
Loading