Skip to content

Commit

Permalink
improvement(watcher): adding/removing many files/dirs more performant (
Browse files Browse the repository at this point in the history
…#1087)

Previously, we weren't handling large numbers of added/removed files well. We'd trigger a lot of version checks and updates. We now buffer additions of files and directories, and more optimally and consistently handle those changes.
  • Loading branch information
edvald authored Aug 12, 2019
1 parent 7336554 commit b1d0f9a
Show file tree
Hide file tree
Showing 12 changed files with 478 additions and 214 deletions.
2 changes: 1 addition & 1 deletion garden-service/src/bin/add-version-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ async function addVersionFiles() {
const versionFilePath = resolve(path, GARDEN_VERSIONFILE_NAME)

const vcsHandler = new GitHandler(garden.gardenDirPath, garden.dotIgnoreFiles)
const treeVersion = await vcsHandler.getTreeVersion(config)
const treeVersion = await vcsHandler.getTreeVersion(garden.log, config)

console.log(`${config.name} -> ${relative(STATIC_DIR, versionFilePath)}`)

Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/commands/get/get-debug-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export async function collectBasicDebugInfo(root: string, gardenDirPath: string,
const vcs = new GitHandler(root, config.dotIgnoreFiles || defaultDotIgnoreFiles)
const include = config.modules && config.modules.include
const exclude = config.modules && config.modules.exclude
const paths = await findConfigPathsInPath(vcs, root, { include, exclude })
const paths = await findConfigPathsInPath({ vcs, dir: root, include, exclude, log })

// Copy all the service configuration files
for (const configPath of paths) {
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export interface Events {
},
moduleSourcesChanged: {
names: string[],
pathChanged: string,
pathsChanged: string[],
},
moduleRemoved: {
},
Expand Down
16 changes: 10 additions & 6 deletions garden-service/src/garden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,11 @@ 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) {
async startWatcher(graph: ConfigGraph, bufferInterval?: number) {
const modules = await 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)
this.watcher = new Watcher(this, this.log, paths, modules, bufferInterval)
}

private registerPlugin(name: string, moduleOrFactory: RegisterPluginParam) {
Expand Down Expand Up @@ -692,7 +692,7 @@ export class Garden {
const dependencies = await this.getRawModuleConfigs(dependencyKeys)
const cacheContexts = dependencies.concat([config]).map(c => getModuleCacheContext(c))

const version = await this.vcs.resolveVersion(config, dependencies)
const version = await this.vcs.resolveVersion(this.log, config, dependencies)

this.cache.set(cacheKey, version, ...cacheContexts)
return version
Expand All @@ -702,9 +702,13 @@ export class Garden {
* Scans the specified directories for Garden config files and returns a list of paths.
*/
async scanForConfigs(path: string) {
return findConfigPathsInPath(
this.vcs, path, { include: this.moduleIncludePatterns, exclude: this.moduleExcludePatterns },
)
return findConfigPathsInPath({
vcs: this.vcs,
dir: path,
include: this.moduleIncludePatterns,
exclude: this.moduleExcludePatterns,
log: this.log,
})
}

/*
Expand Down
22 changes: 19 additions & 3 deletions garden-service/src/util/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { join, basename, win32, posix } from "path"
import { ValidationError } from "../exceptions"
import { platform } from "os"
import { VcsHandler } from "../vcs/vcs"
import { LogEntry } from "../logger/log-entry"

const VALID_CONFIG_FILENAMES = ["garden.yml", "garden.yaml"]
const metadataFilename = "metadata.json"
Expand Down Expand Up @@ -111,11 +112,11 @@ export async function getChildDirNames(parentDir: string): Promise<string[]> {
* @param {string} dir The directory to scan
*/
export async function findConfigPathsInPath(
vcs: VcsHandler, dir: string,
{ include, exclude }: { include?: string[], exclude?: string[] } = {},
{ vcs, dir, include, exclude, log }:
{ vcs: VcsHandler, dir: string, include?: string[], exclude?: string[], log: LogEntry },
) {
// TODO: we could make this lighter/faster using streaming
const files = await vcs.getFiles(dir, include, [...exclude || [], ...fixedExcludes])
const files = await vcs.getFiles({ path: dir, include, exclude: [...exclude || [], ...fixedExcludes], log })
return files
.map(f => f.path)
.filter(f => isConfigFilename(basename(f)))
Expand Down Expand Up @@ -145,6 +146,21 @@ export function matchGlobs(path: string, patterns: string[]): boolean {
return some(patterns, pattern => minimatch(path, pattern))
}

/**
* Check if a path passes through given include/exclude filters.
*
* @param path A POSIX-style path
* @param include List of globs to match for inclusion, or undefined
* @param exclude List of globs to match for exclusion, or undefined
*/
export function matchPath(path: string, include?: string[], exclude?: string[]) {
return (
(!include || matchGlobs(path, include))
&&
(!exclude || !matchGlobs(path, exclude))
)
}

/**
* Gets an ID for the current working copy, given the path to the project's `.garden` directory.
* We do this by storing a `metadata` file in the directory with an ID. The file is created on demand and a new
Expand Down
25 changes: 14 additions & 11 deletions garden-service/src/vcs/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import { ensureDir, pathExists, stat, createReadStream } from "fs-extra"
import { PassThrough } from "stream"
import * as hasha from "hasha"

import { VcsHandler, RemoteSourceParams, VcsFile } from "./vcs"
import { VcsHandler, RemoteSourceParams, VcsFile, GetFilesParams } from "./vcs"
import { ConfigurationError, RuntimeError } from "../exceptions"
import * as Bluebird from "bluebird"
import { matchGlobs } from "../util/fs"
import { matchPath } from "../util/fs"
import { deline } from "../util/string"
import { splitLast } from "../util/util"
import { LogEntry } from "../logger/log-entry"

export function getCommitIdFromRefList(refList: string[]): string {
try {
Expand Down Expand Up @@ -50,8 +51,9 @@ interface GitCli {
export class GitHandler extends VcsHandler {
name = "git"

private gitCli(cwd: string): GitCli {
private gitCli(log: LogEntry, cwd: string): GitCli {
return async (...args: string[]) => {
log.silly(`Calling git with args '${args.join(" ")}`)
const output = await execa.stdout("git", args, { cwd })
return output.split("\n").filter(line => line.length > 0)
}
Expand All @@ -70,8 +72,8 @@ export class GitHandler extends VcsHandler {
}
}

async getFiles(path: string, include?: string[], exclude?: string[]): Promise<VcsFile[]> {
const git = this.gitCli(path)
async getFiles({ log, path, include, exclude }: GetFilesParams): Promise<VcsFile[]> {
const git = this.gitCli(log, path)

let lines: string[] = []
let ignored: string[] = []
Expand Down Expand Up @@ -118,8 +120,7 @@ export class GitHandler extends VcsHandler {
const modified = new Set(modifiedArr)

const filtered = files
.filter(f => !include || matchGlobs(f.path, include))
.filter(f => !exclude || !matchGlobs(f.path, exclude))
.filter(f => matchPath(f.path, include, exclude))
.filter(f => !ignored.includes(f.path))

return Bluebird.map(filtered, async (f) => {
Expand Down Expand Up @@ -147,8 +148,10 @@ export class GitHandler extends VcsHandler {
}).filter(f => f.hash !== "")
}

private async cloneRemoteSource(remoteSourcesPath: string, repositoryUrl: string, hash: string, absPath: string) {
const git = this.gitCli(remoteSourcesPath)
private async cloneRemoteSource(
log: LogEntry, remoteSourcesPath: string, repositoryUrl: string, hash: string, absPath: string,
) {
const git = this.gitCli(log, remoteSourcesPath)
return git("clone", "--depth=1", `--branch=${hash}`, repositoryUrl, absPath)
}

Expand All @@ -165,7 +168,7 @@ export class GitHandler extends VcsHandler {
const { repositoryUrl, hash } = parseGitUrl(url)

try {
await this.cloneRemoteSource(remoteSourcesPath, repositoryUrl, hash, absPath)
await this.cloneRemoteSource(log, remoteSourcesPath, repositoryUrl, hash, absPath)
} catch (err) {
entry.setError()
throw new RuntimeError(`Downloading remote ${sourceType} failed with error: \n\n${err}`, {
Expand All @@ -182,7 +185,7 @@ export class GitHandler extends VcsHandler {

async updateRemoteSource({ url, name, sourceType, log }: RemoteSourceParams) {
const absPath = join(this.gardenDirPath, this.getRemoteSourceRelPath(name, url, sourceType))
const git = this.gitCli(absPath)
const git = this.gitCli(log, absPath)
const { repositoryUrl, hash } = parseGitUrl(url)

await this.ensureRemoteSource({ url, name, sourceType, log })
Expand Down
36 changes: 26 additions & 10 deletions garden-service/src/vcs/vcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { pathExists, readFile, writeFile } from "fs-extra"
import { ConfigurationError } from "../exceptions"
import { ExternalSourceType, getRemoteSourcesDirname, getRemoteSourceRelPath } from "../util/ext-source-util"
import { ModuleConfig, serializeConfig } from "../config/module"
import { LogNode } from "../logger/log-node"
import { LogEntry } from "../logger/log-entry"

export const NEW_MODULE_VERSION = "0000000000"

Expand Down Expand Up @@ -65,11 +65,18 @@ export const moduleVersionSchema = joi.object()
files: fileNamesSchema,
})

export interface GetFilesParams {
log: LogEntry,
path: string,
include?: string[],
exclude?: string[],
}

export interface RemoteSourceParams {
url: string,
name: string,
sourceType: ExternalSourceType,
log: LogNode,
log: LogEntry,
}

export interface VcsFile {
Expand All @@ -81,14 +88,21 @@ export abstract class VcsHandler {
constructor(protected gardenDirPath: string, protected ignoreFiles: string[]) { }

abstract name: string
abstract async getFiles(path: string, include?: string[], exclude?: string[]): Promise<VcsFile[]>
abstract async getFiles(params: GetFilesParams): Promise<VcsFile[]>
abstract async ensureRemoteSource(params: RemoteSourceParams): Promise<string>
abstract async updateRemoteSource(params: RemoteSourceParams): Promise<void>

async getTreeVersion(moduleConfig: ModuleConfig): Promise<TreeVersion> {
async getTreeVersion(log: LogEntry, moduleConfig: ModuleConfig): Promise<TreeVersion> {
const configPath = moduleConfig.configPath

const files = sortBy(await this.getFiles(moduleConfig.path, moduleConfig.include, moduleConfig.exclude), "path")
let files = await this.getFiles({
log,
path: moduleConfig.path,
include: moduleConfig.include,
exclude: moduleConfig.exclude,
})

files = sortBy(files, "path")
// Don't include the config file in the file list
.filter(f => !configPath || f.path !== configPath)

Expand All @@ -97,15 +111,17 @@ export abstract class VcsHandler {
return { contentHash, files: files.map(f => f.path) }
}

async resolveTreeVersion(moduleConfig: ModuleConfig): Promise<TreeVersion> {
async resolveTreeVersion(log: LogEntry, moduleConfig: ModuleConfig): Promise<TreeVersion> {
// the version file is used internally to specify versions outside of source control
const versionFilePath = join(moduleConfig.path, GARDEN_TREEVERSION_FILENAME)
const fileVersion = await readTreeVersionFile(versionFilePath)
return fileVersion || this.getTreeVersion(moduleConfig)
return fileVersion || this.getTreeVersion(log, moduleConfig)
}

async resolveVersion(moduleConfig: ModuleConfig, dependencies: ModuleConfig[]): Promise<ModuleVersion> {
const treeVersion = await this.resolveTreeVersion(moduleConfig)
async resolveVersion(
log: LogEntry, moduleConfig: ModuleConfig, dependencies: ModuleConfig[],
): Promise<ModuleVersion> {
const treeVersion = await this.resolveTreeVersion(log, moduleConfig)

validate(treeVersion, treeVersionSchema, {
context: `${this.name} tree version for module at ${moduleConfig.path}`,
Expand All @@ -125,7 +141,7 @@ export abstract class VcsHandler {

const namedDependencyVersions = await Bluebird.map(
dependencies,
async (m: ModuleConfig) => ({ name: m.name, ...await this.resolveTreeVersion(m) }),
async (m: ModuleConfig) => ({ name: m.name, ...await this.resolveTreeVersion(log, m) }),
)
const dependencyVersions = mapValues(keyBy(namedDependencyVersions, "name"), v => omit(v, "name"))

Expand Down
Loading

0 comments on commit b1d0f9a

Please sign in to comment.