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

fix(profiler): fix sync profiler and collect more data #6586

Merged
merged 3 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions core/src/config/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -571,12 +571,14 @@ export const loadVarfile = profileAsync(async function loadVarfile({
path,
defaultPath,
optional = false,
log,
}: {
// project root (when resolving project config) or module root (when resolving module config)
configRoot: string
path: string | undefined
defaultPath: string | undefined
optional?: boolean
log?: Log
}): Promise<PrimitiveMap> {
if (!path && !defaultPath) {
throw new ParameterError({
Expand All @@ -599,6 +601,7 @@ export const loadVarfile = profileAsync(async function loadVarfile({

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())

Expand Down
3 changes: 3 additions & 0 deletions core/src/config/template-contexts/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { isPrimitive, joi, joiIdentifier } from "../common.js"
import { KeyedSet } from "../../util/keyed-set.js"
import { naturalList } from "../../util/string.js"
import { styles } from "../../logger/styles.js"
import { Profile } from "../../util/profiling.js"

export type ContextKeySegment = string | number
export type ContextKey = ContextKeySegment[]
Expand Down Expand Up @@ -52,10 +53,12 @@ export function schema(joiSchema: Joi.Schema) {

export interface ConfigContextType {
new (...params: any[]): ConfigContext

getSchema(): CustomObjectSchema
}

// Note: we're using classes here to be able to use decorators to describe each context node and key
@Profile()
export abstract class ConfigContext {
private readonly _rootContext: ConfigContext
private readonly _resolvedValues: { [path: string]: any }
Expand Down
8 changes: 5 additions & 3 deletions core/src/graph/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ import { minimatch } from "minimatch"
import type { ConfigContext } from "../config/template-contexts/base.js"
import type { LinkedSource, LinkedSourceMap } from "../config-store/local.js"
import { relative } from "path"
import { profileAsync } from "../util/profiling.js"
import { profile, profileAsync } from "../util/profiling.js"
import { uuidv4 } from "../util/random.js"
import { getSourcePath } from "../vcs/vcs.js"
import { styles } from "../logger/styles.js"
Expand Down Expand Up @@ -532,6 +532,7 @@ export const processActionConfig = profileAsync(async function processActionConf
basePath: effectiveConfigFileLocation,
variables: config.variables,
varfiles: config.varfiles,
log,
})
const mergeVarsEnd = new Date().getTime()
const actionKey = actionReferenceToString(config)
Expand Down Expand Up @@ -765,6 +766,7 @@ export const preprocessActionConfig = profileAsync(async function preprocessActi
basePath: config.internal.basePath,
variables: config.variables,
varfiles: resolvedVarFiles,
log,
})
const mergeVarsEnd = new Date().getTime()
const varfilesDesc =
Expand Down Expand Up @@ -984,7 +986,7 @@ export const preprocessActionConfig = profileAsync(async function preprocessActi
}
})

function dependenciesFromActionConfig({
const dependenciesFromActionConfig = profile(function dependenciesFromActionConfig({
log,
config,
configsByKey,
Expand Down Expand Up @@ -1141,4 +1143,4 @@ function dependenciesFromActionConfig({
}

return deps
}
})
5 changes: 4 additions & 1 deletion core/src/graph/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import type { ActionKind } from "../plugin/action-types.js"
import { loadVarfile } from "../config/base.js"
import type { DeepPrimitiveMap, Varfile } from "../config/common.js"
import type { Task } from "../tasks/base.js"
import type { LogMetadata, TaskLogStatus } from "../logger/log-entry.js"
import type { Log, LogMetadata, TaskLogStatus } from "../logger/log-entry.js"

// Shared type used by ConfigGraph and TaskGraph to facilitate circular dependency detection
export type DependencyGraphNode = {
Expand Down Expand Up @@ -150,10 +150,12 @@ export const mergeVariables = profileAsync(async function mergeVariables({
basePath,
variables,
varfiles,
log,
}: {
basePath: string
variables?: DeepPrimitiveMap
varfiles?: Varfile[]
log: Log
}) {
const varsByFile = await Promise.all(
(varfiles || []).map((varfile) => {
Expand All @@ -163,6 +165,7 @@ export const mergeVariables = profileAsync(async function mergeVariables({
path,
defaultPath: undefined,
optional,
log,
})
})
)
Expand Down
2 changes: 2 additions & 0 deletions core/src/router/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import { ActionConfigContext, ActionSpecContext } from "../config/template-conte
import type { NamespaceStatus } from "../types/namespace.js"
import { TemplatableConfigContext } from "../config/template-contexts/project.js"
import type { ParamsBase } from "../plugin/handlers/base/base.js"
import { Profile } from "../util/profiling.js"

export type CommonParams = keyof PluginActionContextParams

Expand All @@ -54,6 +55,7 @@ export interface BaseRouterParams {
/**
* The ProviderRouter is extended upon in BaseAction-, Module- and Provider routers.
*/
@Profile()
export abstract class BaseRouter {
protected readonly garden: Garden
protected readonly configuredPlugins: GardenPluginSpec[]
Expand Down
8 changes: 7 additions & 1 deletion core/src/tasks/resolve-action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,12 @@ export class ResolveActionTask<T extends Action> extends BaseActionTask<T, Resol
const group = this.graph.getGroup(groupName)

groupVariables = resolveTemplateStrings({
value: await mergeVariables({ basePath: group.path, variables: group.variables, varfiles: group.varfiles }),
value: await mergeVariables({
basePath: group.path,
variables: group.variables,
varfiles: group.varfiles,
log: this.garden.log,
}),
context: inputsContext,
// TODO: map variables to their source
source: undefined,
Expand All @@ -164,6 +169,7 @@ export class ResolveActionTask<T extends Action> extends BaseActionTask<T, Resol
basePath,
variables: config.variables,
varfiles: config.varfiles,
log: this.garden.log,
}),
context: new ActionSpecContext({
garden: this.garden,
Expand Down
48 changes: 28 additions & 20 deletions core/src/template-string/template-string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,11 @@ const shouldUnescape = (ctxOpts: ContextResolveOpts) => {
return !!ctxOpts.unescape || !ctxOpts.allowPartial
}

type ParseParams = Parameters<typeof parser.parse>
const parseWithPegJs = profile(function parseWithPegJs(params: ParseParams) {
return parser.parse(...params)
})

/**
* Parse and resolve a templated string, with the given context. The template format is similar to native JS templated
* strings but only supports simple lookups from the given context, e.g. "prefix-${nested.key}-suffix", and not
Expand All @@ -130,27 +135,30 @@ export const resolveTemplateString = profile(function resolveTemplateString({
}

try {
const parsed = parser.parse(string, {
getKey: (key: string[], resolveOpts?: ContextResolveOpts) => {
return context.resolve({ key, nodePath: [], opts: { ...contextOpts, ...(resolveOpts || {}) } })
const parsed = parseWithPegJs([
string,
{
getKey: (key: string[], resolveOpts?: ContextResolveOpts) => {
return context.resolve({ key, nodePath: [], opts: { ...contextOpts, ...(resolveOpts || {}) } })
},
getValue,
resolveNested: (nested: string) => resolveTemplateString({ string: nested, context, contextOpts }),
buildBinaryExpression,
buildLogicalExpression,
isArray: Array.isArray,
ConfigurationError,
TemplateStringError,
missingKeyExceptionType,
passthroughExceptionType,
allowPartial: !!contextOpts.allowPartial,
unescape: shouldUnescape(contextOpts),
escapePrefix,
optionalSuffix: "}?",
isPlainObject,
isPrimitive,
callHelperFunction,
},
getValue,
resolveNested: (nested: string) => resolveTemplateString({ string: nested, context, contextOpts }),
buildBinaryExpression,
buildLogicalExpression,
isArray: Array.isArray,
ConfigurationError,
TemplateStringError,
missingKeyExceptionType,
passthroughExceptionType,
allowPartial: !!contextOpts.allowPartial,
unescape: shouldUnescape(contextOpts),
escapePrefix,
optionalSuffix: "}?",
isPlainObject,
isPrimitive,
callHelperFunction,
})
])

const outputs: ResolvedClause[] = parsed.map((p: any) => {
return isPlainObject(p) ? p : { resolved: getValue(p) }
Expand Down
2 changes: 1 addition & 1 deletion core/src/util/profiling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,8 @@ export const profile = <T extends Array<any>, U>(fn: (...args: T) => U, profiler
const timingKey = fn.name

return (...args: T): U => {
const result = fn(...args)
const start = performance.now()
const result = fn(...args)

if (!profiler!.isEnabled()) {
return result
Expand Down