fix: Cache results to skip superfluous tasks.

Before these changes, while e.g. running `garden deploy -w`, changing
tracked files in a module would also trigger redeploys of its
dependencies, even if their version was unchanged since their last
deployment.

To fix this, a result cache was added to TaskGraph. When a task is
added, any dependencies having cached results matching their current
VCS version (including their dirty timestamps, if any) are not added to
the graph.

Since results are cached by version in addition to their baseKey, tasks
are now initialized via an async factory method, which ensures that
their version has been fetched before they are added to the task graph.

Note that for performance reasons, some tests skip this step, using a
stubbed version and calling the constructor directly.

src/commands/build.ts

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

src/commands/deploy.ts

@@ -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 }))
       }
     })
 

src/commands/dev.ts

@@ -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)
       }

src/commands/push.ts

@@ -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)
   }
 

src/commands/run/module.ts

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

src/commands/run/service.ts

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

src/commands/run/test.ts

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

src/commands/test.ts

@@ -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

src/plugin-context.ts

@@ -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"
@@ -48,7 +49,6 @@ import {
   RuntimeContext,
   ServiceStatus,
 } from "./types/service"
-import Bluebird = require("bluebird")
 import {
   mapValues,
   toPairs,
@@ -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()
     },

src/task-graph.ts

@@ -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"
@@ -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 = {}
   }
@@ -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)
 
@@ -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()
@@ -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)
@@ -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)
+
     }
   }
 
@@ -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.
 
 /*

src/tasks/build.ts

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

src/tasks/deploy.ts

@@ -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
   }