From 1594fc477d2ccd0070203fd699a12ffd74adeae9 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 11:31:05 -0500 Subject: [PATCH 1/5] Set low CPU priority on watcher Fixes #247. --- packages/server/src/vscode/bootstrapFork.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/server/src/vscode/bootstrapFork.ts b/packages/server/src/vscode/bootstrapFork.ts index c7cb5811a878..6334149ebe1b 100644 --- a/packages/server/src/vscode/bootstrapFork.ts +++ b/packages/server/src/vscode/bootstrapFork.ts @@ -1,7 +1,9 @@ import * as cp from "child_process"; import * as fs from "fs"; +import * as os from "os"; import * as path from "path"; import * as vm from "vm"; +import { logger } from "@coder/logger"; import { buildDir, isCli } from "../constants"; let ipcMsgBuffer: Buffer[] | undefined = []; @@ -151,6 +153,13 @@ export const forkModule = (modulePath: string, args?: string[], options?: cp.For } else { proc = cp.spawn(process.execPath, ["--require", "ts-node/register", "--require", "tsconfig-paths/register", process.argv[1], ...forkArgs], forkOptions); } + if (args && args[0] === "--type=watcherService" && os.platform() === "linux") { + cp.exec(`renice -n 19 -p ${proc.pid}`, (error) => { + if (error) { + logger.warn(error.message); + } + }); + } return proc; }; From a929d9c7edf1268e09abb99685cf648b19a4eab6 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 12:31:48 -0500 Subject: [PATCH 2/5] Batch stat and readdir calls --- packages/protocol/src/browser/modules/fs.ts | 34 +++++++++-- packages/protocol/src/common/proxy.ts | 63 +++++++++++++++++++++ packages/protocol/src/node/modules/fs.ts | 8 +++ 3 files changed, 101 insertions(+), 4 deletions(-) diff --git a/packages/protocol/src/browser/modules/fs.ts b/packages/protocol/src/browser/modules/fs.ts index c8b793af6d59..75826434ce8a 100644 --- a/packages/protocol/src/browser/modules/fs.ts +++ b/packages/protocol/src/browser/modules/fs.ts @@ -1,12 +1,32 @@ import * as fs from "fs"; import { callbackify } from "util"; -import { ClientProxy } from "../../common/proxy"; +import { ClientProxy, Batch } from "../../common/proxy"; import { IEncodingOptions, IEncodingOptionsCallback } from "../../common/util"; import { FsModuleProxy, Stats as IStats, WatcherProxy, WriteStreamProxy } from "../../node/modules/fs"; import { Writable } from "./stream"; // tslint:disable no-any +class StatBatch extends Batch { + public constructor(private readonly proxy: FsModuleProxy) { + super(); + } + + protected remoteCall(batch: { path: fs.PathLike }[]): Promise<(IStats | Error)[]> { + return this.proxy.statBatch(batch); + } +} + +class ReaddirBatch extends Batch { + public constructor(private readonly proxy: FsModuleProxy) { + super(); + } + + protected remoteCall(queue: { path: fs.PathLike, options: IEncodingOptions }[]): Promise<(Buffer[] | fs.Dirent[] | string[] | Error)[]> { + return this.proxy.readdirBatch(queue); + } +} + class Watcher extends ClientProxy implements fs.FSWatcher { public close(): void { this.proxy.close(); @@ -28,7 +48,13 @@ class WriteStream extends Writable implements fs.WriteStream { } export class FsModule { - public constructor(private readonly proxy: FsModuleProxy) {} + private readonly statBatch: StatBatch; + private readonly readdirBatch: ReaddirBatch; + + public constructor(private readonly proxy: FsModuleProxy) { + this.statBatch = new StatBatch(this.proxy); + this.readdirBatch = new ReaddirBatch(this.proxy); + } public access = (path: fs.PathLike, mode: number | undefined | ((err: NodeJS.ErrnoException) => void), callback?: (err: NodeJS.ErrnoException) => void): void => { if (typeof mode === "function") { @@ -175,7 +201,7 @@ export class FsModule { callback = options; options = undefined; } - callbackify(this.proxy.readdir)(path, options, callback!); + callbackify(this.readdirBatch.add)({ path, options }, callback!); } public readlink = (path: fs.PathLike, options: IEncodingOptionsCallback, callback?: (err: NodeJS.ErrnoException, linkString: string | Buffer) => void): void => { @@ -203,7 +229,7 @@ export class FsModule { } public stat = (path: fs.PathLike, callback: (err: NodeJS.ErrnoException, stats: fs.Stats) => void): void => { - callbackify(this.proxy.stat)(path, (error, stats) => { + callbackify(this.statBatch.add)({ path }, (error, stats) => { callback(error, stats && new Stats(stats)); }); } diff --git a/packages/protocol/src/common/proxy.ts b/packages/protocol/src/common/proxy.ts index dd0feda2224e..0dabaa5f8a57 100644 --- a/packages/protocol/src/common/proxy.ts +++ b/packages/protocol/src/common/proxy.ts @@ -81,3 +81,66 @@ export enum Module { NodePty = "node-pty", Trash = "trash", } + +interface BatchItem { + args: A; + resolve: (t: T) => void; + reject: (e: Error) => void; +} + +/** + * Batch remote calls. + */ +export abstract class Batch { + /** + * Flush after reaching this count. + */ + private maxCount = 100; + + /** + * Flush after not receiving more requests for this amount of time. + */ + private maxTime = 100; + + private flushTimeout: number | NodeJS.Timer | undefined; + + private batch = []>[]; + + public add = (args: A): Promise => { + return new Promise((resolve, reject) => { + this.batch.push({ + args, + resolve, + reject, + }); + if (this.batch.length >= this.maxCount) { + this.flush(); + } else { + clearTimeout(this.flushTimeout as any); + this.flushTimeout = setTimeout(this.flush, this.maxTime); + } + }); + } + + protected abstract remoteCall(batch: A[]): Promise<(T | Error)[]>; + + private flush = (): void => { + clearTimeout(this.flushTimeout as any); + + const batch = this.batch; + this.batch = []; + + this.remoteCall(batch.map((q) => q.args)).then((results) => { + batch.forEach((item, i) => { + const result = results[i]; + if (!result) { + item.reject(new Error("no response")); + } else if (result instanceof Error) { + item.reject(result); + } else { + item.resolve(result); + } + }); + }).catch((error) => batch.forEach((item) => item.reject(error))); + } +} diff --git a/packages/protocol/src/node/modules/fs.ts b/packages/protocol/src/node/modules/fs.ts index c2e164a2ba83..62047b6a403e 100644 --- a/packages/protocol/src/node/modules/fs.ts +++ b/packages/protocol/src/node/modules/fs.ts @@ -182,6 +182,10 @@ export class FsModuleProxy { return promisify(fs.readdir)(path, options); } + public readdirBatch(args: { path: fs.PathLike, options: IEncodingOptions }[]): Promise<(Buffer[] | fs.Dirent[] | string[] | Error)[]> { + return Promise.all(args.map((args) => this.readdir(args.path, args.options).catch((e) => e))); + } + public readlink(path: fs.PathLike, options: IEncodingOptions): Promise { return promisify(fs.readlink)(path, options); } @@ -202,6 +206,10 @@ export class FsModuleProxy { return this.makeStatsSerializable(await promisify(fs.stat)(path)); } + public async statBatch(args: { path: fs.PathLike }[]): Promise<(Stats | Error)[]> { + return Promise.all(args.map((args) => this.stat(args.path).catch((e) => e))); + } + public symlink(target: fs.PathLike, path: fs.PathLike, type?: fs.symlink.Type | null): Promise { return promisify(fs.symlink)(target, path, type); } From 6274f176792b3ea513574fa6e9c6d3d6f29bd3d0 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 13:01:29 -0500 Subject: [PATCH 3/5] Fix fs.exists callbackify seems to always adds an error as the first argument. Opted to just use the promise for this one. --- packages/protocol/src/browser/modules/fs.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/protocol/src/browser/modules/fs.ts b/packages/protocol/src/browser/modules/fs.ts index 75826434ce8a..ef7cf4e6e5da 100644 --- a/packages/protocol/src/browser/modules/fs.ts +++ b/packages/protocol/src/browser/modules/fs.ts @@ -98,9 +98,7 @@ export class FsModule { } public exists = (path: fs.PathLike, callback: (exists: boolean) => void): void => { - callbackify(this.proxy.exists)(path, (exists) => { - callback!(exists as any); - }); + this.proxy.exists(path).then((exists) => callback(exists)).catch(() => callback(false)); } public fchmod = (fd: number, mode: string | number, callback: (err: NodeJS.ErrnoException) => void): void => { From b630b77303a647e1ffda6868972a1aa2c8545f13 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 13:50:13 -0500 Subject: [PATCH 4/5] Batch lstat --- packages/protocol/src/browser/modules/fs.ts | 14 +++++++++++++- packages/protocol/src/node/modules/fs.ts | 4 ++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/protocol/src/browser/modules/fs.ts b/packages/protocol/src/browser/modules/fs.ts index ef7cf4e6e5da..bb23c6b06f5c 100644 --- a/packages/protocol/src/browser/modules/fs.ts +++ b/packages/protocol/src/browser/modules/fs.ts @@ -17,6 +17,16 @@ class StatBatch extends Batch { } } +class LstatBatch extends Batch { + public constructor(private readonly proxy: FsModuleProxy) { + super(); + } + + protected remoteCall(batch: { path: fs.PathLike }[]): Promise<(IStats | Error)[]> { + return this.proxy.lstatBatch(batch); + } +} + class ReaddirBatch extends Batch { public constructor(private readonly proxy: FsModuleProxy) { super(); @@ -49,10 +59,12 @@ class WriteStream extends Writable implements fs.WriteStream { export class FsModule { private readonly statBatch: StatBatch; + private readonly lstatBatch: LstatBatch; private readonly readdirBatch: ReaddirBatch; public constructor(private readonly proxy: FsModuleProxy) { this.statBatch = new StatBatch(this.proxy); + this.lstatBatch = new LstatBatch(this.proxy); this.readdirBatch = new ReaddirBatch(this.proxy); } @@ -148,7 +160,7 @@ export class FsModule { } public lstat = (path: fs.PathLike, callback: (err: NodeJS.ErrnoException, stats: fs.Stats) => void): void => { - callbackify(this.proxy.lstat)(path, (error, stats) => { + callbackify(this.lstatBatch.add)({ path }, (error, stats) => { callback(error, stats && new Stats(stats)); }); } diff --git a/packages/protocol/src/node/modules/fs.ts b/packages/protocol/src/node/modules/fs.ts index 62047b6a403e..184ca25bcc89 100644 --- a/packages/protocol/src/node/modules/fs.ts +++ b/packages/protocol/src/node/modules/fs.ts @@ -156,6 +156,10 @@ export class FsModuleProxy { return this.makeStatsSerializable(await promisify(fs.lstat)(path)); } + public async lstatBatch(args: { path: fs.PathLike }[]): Promise<(Stats | Error)[]> { + return Promise.all(args.map((args) => this.lstat(args.path).catch((e) => e))); + } + public mkdir(path: fs.PathLike, mode: number | string | fs.MakeDirectoryOptions | undefined | null): Promise { return promisify(fs.mkdir)(path, mode); } From c5b2debbe48a21f77d002f68fa86cf4f5b10cf65 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 27 Mar 2019 14:11:03 -0500 Subject: [PATCH 5/5] Add maximum time for flushing batches --- packages/protocol/src/common/proxy.ts | 44 ++++++++++++++---------- packages/protocol/src/node/modules/fs.ts | 6 ++-- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/packages/protocol/src/common/proxy.ts b/packages/protocol/src/common/proxy.ts index 0dabaa5f8a57..6558ea2a41f8 100644 --- a/packages/protocol/src/common/proxy.ts +++ b/packages/protocol/src/common/proxy.ts @@ -92,20 +92,25 @@ interface BatchItem { * Batch remote calls. */ export abstract class Batch { - /** - * Flush after reaching this count. - */ - private maxCount = 100; - - /** - * Flush after not receiving more requests for this amount of time. - */ - private maxTime = 100; - - private flushTimeout: number | NodeJS.Timer | undefined; - + private idleTimeout: number | NodeJS.Timer | undefined; + private maxTimeout: number | NodeJS.Timer | undefined; private batch = []>[]; + public constructor( + /** + * Flush after reaching this amount of time. + */ + private readonly maxTime = 1000, + /** + * Flush after reaching this count. + */ + private readonly maxCount = 100, + /** + * Flush after not receiving more requests for this amount of time. + */ + private readonly idleTime = 100, + ) {} + public add = (args: A): Promise => { return new Promise((resolve, reject) => { this.batch.push({ @@ -116,8 +121,11 @@ export abstract class Batch { if (this.batch.length >= this.maxCount) { this.flush(); } else { - clearTimeout(this.flushTimeout as any); - this.flushTimeout = setTimeout(this.flush, this.maxTime); + clearTimeout(this.idleTimeout as any); + this.idleTimeout = setTimeout(this.flush, this.idleTime); + if (typeof this.maxTimeout === "undefined") { + this.maxTimeout = setTimeout(this.flush, this.maxTime); + } } }); } @@ -125,7 +133,9 @@ export abstract class Batch { protected abstract remoteCall(batch: A[]): Promise<(T | Error)[]>; private flush = (): void => { - clearTimeout(this.flushTimeout as any); + clearTimeout(this.idleTimeout as any); + clearTimeout(this.maxTimeout as any); + this.maxTimeout = undefined; const batch = this.batch; this.batch = []; @@ -133,9 +143,7 @@ export abstract class Batch { this.remoteCall(batch.map((q) => q.args)).then((results) => { batch.forEach((item, i) => { const result = results[i]; - if (!result) { - item.reject(new Error("no response")); - } else if (result instanceof Error) { + if (result && result instanceof Error) { item.reject(result); } else { item.resolve(result); diff --git a/packages/protocol/src/node/modules/fs.ts b/packages/protocol/src/node/modules/fs.ts index 184ca25bcc89..cc6177b25e8a 100644 --- a/packages/protocol/src/node/modules/fs.ts +++ b/packages/protocol/src/node/modules/fs.ts @@ -157,7 +157,7 @@ export class FsModuleProxy { } public async lstatBatch(args: { path: fs.PathLike }[]): Promise<(Stats | Error)[]> { - return Promise.all(args.map((args) => this.lstat(args.path).catch((e) => e))); + return Promise.all(args.map((a) => this.lstat(a.path).catch((e) => e))); } public mkdir(path: fs.PathLike, mode: number | string | fs.MakeDirectoryOptions | undefined | null): Promise { @@ -187,7 +187,7 @@ export class FsModuleProxy { } public readdirBatch(args: { path: fs.PathLike, options: IEncodingOptions }[]): Promise<(Buffer[] | fs.Dirent[] | string[] | Error)[]> { - return Promise.all(args.map((args) => this.readdir(args.path, args.options).catch((e) => e))); + return Promise.all(args.map((a) => this.readdir(a.path, a.options).catch((e) => e))); } public readlink(path: fs.PathLike, options: IEncodingOptions): Promise { @@ -211,7 +211,7 @@ export class FsModuleProxy { } public async statBatch(args: { path: fs.PathLike }[]): Promise<(Stats | Error)[]> { - return Promise.all(args.map((args) => this.stat(args.path).catch((e) => e))); + return Promise.all(args.map((a) => this.stat(a.path).catch((e) => e))); } public symlink(target: fs.PathLike, path: fs.PathLike, type?: fs.symlink.Type | null): Promise {