From 4ed80b62df9531db3863854c702809eb3b0e7bb7 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 16 Oct 2019 11:24:24 -0700 Subject: [PATCH 1/7] Stop pre-emptively creating directories Checking for directory existence is expensive and frequently indicates success. Instead of pre-emptively creating the directory containing a file to be written, attempt to create the file and only do the directory scaffolding if the write fails. Appears to reduce file write time by 10-20% for a file-I/O heavy partner build. Thanks to @rbuckton for the suggestion! --- src/compiler/program.ts | 22 +++++++++++++++++----- src/compiler/sys.ts | 17 +++++++++++++---- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 0c8444daa2..b9495c787c 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -151,13 +151,16 @@ namespace ts { function writeFile(fileName: string, data: string, writeByteOrderMark: boolean, onError?: (message: string) => void) { try { performance.mark("beforeIOWrite"); - ensureDirectoriesExist(getDirectoryPath(normalizePath(fileName))); - if (isWatchSet(options) && system.createHash && system.getModifiedTime) { - writeFileIfUpdated(fileName, data, writeByteOrderMark); + // PERF: Checking for directory existence is expensive. + // Instead, assume the directory exists and fall back + // to creating it if the file write fails. + try { + writeFileWorker(fileName, data, writeByteOrderMark); } - else { - system.writeFile(fileName, data, writeByteOrderMark); + catch (_) { + ensureDirectoriesExist(getDirectoryPath(normalizePath(fileName))); + writeFileWorker(fileName, data, writeByteOrderMark); } performance.mark("afterIOWrite"); @@ -170,6 +173,15 @@ namespace ts { } } + function writeFileWorker(fileName: string, data: string, writeByteOrderMark: boolean) { + if (isWatchSet(options) && system.createHash && system.getModifiedTime) { + writeFileIfUpdated(fileName, data, writeByteOrderMark); + } + else { + system.writeFile(fileName, data, writeByteOrderMark); + } + } + function getDefaultLibLocation(): string { return getDirectoryPath(normalizePath(system.getExecutingFilePath())); } diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts index ad866cba13..c85bb48bb9 100644 --- a/src/compiler/sys.ts +++ b/src/compiler/sys.ts @@ -541,11 +541,20 @@ namespace ts { // patch writefile to create folder before writing the file const originalWriteFile = sys.writeFile; sys.writeFile = (path, data, writeBom) => { - const directoryPath = getDirectoryPath(normalizeSlashes(path)); - if (directoryPath && !sys.directoryExists(directoryPath)) { - recursiveCreateDirectory(directoryPath, sys); + // PERF: Checking for directory existence is expensive. + // Instead, assume the directory exists and fall back + // to creating it if the file write fails. + try { + originalWriteFile.call(sys, path, data, writeBom); + } + catch (_) { + const directoryPath = getDirectoryPath(normalizeSlashes(path)); + if (directoryPath && !sys.directoryExists(directoryPath)) { + recursiveCreateDirectory(directoryPath, sys); + } + + originalWriteFile.call(sys, path, data, writeBom); } - originalWriteFile.call(sys, path, data, writeBom); }; } From b6659e5d6eed1a2daf5cca5a87febb5cfceb5372 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Wed, 16 Oct 2019 11:31:44 -0700 Subject: [PATCH 2/7] Inline function to tidy up control flow --- src/compiler/program.ts | 79 +++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index b9495c787c..44cf374ea7 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -96,7 +96,7 @@ namespace ts { if (existingDirectories.has(directoryPath)) { return true; } - if (system.directoryExists(directoryPath)) { + if ((compilerHost.directoryExists || system.directoryExists)(directoryPath)) { existingDirectories.set(directoryPath, true); return true; } @@ -107,47 +107,10 @@ namespace ts { if (directoryPath.length > getRootLength(directoryPath) && !directoryExists(directoryPath)) { const parentDirectory = getDirectoryPath(directoryPath); ensureDirectoriesExist(parentDirectory); - if (compilerHost.createDirectory) { - compilerHost.createDirectory(directoryPath); - } - else { - system.createDirectory(directoryPath); - } + (compilerHost.createDirectory || system.createDirectory)(directoryPath); } } - let outputFingerprints: Map; - - function writeFileIfUpdated(fileName: string, data: string, writeByteOrderMark: boolean): void { - if (!outputFingerprints) { - outputFingerprints = createMap(); - } - - const hash = system.createHash!(data); // TODO: GH#18217 - const mtimeBefore = system.getModifiedTime!(fileName); // TODO: GH#18217 - - if (mtimeBefore) { - const fingerprint = outputFingerprints.get(fileName); - // If output has not been changed, and the file has no external modification - if (fingerprint && - fingerprint.byteOrderMark === writeByteOrderMark && - fingerprint.hash === hash && - fingerprint.mtime.getTime() === mtimeBefore.getTime()) { - return; - } - } - - system.writeFile(fileName, data, writeByteOrderMark); - - const mtimeAfter = system.getModifiedTime!(fileName) || missingFileModifiedTime; // TODO: GH#18217 - - outputFingerprints.set(fileName, { - hash, - byteOrderMark: writeByteOrderMark, - mtime: mtimeAfter - }); - } - function writeFile(fileName: string, data: string, writeByteOrderMark: boolean, onError?: (message: string) => void) { try { performance.mark("beforeIOWrite"); @@ -155,6 +118,9 @@ namespace ts { // PERF: Checking for directory existence is expensive. // Instead, assume the directory exists and fall back // to creating it if the file write fails. + // NOTE: If patchWriteFileEnsuringDirectory has been called, + // the file write will do its own directory creation and + // the ensureDirectoriesExist call will always be redundant. try { writeFileWorker(fileName, data, writeByteOrderMark); } @@ -173,13 +139,40 @@ namespace ts { } } + let outputFingerprints: Map; function writeFileWorker(fileName: string, data: string, writeByteOrderMark: boolean) { - if (isWatchSet(options) && system.createHash && system.getModifiedTime) { - writeFileIfUpdated(fileName, data, writeByteOrderMark); - } - else { + if (!isWatchSet(options) || !system.createHash || !system.getModifiedTime) { system.writeFile(fileName, data, writeByteOrderMark); + return; } + + if (!outputFingerprints) { + outputFingerprints = createMap(); + } + + const hash = system.createHash(data); + const mtimeBefore = system.getModifiedTime(fileName); + + if (mtimeBefore) { + const fingerprint = outputFingerprints.get(fileName); + // If output has not been changed, and the file has no external modification + if (fingerprint && + fingerprint.byteOrderMark === writeByteOrderMark && + fingerprint.hash === hash && + fingerprint.mtime.getTime() === mtimeBefore.getTime()) { + return; + } + } + + system.writeFile(fileName, data, writeByteOrderMark); + + const mtimeAfter = system.getModifiedTime(fileName) || missingFileModifiedTime; + + outputFingerprints.set(fileName, { + hash, + byteOrderMark: writeByteOrderMark, + mtime: mtimeAfter + }); } function getDefaultLibLocation(): string { From f39b49d756110e24ac75c81413cb13e8a0a423b0 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Thu, 17 Oct 2019 11:36:45 -0700 Subject: [PATCH 3/7] Update another writeFile call-site --- src/compiler/program.ts | 2 +- src/compiler/sys.ts | 2 +- src/compiler/watch.ts | 15 +++++++++++++-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 44cf374ea7..6a697b9c18 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -124,7 +124,7 @@ namespace ts { try { writeFileWorker(fileName, data, writeByteOrderMark); } - catch (_) { + catch { ensureDirectoriesExist(getDirectoryPath(normalizePath(fileName))); writeFileWorker(fileName, data, writeByteOrderMark); } diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts index c85bb48bb9..c4a5012b26 100644 --- a/src/compiler/sys.ts +++ b/src/compiler/sys.ts @@ -547,7 +547,7 @@ namespace ts { try { originalWriteFile.call(sys, path, data, writeBom); } - catch (_) { + catch { const directoryPath = getDirectoryPath(normalizeSlashes(path)); if (directoryPath && !sys.directoryExists(directoryPath)) { recursiveCreateDirectory(directoryPath, sys); diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index 06871fc9ca..f6e334aa47 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -307,9 +307,20 @@ namespace ts { function writeFile(fileName: string, text: string, writeByteOrderMark: boolean, onError: (message: string) => void) { try { performance.mark("beforeIOWrite"); - ensureDirectoriesExist(getDirectoryPath(normalizePath(fileName))); - host.writeFile!(fileName, text, writeByteOrderMark); + // PERF: Checking for directory existence is expensive. + // Instead, assume the directory exists and fall back + // to creating it if the file write fails. + // NOTE: If patchWriteFileEnsuringDirectory has been called, + // the file write will do its own directory creation and + // the ensureDirectoriesExist call will always be redundant. + try { + host.writeFile!(fileName, text, writeByteOrderMark); + } + catch { + ensureDirectoriesExist(getDirectoryPath(normalizePath(fileName))); + host.writeFile!(fileName, text, writeByteOrderMark); + } performance.mark("afterIOWrite"); performance.measure("I/O Write", "beforeIOWrite", "afterIOWrite"); From 205b3dae3bb86b7b78864f585364f73b0830d7e7 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Thu, 17 Oct 2019 16:26:43 -0700 Subject: [PATCH 4/7] Extract shared helper --- src/compiler/program.ts | 27 ++++++++------------------- src/compiler/sys.ts | 35 ++++++++--------------------------- src/compiler/utilities.ts | 30 ++++++++++++++++++++++++++++++ src/compiler/watch.ts | 21 ++------------------- 4 files changed, 48 insertions(+), 65 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 6a697b9c18..0be96b7d55 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -103,31 +103,20 @@ namespace ts { return false; } - function ensureDirectoriesExist(directoryPath: string) { - if (directoryPath.length > getRootLength(directoryPath) && !directoryExists(directoryPath)) { - const parentDirectory = getDirectoryPath(directoryPath); - ensureDirectoriesExist(parentDirectory); - (compilerHost.createDirectory || system.createDirectory)(directoryPath); - } - } - function writeFile(fileName: string, data: string, writeByteOrderMark: boolean, onError?: (message: string) => void) { try { performance.mark("beforeIOWrite"); - // PERF: Checking for directory existence is expensive. - // Instead, assume the directory exists and fall back - // to creating it if the file write fails. // NOTE: If patchWriteFileEnsuringDirectory has been called, - // the file write will do its own directory creation and + // the system.writeFile will do its own directory creation and // the ensureDirectoriesExist call will always be redundant. - try { - writeFileWorker(fileName, data, writeByteOrderMark); - } - catch { - ensureDirectoriesExist(getDirectoryPath(normalizePath(fileName))); - writeFileWorker(fileName, data, writeByteOrderMark); - } + writeFileEnsuringDirectories( + fileName, + data, + writeByteOrderMark, + writeFileWorker, + compilerHost.createDirectory || system.createDirectory, + directoryExists); performance.mark("afterIOWrite"); performance.measure("I/O Write", "beforeIOWrite", "afterIOWrite"); diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts index c4a5012b26..9de6193d73 100644 --- a/src/compiler/sys.ts +++ b/src/compiler/sys.ts @@ -522,17 +522,6 @@ namespace ts { } } - function recursiveCreateDirectory(directoryPath: string, sys: System) { - const basePath = getDirectoryPath(directoryPath); - const shouldCreateParent = basePath !== "" && directoryPath !== basePath && !sys.directoryExists(basePath); - if (shouldCreateParent) { - recursiveCreateDirectory(basePath, sys); - } - if (shouldCreateParent || !sys.directoryExists(directoryPath)) { - sys.createDirectory(directoryPath); - } - } - /** * patch writefile to create folder before writing the file */ @@ -540,22 +529,14 @@ namespace ts { export function patchWriteFileEnsuringDirectory(sys: System) { // patch writefile to create folder before writing the file const originalWriteFile = sys.writeFile; - sys.writeFile = (path, data, writeBom) => { - // PERF: Checking for directory existence is expensive. - // Instead, assume the directory exists and fall back - // to creating it if the file write fails. - try { - originalWriteFile.call(sys, path, data, writeBom); - } - catch { - const directoryPath = getDirectoryPath(normalizeSlashes(path)); - if (directoryPath && !sys.directoryExists(directoryPath)) { - recursiveCreateDirectory(directoryPath, sys); - } - - originalWriteFile.call(sys, path, data, writeBom); - } - }; + sys.writeFile = (path, data, writeBom) => + writeFileEnsuringDirectories( + path, + data, + writeBom, + (p, d, w) => originalWriteFile.call(sys, p, d, w), + sys.createDirectory, + sys.directoryExists); } /*@internal*/ diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 2fb1173dee..8f7602cbe2 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3700,6 +3700,36 @@ namespace ts { }, sourceFiles); } + function ensureDirectoriesExist( + directoryPath: string, + createDirectory: (path: string) => void, + directoryExists: (path: string) => boolean): void { + if (directoryPath.length > getRootLength(directoryPath) && !directoryExists(directoryPath)) { + const parentDirectory = getDirectoryPath(directoryPath); + ensureDirectoriesExist(parentDirectory, createDirectory, directoryExists); + createDirectory(directoryPath); + } + } + + export function writeFileEnsuringDirectories( + path: string, + data: string, + writeByteOrderMark: boolean | undefined, + writeFile: (path: string, data: string, writeByteOrderMark?: boolean) => void, + createDirectory: (path: string) => void, + directoryExists: (path: string) => boolean): void { + + // PERF: Checking for directory existence is expensive. Instead, assume the directory exists + // and fall back to creating it if the file write fails. + try { + writeFile(path, data, writeByteOrderMark); + } + catch { + ensureDirectoriesExist(getDirectoryPath(normalizePath(path)), createDirectory, directoryExists); + writeFile(path, data, writeByteOrderMark); + } + } + export function getLineOfLocalPosition(currentSourceFile: SourceFile, pos: number) { return getLineAndCharacterOfPosition(currentSourceFile, pos).line; } diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index f6e334aa47..cc6bd0bcce 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -296,31 +296,14 @@ namespace ts { readDirectory: maybeBind(host, host.readDirectory), }; - function ensureDirectoriesExist(directoryPath: string) { - if (directoryPath.length > getRootLength(directoryPath) && !host.directoryExists!(directoryPath)) { - const parentDirectory = getDirectoryPath(directoryPath); - ensureDirectoriesExist(parentDirectory); - if (host.createDirectory) host.createDirectory(directoryPath); - } - } - function writeFile(fileName: string, text: string, writeByteOrderMark: boolean, onError: (message: string) => void) { try { performance.mark("beforeIOWrite"); - // PERF: Checking for directory existence is expensive. - // Instead, assume the directory exists and fall back - // to creating it if the file write fails. // NOTE: If patchWriteFileEnsuringDirectory has been called, - // the file write will do its own directory creation and + // the host.writeFile will do its own directory creation and // the ensureDirectoriesExist call will always be redundant. - try { - host.writeFile!(fileName, text, writeByteOrderMark); - } - catch { - ensureDirectoriesExist(getDirectoryPath(normalizePath(fileName))); - host.writeFile!(fileName, text, writeByteOrderMark); - } + writeFileEnsuringDirectories(fileName, text, writeByteOrderMark, host.writeFile!, host.createDirectory!, host.directoryExists!); performance.mark("afterIOWrite"); performance.measure("I/O Write", "beforeIOWrite", "afterIOWrite"); From 6429e4cd36f136ac11cda325ecdfdd8ea4d0dc9e Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Mon, 21 Oct 2019 13:32:42 -0700 Subject: [PATCH 5/7] Fix undefined `this` --- src/compiler/sys.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts index 9de6193d73..772eb148fc 100644 --- a/src/compiler/sys.ts +++ b/src/compiler/sys.ts @@ -535,8 +535,8 @@ namespace ts { data, writeBom, (p, d, w) => originalWriteFile.call(sys, p, d, w), - sys.createDirectory, - sys.directoryExists); + p => sys.createDirectory.call(sys, p), + p => sys.directoryExists.call(sys, p)); } /*@internal*/ From ca31f008a83801d60d564fd4f6194ac1aa352302 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Mon, 21 Oct 2019 14:01:12 -0700 Subject: [PATCH 6/7] Address more potential `this` issues --- src/compiler/program.ts | 6 +++--- src/compiler/sys.ts | 6 +++--- src/compiler/utilities.ts | 4 ++-- src/compiler/watch.ts | 8 +++++++- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 0be96b7d55..0b22d78f35 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -114,9 +114,9 @@ namespace ts { fileName, data, writeByteOrderMark, - writeFileWorker, - compilerHost.createDirectory || system.createDirectory, - directoryExists); + (f, d, w) => writeFileWorker(f, d, w), + p => (compilerHost.createDirectory || system.createDirectory)(p), + p => directoryExists(p)); performance.mark("afterIOWrite"); performance.measure("I/O Write", "beforeIOWrite", "afterIOWrite"); diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts index 772eb148fc..933cea7b72 100644 --- a/src/compiler/sys.ts +++ b/src/compiler/sys.ts @@ -533,10 +533,10 @@ namespace ts { writeFileEnsuringDirectories( path, data, - writeBom, + !!writeBom, (p, d, w) => originalWriteFile.call(sys, p, d, w), - p => sys.createDirectory.call(sys, p), - p => sys.directoryExists.call(sys, p)); + p => sys.createDirectory(p), + p => sys.directoryExists(p)); } /*@internal*/ diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index 8f7602cbe2..ac29da9389 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3714,8 +3714,8 @@ namespace ts { export function writeFileEnsuringDirectories( path: string, data: string, - writeByteOrderMark: boolean | undefined, - writeFile: (path: string, data: string, writeByteOrderMark?: boolean) => void, + writeByteOrderMark: boolean, + writeFile: (path: string, data: string, writeByteOrderMark: boolean) => void, createDirectory: (path: string) => void, directoryExists: (path: string) => boolean): void { diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index cc6bd0bcce..eef4a03c07 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -303,7 +303,13 @@ namespace ts { // NOTE: If patchWriteFileEnsuringDirectory has been called, // the host.writeFile will do its own directory creation and // the ensureDirectoriesExist call will always be redundant. - writeFileEnsuringDirectories(fileName, text, writeByteOrderMark, host.writeFile!, host.createDirectory!, host.directoryExists!); + writeFileEnsuringDirectories( + fileName, + text, + writeByteOrderMark, + (f, w, d) => host.writeFile!(f, w, d), + p => host.createDirectory!(p), + p => host.directoryExists!(p)); performance.mark("afterIOWrite"); performance.measure("I/O Write", "beforeIOWrite", "afterIOWrite"); From af2f46e8996476053e3e6d0c64d2f9d972191564 Mon Sep 17 00:00:00 2001 From: Andrew Casey Date: Mon, 21 Oct 2019 16:22:10 -0700 Subject: [PATCH 7/7] Use longer lambda parameter names --- src/compiler/program.ts | 6 +++--- src/compiler/sys.ts | 6 +++--- src/compiler/watch.ts | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/compiler/program.ts b/src/compiler/program.ts index 0b22d78f35..59b57bfa99 100644 --- a/src/compiler/program.ts +++ b/src/compiler/program.ts @@ -114,9 +114,9 @@ namespace ts { fileName, data, writeByteOrderMark, - (f, d, w) => writeFileWorker(f, d, w), - p => (compilerHost.createDirectory || system.createDirectory)(p), - p => directoryExists(p)); + (path, data, writeByteOrderMark) => writeFileWorker(path, data, writeByteOrderMark), + path => (compilerHost.createDirectory || system.createDirectory)(path), + path => directoryExists(path)); performance.mark("afterIOWrite"); performance.measure("I/O Write", "beforeIOWrite", "afterIOWrite"); diff --git a/src/compiler/sys.ts b/src/compiler/sys.ts index 933cea7b72..31b422a12b 100644 --- a/src/compiler/sys.ts +++ b/src/compiler/sys.ts @@ -534,9 +534,9 @@ namespace ts { path, data, !!writeBom, - (p, d, w) => originalWriteFile.call(sys, p, d, w), - p => sys.createDirectory(p), - p => sys.directoryExists(p)); + (path, data, writeByteOrderMark) => originalWriteFile.call(sys, path, data, writeByteOrderMark), + path => sys.createDirectory(path), + path => sys.directoryExists(path)); } /*@internal*/ diff --git a/src/compiler/watch.ts b/src/compiler/watch.ts index eef4a03c07..5e8991dbb6 100644 --- a/src/compiler/watch.ts +++ b/src/compiler/watch.ts @@ -307,9 +307,9 @@ namespace ts { fileName, text, writeByteOrderMark, - (f, w, d) => host.writeFile!(f, w, d), - p => host.createDirectory!(p), - p => host.directoryExists!(p)); + (path, data, writeByteOrderMark) => host.writeFile!(path, data, writeByteOrderMark), + path => host.createDirectory!(path), + path => host.directoryExists!(path)); performance.mark("afterIOWrite"); performance.measure("I/O Write", "beforeIOWrite", "afterIOWrite");