From b11247234ddaadbdd63e830f0dc9e1d2469e62ea Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Fri, 10 Sep 2021 10:34:07 +0200 Subject: [PATCH] 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 | 16 +++++++---- core/src/commands/dev.ts | 7 +++-- core/src/commands/helpers.ts | 14 ++++++---- core/src/garden.ts | 32 ++++++++++++++++++++-- core/src/process.ts | 9 ++++-- core/src/watch.ts | 38 ++++++++++++++++++++------ core/test/unit/src/commands/helpers.ts | 6 ++-- core/test/unit/src/watch.ts | 9 ++++-- 8 files changed, 97 insertions(+), 34 deletions(-) diff --git a/core/src/commands/deploy.ts b/core/src/commands/deploy.ts index 2c817da40c..bdc8823c29 100644 --- a/core/src/commands/deploy.ts +++ b/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 { } 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 { 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 { footerLog, modules, initialTasks, + skipWatchModules: getDevModeModules(devModeServiceNames, initGraph), watch, changeHandler: async (graph, module) => { const tasks: BaseTask[] = await getModuleWatchTasks({ diff --git a/core/src/commands/dev.ts b/core/src/commands/dev.ts index 32f9dea2f6..3706269df3 100644 --- a/core/src/commands/dev.ts +++ b/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 { 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 { modules, watch: true, initialTasks, + skipWatchModules: getDevModeModules(devModeServiceNames, graph), changeHandler: async (updatedGraph: ConfigGraph, module: GardenModule) => { return getDevCommandWatchTasks({ garden, diff --git a/core/src/commands/helpers.ts b/core/src/commands/helpers.ts index 2fc54e6c7f..823a6cfa63 100644 --- a/core/src/commands/helpers.ts +++ b/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 { +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) diff --git a/core/src/garden.ts b/core/src/garden.ts index f4a86aecaf..018e025b92 100644 --- a/core/src/garden.ts +++ b/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 { diff --git a/core/src/process.ts b/core/src/process.ts index e43af07731..5c6cbf9846 100644 --- a/core/src/process.ts +++ b/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 { @@ -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) { diff --git a/core/src/watch.ts b/core/src/watch.ts index a8cebcfaea..696b62b987 100644 --- a/core/src/watch.ts +++ b/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}`) diff --git a/core/test/unit/src/commands/helpers.ts b/core/test/unit/src/commands/helpers.ts index 760490aa58..de4646bbfb 100644 --- a/core/test/unit/src/commands/helpers.ts +++ b/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"]) }) }) diff --git a/core/test/unit/src/watch.ts b/core/test/unit/src/watch.ts index bab55bb5fa..94b8f45284 100644 --- a/core/test/unit/src/watch.ts +++ b/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 () => {