improvement(core): don't watch dev-enabled modules

This is a performance optimization that lowers resource use when using
dev mode. We now don't watch files inside the module roots of modules
having one or more services deployed with dev mode, except for the
modules' Garden config paths.

The rationale here is that FS watches can be resource intensive,
especially when there's a lot of churn (and especially on Linux). Since
Mutagen takes care of the dev-mode sync (and doesn't go through the
framework-native watch logic), this is usually unwanted overhead.

core/src/commands/deploy.ts

@@ -8,6 +8,7 @@
 
 import deline = require("deline")
 import dedent = require("dedent")
+import chalk = require("chalk")
 
 import {
   Command,
@@ -22,11 +23,15 @@ import { getModuleWatchTasks } from "../tasks/helpers"
 import { processModules } from "../process"
 import { printHeader } from "../logger/util"
 import { BaseTask } from "../tasks/base"
-import { getDevModeServiceNames, getHotReloadServiceNames, validateHotReloadServiceNames } from "./helpers"
+import {
+  getDevModeModules,
+  getDevModeServiceNames,
+  getHotReloadServiceNames,
+  validateHotReloadServiceNames,
+} from "./helpers"
 import { startServer } from "../server/server"
 import { DeployTask } from "../tasks/deploy"
 import { naturalList } from "../util/string"
-import chalk = require("chalk")
 import { StringsParameter, BooleanParameter, ParameterValues } from "../cli/params"
 import { Garden } from "../garden"
 
@@ -165,8 +170,8 @@ export class DeployCommand extends Command<Args, Opts> {
     }
 
     const modules = Array.from(new Set(services.map((s) => s.module)))
-    const devModeServiceNames = await getDevModeServiceNames(opts["dev-mode"], initGraph)
-    const hotReloadServiceNames = await getHotReloadServiceNames(opts["hot-reload"], initGraph)
+    const devModeServiceNames = getDevModeServiceNames(opts["dev-mode"], initGraph)
+    const hotReloadServiceNames = getHotReloadServiceNames(opts["hot-reload"], initGraph)
 
     let watch = opts.watch
 
@@ -176,7 +181,7 @@ export class DeployCommand extends Command<Args, Opts> {
 
     if (hotReloadServiceNames.length > 0) {
       initGraph.getServices({ names: hotReloadServiceNames }) // validate the existence of these services
-      const errMsg = await validateHotReloadServiceNames(hotReloadServiceNames, initGraph)
+      const errMsg = validateHotReloadServiceNames(hotReloadServiceNames, initGraph)
       if (errMsg) {
         log.error({ msg: errMsg })
         return { result: { builds: {}, deployments: {}, tests: {}, graphResults: {} } }
@@ -209,6 +214,7 @@ export class DeployCommand extends Command<Args, Opts> {
       footerLog,
       modules,
       initialTasks,
+      skipWatchModules: getDevModeModules(devModeServiceNames, initGraph),
       watch,
       changeHandler: async (graph, module) => {
         const tasks: BaseTask[] = await getModuleWatchTasks({

core/src/commands/dev.ts

@@ -22,7 +22,7 @@ import { processModules } from "../process"
 import { GardenModule } from "../types/module"
 import { getTestTasks } from "../tasks/test"
 import { ConfigGraph } from "../config-graph"
-import { getHotReloadServiceNames, validateHotReloadServiceNames } from "./helpers"
+import { getDevModeModules, getHotReloadServiceNames, validateHotReloadServiceNames } from "./helpers"
 import { startServer } from "../server/server"
 import { BuildTask } from "../tasks/build"
 import { DeployTask } from "../tasks/deploy"
@@ -140,9 +140,9 @@ export class DevCommand extends Command<DevCommandArgs, DevCommandOpts> {
       return {}
     }
 
-    const hotReloadServiceNames = await getHotReloadServiceNames(opts["hot-reload"], graph)
+    const hotReloadServiceNames = getHotReloadServiceNames(opts["hot-reload"], graph)
     if (hotReloadServiceNames.length > 0) {
-      const errMsg = await validateHotReloadServiceNames(hotReloadServiceNames, graph)
+      const errMsg = validateHotReloadServiceNames(hotReloadServiceNames, graph)
       if (errMsg) {
         log.error({ msg: errMsg })
         return { result: {} }
@@ -176,6 +176,7 @@ export class DevCommand extends Command<DevCommandArgs, DevCommandOpts> {
       modules,
       watch: true,
       initialTasks,
+      skipWatchModules: getDevModeModules(devModeServiceNames, graph),
       changeHandler: async (updatedGraph: ConfigGraph, module: GardenModule) => {
         return getDevCommandWatchTasks({
           garden,

core/src/commands/helpers.ts

@@ -15,8 +15,9 @@ import { GardenModule } from "../types/module"
 import { GardenService } from "../types/service"
 import { GardenTask } from "../types/task"
 import { GardenTest } from "../types/test"
+import { uniqByName } from "../util/util"
 
-export async function getDevModeServiceNames(namesFromOpt: string[] | undefined, configGraph: ConfigGraph) {
+export function getDevModeServiceNames(namesFromOpt: string[] | undefined, configGraph: ConfigGraph) {
   const names = namesFromOpt || []
   if (names.includes("*") || (!!namesFromOpt && namesFromOpt.length === 0)) {
     return configGraph.getServices().map((s) => s.name)
@@ -25,7 +26,7 @@ export async function getDevModeServiceNames(namesFromOpt: string[] | undefined,
   }
 }
 
-export async function getHotReloadServiceNames(namesFromOpt: string[] | undefined, configGraph: ConfigGraph) {
+export function getHotReloadServiceNames(namesFromOpt: string[] | undefined, configGraph: ConfigGraph) {
   const names = namesFromOpt || []
   if (names.includes("*")) {
     return configGraph
@@ -37,14 +38,15 @@ export async function getHotReloadServiceNames(namesFromOpt: string[] | undefine
   }
 }
 
+export function getDevModeModules(devModeServiceNames: string[], graph: ConfigGraph): GardenModule[] {
+  return uniqByName(graph.getServices({ names: devModeServiceNames }).map((s) => s.module))
+}
+
 /**
  * Returns an error message string if one or more serviceNames refers to a service that's not configured for
  * hot reloading, or if one or more of serviceNames referes to a non-existent service. Returns null otherwise.
  */
-export async function validateHotReloadServiceNames(
-  serviceNames: string[],
-  configGraph: ConfigGraph
-): Promise<string | null> {
+export function validateHotReloadServiceNames(serviceNames: string[], configGraph: ConfigGraph): string | null {
   const services = configGraph.getServices({ names: serviceNames, includeDisabled: true })
 
   const notHotreloadable = services.filter((s) => !supportsHotReloading(s)).map((s) => s.name)

core/src/garden.ts

@@ -8,7 +8,7 @@
 
 import Bluebird from "bluebird"
 import chalk from "chalk"
-import { ensureDir } from "fs-extra"
+import { ensureDir, readdir } from "fs-extra"
 import dedent from "dedent"
 import { platform, arch } from "os"
 import { relative, resolve, join } from "path"
@@ -352,11 +352,37 @@ export class Garden {
    * Enables the file watcher for the project.
    * Make sure to stop it using `.close()` when cleaning up or when watching is no longer needed.
    */
-  async startWatcher(graph: ConfigGraph, bufferInterval?: number) {
+  async startWatcher({
+    graph,
+    skipModules,
+    bufferInterval,
+  }: {
+    graph: ConfigGraph
+    skipModules?: GardenModule[]
+    bufferInterval?: number
+  }) {
     const modules = graph.getModules()
     const linkedPaths = (await getLinkedSources(this)).map((s) => s.path)
     const paths = [this.projectRoot, ...linkedPaths]
-    this.watcher = new Watcher(this, this.log, paths, modules, bufferInterval)
+
+    // For skipped modules (e.g. those with services in dev mode), we skip watching all files and folders in the
+    // module root except for the module's config path. This way, we can still react to changes in the module's
+    // configuration.
+    const skipPaths = flatten(
+      await Bluebird.map(skipModules || [], async (skipped: GardenModule) => {
+        return (await readdir(skipped.path))
+          .map((relPath) => resolve(skipped.path, relPath))
+          .filter((absPath) => absPath !== skipped.configPath)
+      })
+    )
+    this.watcher = new Watcher({
+      garden: this,
+      log: this.log,
+      paths,
+      modules,
+      skipPaths,
+      bufferInterval,
+    })
   }
 
   async getPlugin(pluginName: string): Promise<GardenPlugin> {

core/src/process.ts

@@ -30,6 +30,10 @@ interface ProcessParams {
   log: LogEntry
   footerLog?: LogEntry
   watch: boolean
+  /**
+   * If provided, and if `watch === true`, don't watch files in the module roots of these modules.
+   */
+  skipWatchModules?: GardenModule[]
   initialTasks: BaseTask[]
   /**
    * Use this if the behavior should be different on watcher changes than on initial processing
@@ -53,6 +57,7 @@ export async function processModules({
   footerLog,
   modules,
   initialTasks,
+  skipWatchModules,
   watch,
   changeHandler,
 }: ProcessModulesParams): Promise<ProcessResults> {
@@ -66,7 +71,7 @@ export async function processModules({
 
   if (linkedModulesMsg.length > 0) {
     log.info(renderDivider())
-    log.info(chalk.gray(`Following modules are linked to a local path:\n${linkedModulesMsg.join("\n")}`))
+    log.info(chalk.gray(`The following modules are linked to a local path:\n${linkedModulesMsg.join("\n")}`))
     log.info(renderDivider())
   }
 
@@ -119,7 +124,7 @@ export async function processModules({
   const modulesToWatch = uniqByName(deps.build.concat(modules))
   const modulesByName = keyBy(modulesToWatch, "name")
 
-  await garden.startWatcher(graph)
+  await garden.startWatcher({ graph, skipModules: skipWatchModules })
 
   const waiting = () => {
     if (!!statusLine) {

core/src/watch.ts

@@ -39,19 +39,39 @@ let watcher: FSWatcher | undefined
  * This needs to be enabled by calling the `.start()` method, and stopped with the `.stop()` method.
  */
 export class Watcher extends EventEmitter {
+  private garden: Garden
+  private log: LogEntry
+  private paths: string[]
+  private skipPaths: string[]
+  private modules: GardenModule[]
+  private bufferInterval: number = DEFAULT_BUFFER_INTERVAL
   private watcher?: FSWatcher
   private buffer: { [path: string]: ChangedPath }
   private running: boolean
   public processing: boolean
 
-  constructor(
-    private garden: Garden,
-    private log: LogEntry,
-    private paths: string[],
-    private modules: GardenModule[],
-    private bufferInterval: number = DEFAULT_BUFFER_INTERVAL
-  ) {
+  constructor({
+    garden,
+    log,
+    paths,
+    modules,
+    skipPaths,
+    bufferInterval,
+  }: {
+    garden: Garden
+    log: LogEntry
+    paths: string[]
+    modules: GardenModule[]
+    skipPaths?: string[]
+    bufferInterval?: number
+  }) {
     super()
+    this.garden = garden
+    this.log = log
+    this.paths = paths
+    this.modules = modules
+    this.skipPaths = skipPaths || []
+    this.bufferInterval = bufferInterval || DEFAULT_BUFFER_INTERVAL
     this.buffer = {}
     this.running = false
     this.processing = false
@@ -83,7 +103,7 @@ export class Watcher extends EventEmitter {
       // See https://github.com/garden-io/garden/issues/1269.
       // TODO: see if we can extract paths from dotignore files as well (we'd have to deal with negations etc. somehow).
       const projectExcludes = this.garden.moduleExcludePatterns.map((p) => resolve(this.garden.projectRoot, p))
-      const ignored = [...projectExcludes]
+      const ignored = [...projectExcludes, ...this.skipPaths]
       // TODO: filter paths based on module excludes as well
       //       (requires more complex logic to handle overlapping module sources).
       // const moduleExcludes = flatten(this.modules.map((m) => (m.exclude || []).map((p) => resolve(m.path, p))))
@@ -93,7 +113,7 @@ export class Watcher extends EventEmitter {
         this.log.debug(`Watcher: Using existing FSWatcher`)
         this.watcher = watcher
 
-        this.log.debug(`Watcher: Ignore ${ignored.join(", ")}`)
+        this.log.debug(`Watcher: Ignoring paths ${ignored.join(", ")}`)
         watcher.unwatch(ignored)
 
         this.log.debug(`Watcher: Watch ${this.paths}`)

core/test/unit/src/commands/helpers.ts

@@ -20,17 +20,17 @@ describe("getDevModeServiceNames", () => {
   })
 
   it("should return all services if --dev-mode=* is set", async () => {
-    const result = await getDevModeServiceNames(["*"], graph)
+    const result = getDevModeServiceNames(["*"], graph)
     expect(result).to.eql(graph.getServices().map((s) => s.name))
   })
 
   it("should return all services if --dev-mode is set with no value", async () => {
-    const result = await getDevModeServiceNames([], graph)
+    const result = getDevModeServiceNames([], graph)
     expect(result).to.eql(graph.getServices().map((s) => s.name))
   })
 
   it("should return specific service if --dev-mode is set with a service name", async () => {
-    const result = await getDevModeServiceNames(["service-a"], graph)
+    const result = getDevModeServiceNames(["service-a"], graph)
     expect(result).to.eql(["service-a"])
   })
 })

core/test/unit/src/watch.ts

@@ -43,7 +43,10 @@ describe("Watcher", () => {
     doubleModulePath = resolve(garden.projectRoot, "double-module")
     includeModulePath = resolve(garden.projectRoot, "with-include")
     moduleContext = pathToCacheContext(modulePath)
-    await garden.startWatcher(await garden.getConfigGraph({ log: garden.log, emit: false }), 10)
+    await garden.startWatcher({
+      graph: await garden.getConfigGraph({ log: garden.log, emit: false }),
+      bufferInterval: 10,
+    })
   })
 
   beforeEach(async () => {
@@ -337,7 +340,7 @@ describe("Watcher", () => {
       // This is not an issue in practice because there are specific commands just for linking
       // so the user will always have a new instance of Garden when they run their next command.
       garden = await makeExtModuleSourcesGarden()
-      await garden.startWatcher(await garden.getConfigGraph({ log: garden.log, emit: false }))
+      await garden.startWatcher({ graph: await garden.getConfigGraph({ log: garden.log, emit: false }) })
     })
 
     after(async () => {
@@ -400,7 +403,7 @@ describe("Watcher", () => {
       // This is not an issue in practice because there are specific commands just for linking
       // so the user will always have a new instance of Garden when they run their next command.
       garden = await makeExtProjectSourcesGarden()
-      await garden.startWatcher(await garden.getConfigGraph({ log: garden.log, emit: false }))
+      await garden.startWatcher({ graph: await garden.getConfigGraph({ log: garden.log, emit: false }) })
     })
 
     after(async () => {