Skip to content

Commit

Permalink
Merge pull request #97 from garden-io/redundant-redeploys-in-watch
Browse files Browse the repository at this point in the history
fix: Cache results to skip superfluous tasks.
  • Loading branch information
thsig authored May 23, 2018
2 parents 307e21b + 0632e36 commit 8cc8c63
Show file tree
Hide file tree
Showing 19 changed files with 261 additions and 78 deletions.
2 changes: 1 addition & 1 deletion src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class BuildCommand extends Command<typeof buildArguments, typeof buildOpt
ctx.log.header({ emoji: "hammer", command: "build" })

const result = await ctx.processModules(modules, opts.watch, async (module) => {
await ctx.addTask(new BuildTask(ctx, module, opts.force))
await ctx.addTask(await BuildTask.factory({ ctx, module, force: opts.force }))
})

ctx.log.info("")
Expand Down
2 changes: 1 addition & 1 deletion src/commands/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class DeployCommand extends Command<typeof deployArgs, typeof deployOpts>
const result = await ctx.processModules(modules, watch, async (module) => {
const servicesToDeploy = await module.getServices()
for (const service of servicesToDeploy) {
await ctx.addTask(new DeployTask(ctx, service, force, forceBuild))
await ctx.addTask(await DeployTask.factory({ ctx, service, force, forceBuild }))
}
})

Expand Down
2 changes: 1 addition & 1 deletion src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export class DevCommand extends Command {
const tasks = testTasks.concat(deployTasks)

if (tasks.length === 0) {
await ctx.addTask(new BuildTask(ctx, module, false))
await ctx.addTask(await BuildTask.factory({ ctx, module, force: false }))
} else {
await Bluebird.map(tasks, ctx.addTask)
}
Expand Down
2 changes: 1 addition & 1 deletion src/commands/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export async function pushModules(
)
}

const task = new PushTask(ctx, module, forceBuild)
const task = await PushTask.factory({ ctx, module, forceBuild })
await ctx.addTask(task)
}

Expand Down
2 changes: 1 addition & 1 deletion src/commands/run/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class RunModuleCommand extends Command<typeof runArgs, typeof runOpts> {

await ctx.configureEnvironment()

const buildTask = new BuildTask(ctx, module, opts["force-build"])
const buildTask = await BuildTask.factory({ ctx, module, force: opts["force-build"] })
await ctx.addTask(buildTask)
await ctx.processTasks()

Expand Down
2 changes: 1 addition & 1 deletion src/commands/run/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class RunServiceCommand extends Command<typeof runArgs, typeof runOpts> {

await ctx.configureEnvironment()

const buildTask = new BuildTask(ctx, module, opts["force-build"])
const buildTask = await BuildTask.factory({ ctx, module, force: opts["force-build"] })
await ctx.addTask(buildTask)
await ctx.processTasks()

Expand Down
2 changes: 1 addition & 1 deletion src/commands/run/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class RunTestCommand extends Command<typeof runArgs, typeof runOpts> {

await ctx.configureEnvironment()

const buildTask = new BuildTask(ctx, module, opts["force-build"])
const buildTask = await BuildTask.factory({ ctx, module, force: opts["force-build"] })
await ctx.addTask(buildTask)
await ctx.processTasks()

Expand Down
2 changes: 1 addition & 1 deletion src/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class TestCommand extends Command<typeof testArgs, typeof testOpts> {

const results = await ctx.processModules(modules, opts.watch, async (module) => {
const tasks = await module.getTestTasks({ group, force, forceBuild })
await Bluebird.map(tasks, ctx.addTask)
return Bluebird.map(tasks, ctx.addTask)
})

const failed = values(results).filter(r => !!r.error).length
Expand Down
8 changes: 4 additions & 4 deletions src/plugin-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import Bluebird = require("bluebird")
import chalk from "chalk"
import { Stream } from "ts-stream"
import { NotFoundError } from "./exceptions"
Expand Down Expand Up @@ -48,7 +49,6 @@ import {
RuntimeContext,
ServiceStatus,
} from "./types/service"
import Bluebird = require("bluebird")
import {
mapValues,
toPairs,
Expand Down Expand Up @@ -367,10 +367,10 @@ export function createPluginContext(garden: Garden): PluginContext {
deployServices: async ({ names, force = false, forceBuild = false, logEntry }) => {
const services = await ctx.getServices(names)

for (const service of services) {
const task = new DeployTask(ctx, service, force, forceBuild, logEntry)
await Bluebird.map(services, async (service) => {
const task = await DeployTask.factory({ ctx, service, force, forceBuild, logEntry })
await ctx.addTask(task)
}
})

return ctx.processTasks()
},
Expand Down
78 changes: 71 additions & 7 deletions src/task-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import * as Bluebird from "bluebird"
import chalk from "chalk"
import { pick } from "lodash"
import { merge, pick } from "lodash"
import { Task, TaskDefinitionError } from "./types/task"

import { EntryStyle, LogSymbolType } from "./logger/types"
Expand Down Expand Up @@ -55,12 +55,14 @@ export class TaskGraph {
private inProgress: TaskNodeMap
private logEntryMap: LogEntryMap

private resultCache: ResultCache
private opQueue: OperationQueue

constructor(private ctx: PluginContext, private concurrency: number = DEFAULT_CONCURRENCY) {
this.roots = new TaskNodeMap()
this.index = new TaskNodeMap()
this.inProgress = new TaskNodeMap()
this.resultCache = new ResultCache()
this.opQueue = new OperationQueue(this)
this.logEntryMap = {}
}
Expand All @@ -70,7 +72,6 @@ export class TaskGraph {
}

async addTaskInternal(task: Task) {
// TODO: Detect circular dependencies.
const predecessor = this.getPredecessor(task)
let node = this.getNode(task)

Expand Down Expand Up @@ -137,6 +138,7 @@ export class TaskGraph {
this.initLogging()

return Bluebird.map(batch, async (node: TaskNode) => {
const task = node.task
const type = node.getType()
const baseKey = node.getBaseKey()
const description = node.getDescription()
Expand All @@ -145,15 +147,21 @@ export class TaskGraph {
this.logTask(node)
this.logEntryMap.inProgress.setState(inProgressToStr(this.inProgress.getNodes()))

const dependencyBaseKeys = (await node.task.getDependencies())
.map(task => task.getBaseKey())
const dependencyBaseKeys = (await task.getDependencies())
.map(dep => dep.getBaseKey())

const dependencyResults = pick(results, dependencyBaseKeys)
const dependencyResults = merge(
this.resultCache.pick(dependencyBaseKeys),
pick(results, dependencyBaseKeys))

let result
try {
results[baseKey] = await node.process(dependencyResults)
result = await node.process(dependencyResults)
} catch (error) {
results[baseKey] = { type, description, error }
result = { type, description, error }
} finally {
results[baseKey] = result
this.resultCache.put(baseKey, task.version.versionString, result)
}
} finally {
this.completeTask(node)
Expand Down Expand Up @@ -197,9 +205,15 @@ export class TaskGraph {
private async addDependencies(node: TaskNode) {
const task = node.task
for (const d of await task.getDependencies()) {

if (this.resultCache.get(d.getBaseKey(), d.version.versionString)) {
continue
}

const dependency = this.getPredecessor(d) || this.getNode(d)
this.index.addNode(dependency)
node.addDependency(dependency)

}
}

Expand Down Expand Up @@ -390,6 +404,56 @@ class TaskNode {
}
}

interface CachedResult {
result: TaskResult,
versionString: string
}

class ResultCache {
/*
By design, at most one TaskResult (the most recently processed) is cached for a given baseKey.
Invariant: No concurrent calls are made to this class' instance methods, since they
only happen within TaskGraph's addTaskInternal and processTasksInternal methods,
which are never executed concurrently, since they are executed sequentially by the
operation queue.
*/
private cache: { [key: string]: CachedResult }

constructor() {
this.cache = {}
}

put(baseKey: string, versionString: string, result: TaskResult): void {
this.cache[baseKey] = { result, versionString }
}

get(baseKey: string, versionString: string): TaskResult | null {
const r = this.cache[baseKey]
return (r && r.versionString === versionString && !r.result.error) ? r.result : null
}

getNewest(baseKey: string): TaskResult | null {
const r = this.cache[baseKey]
return (r && !r.result.error) ? r.result : null
}

// Returns newest cached results, if any, for baseKeys
pick(baseKeys: string[]): TaskResults {
const results: TaskResults = {}

for (const baseKey of baseKeys) {
const cachedResult = this.getNewest(baseKey)
if (cachedResult) {
results[baseKey] = cachedResult
}
}

return results
}

}

// TODO: Add more typing to this class.

/*
Expand Down
37 changes: 31 additions & 6 deletions src/tasks/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,49 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import * as Bluebird from "bluebird"
import chalk from "chalk"
import { round } from "lodash"
import { PluginContext } from "../plugin-context"
import { Module } from "../types/module"
import { EntryStyle } from "../logger/types"
import { BuildResult } from "../types/plugin"
import { Task } from "../types/task"
import { Task, TaskParams, TaskVersion } from "../types/task"

export class BuildTask<T extends Module> extends Task {
export interface BuildTaskParams extends TaskParams {
ctx: PluginContext
module: Module
force: boolean
}

export class BuildTask extends Task {
type = "build"

constructor(private ctx: PluginContext, private module: T, private force: boolean) {
super()
private ctx: PluginContext
private module: Module
private force: boolean

constructor(initArgs: BuildTaskParams & TaskVersion) {
super(initArgs)
this.ctx = initArgs.ctx
this.module = initArgs.module
this.force = initArgs.force
}

async getDependencies() {
/*
TODO: Replace with a generic factory method on the Task class to avoid repetition. This applies equally to other
child classes of Task that implement an equivalent factory method.
*/
static async factory(initArgs: BuildTaskParams): Promise<BuildTask> {
initArgs.version = await initArgs.module.getVersion()
return new BuildTask(<BuildTaskParams & TaskVersion>initArgs)
}

async getDependencies(): Promise<BuildTask[]> {
const deps = await this.module.getBuildDependencies()
return deps.map(<M extends Module>(m: M) => new BuildTask(this.ctx, m, this.force))
return Bluebird.map(deps, async (m: Module) => {
return BuildTask.factory({ ctx: this.ctx, module: m, force: this.force })
})
}

protected getName() {
Expand Down
51 changes: 38 additions & 13 deletions src/tasks/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,62 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import * as Bluebird from "bluebird"
import chalk from "chalk"
import { LogEntry } from "../logger"
import { PluginContext } from "../plugin-context"
import { BuildTask } from "./build"
import { Task } from "../types/task"
import { Task, TaskParams, TaskVersion } from "../types/task"
import {
Service,
ServiceStatus,
} from "../types/service"
import { EntryStyle } from "../logger/types"
import chalk from "chalk"

export class DeployTask<T extends Service<any>> extends Task {
export interface DeployTaskParams extends TaskParams {
ctx: PluginContext
service: Service
force: boolean
forceBuild: boolean
logEntry?: LogEntry
}

export class DeployTask extends Task {
type = "deploy"

constructor(
private ctx: PluginContext,
private service: T,
private force: boolean,
private forceBuild: boolean,
private logEntry?: LogEntry) {
super()
private ctx: PluginContext
private service: Service
private force: boolean
private forceBuild: boolean
private logEntry?: LogEntry

constructor(initArgs: DeployTaskParams & TaskVersion) {
super(initArgs)
this.ctx = initArgs.ctx
this.service = initArgs.service
this.force = initArgs.force
this.forceBuild = initArgs.forceBuild
this.logEntry = initArgs.logEntry
}

static async factory(initArgs: DeployTaskParams): Promise<DeployTask> {
initArgs.version = await initArgs.service.module.getVersion()
return new DeployTask(<DeployTaskParams & TaskVersion>initArgs)
}

async getDependencies() {
const serviceDeps = this.service.config.dependencies
const services = await this.ctx.getServices(serviceDeps)
const deps: Task[] = services.map((s) => {
return new DeployTask(this.ctx, s, this.force, this.forceBuild)
const deps: Task[] = await Bluebird.map(services, async (service) => {
return DeployTask.factory({
service,
ctx: this.ctx,
force: this.force,
forceBuild: this.forceBuild,
})
})

deps.push(new BuildTask(this.ctx, this.service.module, this.forceBuild))
deps.push(await BuildTask.factory({ ctx: this.ctx, module: this.service.module, force: this.forceBuild }))
return deps
}

Expand Down
Loading

0 comments on commit 8cc8c63

Please sign in to comment.