Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added cache for some FS operations while compile #829

Merged
merged 12 commits into from
Sep 9, 2018
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,15 @@ Extending `tsconfig.json`:

Note that changes in the extending file while not be respected by `ts-loader`. Its purpose is to satisfy the code editor.

### experimentalFileCaching _(boolean) (default=false)_

By default whenever the TypeScript compiler needs to check that a file/directory exists or resolve symlinks it makes syscalls.
It does not cache the result of this operations and this may result in many syscalls with the same arguments ([see comment](https://github.com/TypeStrong/ts-loader/issues/825#issue-354725524) with example).
In some cases it may produce performance degradation.

This flag enables caching for some FS-functions like `fileExists`, `realpath` and `directoryExists` for TypeScript compiler.
Note that caches are cleared between compilations.

### `LoaderOptionsPlugin`

[There's a known "gotcha"](https://github.com/TypeStrong/ts-loader/issues/283) if you are using webpack 2 with the `LoaderOptionsPlugin`. If you are faced with the `Cannot read property 'unsafeCache' of undefined` error then you probably need to supply a `resolve` object as below: (Thanks @jeffijoe!)
Expand Down
6 changes: 4 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ const validLoaderOptions: ValidLoaderOptions[] = [
'getCustomTransformers',
'reportFiles',
'experimentalWatchApi',
'allowTsInNodeModules'
'allowTsInNodeModules',
'experimentalFileCaching'
];

/**
Expand Down Expand Up @@ -210,7 +211,8 @@ function makeLoaderOptions(instanceName: string, loaderOptions: LoaderOptions) {
reportFiles: [],
// When the watch API usage stabilises look to remove this option and make watch usage the default behaviour when available
experimentalWatchApi: false,
allowTsInNodeModules: false
allowTsInNodeModules: false,
experimentalFileCaching: false
} as Partial<LoaderOptions>,
loaderOptions
);
Expand Down
25 changes: 18 additions & 7 deletions src/instances.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,12 @@ function successfulTypeScriptInstance(
colors
});

if (!loader._compiler.hooks) {
throw new Error(
"You may be using an old version of webpack; please check you're using at least version 4"
);
}

if (loaderOptions.experimentalWatchApi && compiler.createWatchProgram) {
log.logInfo('Using watch api');

Expand All @@ -266,17 +272,22 @@ function successfulTypeScriptInstance(
.getProgram()
.getProgram();
} else {
const servicesHost = makeServicesHost(scriptRegex, log, loader, instance);
const servicesHost = makeServicesHost(
scriptRegex,
log,
loader,
instance,
loaderOptions.experimentalFileCaching
);

instance.languageService = compiler.createLanguageService(
servicesHost,
servicesHost.servicesHost,
compiler.createDocumentRegistry()
);
}

if (!loader._compiler.hooks) {
throw new Error(
"You may be using an old version of webpack; please check you're using at least version 4"
);
if (servicesHost.clearCache !== null) {
loader._compiler.hooks.watchRun.tap('ts-loader', servicesHost.clearCache);
}
}

loader._compiler.hooks.afterCompile.tapAsync(
Expand Down
1 change: 1 addition & 0 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ export interface LoaderOptions {
| (() => typescript.CustomTransformers | undefined);
experimentalWatchApi: boolean;
allowTsInNodeModules: boolean;
experimentalFileCaching: boolean;
}

export interface TSFile {
Expand Down
73 changes: 66 additions & 7 deletions src/servicesHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,23 @@ import {
Webpack
} from './interfaces';

export type Action = () => void;

export interface ServiceHostWhichMayBeCacheable {
servicesHost: typescript.LanguageServiceHost;
clearCache: Action | null;
}

/**
* Create the TypeScript language service
*/
export function makeServicesHost(
scriptRegex: RegExp,
log: logger.Logger,
loader: Webpack,
instance: TSInstance
) {
instance: TSInstance,
enableFileCaching: boolean
): ServiceHostWhichMayBeCacheable {
const {
compiler,
compilerOptions,
Expand Down Expand Up @@ -52,9 +60,12 @@ export function makeServicesHost(
const moduleResolutionHost: ModuleResolutionHost = {
fileExists,
readFile: readFileWithFallback,
realpath: compiler.sys.realpath
realpath: compiler.sys.realpath,
directoryExists: compiler.sys.directoryExists
};

const clearCache = enableFileCaching ? addCache(moduleResolutionHost) : null;

// loader.context seems to work fine on Linux / Mac regardless causes problems for @types resolution on Windows for TypeScript < 2.3
const getCurrentDirectory = () => loader.context;

Expand Down Expand Up @@ -97,13 +108,15 @@ export function makeServicesHost(
/**
* For @types expansion, these two functions are needed.
*/
directoryExists: compiler.sys.directoryExists,
directoryExists: moduleResolutionHost.directoryExists,

useCaseSensitiveFileNames: () => compiler.sys.useCaseSensitiveFileNames,

realpath: moduleResolutionHost.realpath,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We didn't have realpath before I think; I'm just curious; what is it used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To resolve path to original one. For example, if you have symlink.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - we've had since we added symlink support here: https://github.com/TypeStrong/ts-loader/pull/774/files

We just didn't supply it to the LanguageServiceHost until now though. I'm kind of surprised we didn't get a compilation error previously 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, interesting. Perhaps microsoft/TypeScript#12020 (comment) (and the whole PR) is related to this.


// The following three methods are necessary for @types resolution from TS 2.4.1 onwards see: https://github.com/Microsoft/TypeScript/issues/16772
fileExists: compiler.sys.fileExists,
readFile: compiler.sys.readFile,
fileExists: moduleResolutionHost.fileExists,
readFile: moduleResolutionHost.readFile,
readDirectory: compiler.sys.readDirectory,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was there a reason that readDirectory wasn't included in the caching functionality?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. I didn't see this function in the profiler before and I don't think that it can reduce perf, but if you want - I can check and provide the stats.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could check that'd be awesome. I'm currently working on tweaking the test packs so they can be made to work against a variety of experimental... flags - the idea being that while the cache functionality lies behind a flag we can still make sure it is tested by the existing test pack. Watch this space...

#834

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you could check that'd be awesome

Unfortunately for now I'm away from work and cannot check it. I'll do it in next couple of days and will provide here results.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnnyreilly I just checked readDirectory - it seems that this function is never called while compilation (at least in our case).


getCurrentDirectory,
Expand Down Expand Up @@ -137,7 +150,7 @@ export function makeServicesHost(
getCustomTransformers: () => instance.transformers
};

return servicesHost;
return { servicesHost, clearCache };
}

/**
Expand Down Expand Up @@ -509,3 +522,49 @@ function populateDependencyGraphs(
] = true;
});
}

function addCache(servicesHost: typescript.ModuleResolutionHost) {
const clearCacheFuncs: Action[] = [];

type CachedProperty = Extract<
keyof typescript.ModuleResolutionHost,
'fileExists' | 'directoryExists' | 'realpath'
>;
const cachedProperties: CachedProperty[] = [
'fileExists',
'directoryExists',
'realpath'
];

cachedProperties.forEach((propertyToCache: CachedProperty) => {
const origFn = servicesHost[propertyToCache];
if (origFn !== undefined) {
const cache = createCache<ReturnType<typeof origFn>>(origFn);
servicesHost[
propertyToCache
] = cache.cached as typescript.ModuleResolutionHost[CachedProperty];
clearCacheFuncs.push(cache.clear);
}
});

return () => clearCacheFuncs.forEach(clear => clear());
}

timocov marked this conversation as resolved.
Show resolved Hide resolved
function createCache<TOut>(func: (arg: string) => TOut) {
const cache = new Map<string, TOut>();
return {
clear: () => {
cache.clear();
},
cached: (arg: string) => {
let res = cache.get(arg);
if (res !== undefined) {
return res;
}

res = func(arg);
cache.set(arg, res);
return res;
}
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
/*! no static exports found */
/***/ (function(module, exports) {

eval("throw new Error(\"Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption/n/nPlease take a look at the options you are supplying; the following are valid options:/nsilent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules/n/n at validateLoaderOptions (/Users/alawson/dev/ts-loader/dist/index.js:110:19)/n at getLoaderOptions (/Users/alawson/dev/ts-loader/dist/index.js:72:5)/n at Object.loader (/Users/alawson/dev/ts-loader/dist/index.js:15:21)\");\n\n//# sourceURL=webpack:///./app.ts?");
eval("throw new Error(\"Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption/n/nPlease take a look at the options you are supplying; the following are valid options:/nsilent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching/n/n at validateLoaderOptions (/Users/alawson/dev/ts-loader/dist/index.js:110:19)/n at getLoaderOptions (/Users/alawson/dev/ts-loader/dist/index.js:72:5)/n at Object.loader (/Users/alawson/dev/ts-loader/dist/index.js:15:21)\");\n\n//# sourceURL=webpack:///./app.ts?");

/***/ })

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ ERROR in ./app.ts
Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption

Please take a look at the options you are supplying; the following are valid options:
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching

at validateLoaderOptions (dist/index.js:110:19)
at getLoaderOptions (dist/index.js:72:5)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
/*! no static exports found */
/***/ (function(module, exports) {

eval("throw new Error(\"Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption/n/nPlease take a look at the options you are supplying; the following are valid options:/nsilent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules/n/n at validateLoaderOptions (C://source//ts-loader//dist//index.js:110:19)/n at getLoaderOptions (C://source//ts-loader//dist//index.js:72:5)/n at Object.loader (C://source//ts-loader//dist//index.js:15:21)\");\n\n//# sourceURL=webpack:///./app.ts?");
eval("throw new Error(\"Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption/n/nPlease take a look at the options you are supplying; the following are valid options:/nsilent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching/n/n at validateLoaderOptions (C://source//ts-loader//dist//index.js:110:19)/n at getLoaderOptions (C://source//ts-loader//dist//index.js:72:5)/n at Object.loader (C://source//ts-loader//dist//index.js:15:21)\");\n\n//# sourceURL=webpack:///./app.ts?");

/***/ })

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ ERROR in ./app.ts
Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption

Please take a look at the options you are supplying; the following are valid options:
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching

at validateLoaderOptions (dist\index.js:110:19)
at getLoaderOptions (dist\index.js:72:5)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@
/*! no static exports found */
/***/ (function(module, exports) {

eval("throw new Error(\"Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption/n/nPlease take a look at the options you are supplying; the following are valid options:/nsilent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules/n/n at validateLoaderOptions (C://source//ts-loader//dist//index.js:110:19)/n at getLoaderOptions (C://source//ts-loader//dist//index.js:72:5)/n at Object.loader (C://source//ts-loader//dist//index.js:15:21)\");\n\n//# sourceURL=webpack:///./app.ts?");
eval("throw new Error(\"Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption/n/nPlease take a look at the options you are supplying; the following are valid options:/nsilent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching/n/n at validateLoaderOptions (C://source//ts-loader//dist//index.js:111:19)/n at getLoaderOptions (C://source//ts-loader//dist//index.js:72:5)/n at Object.loader (C://source//ts-loader//dist//index.js:15:21)\");\n\n//# sourceURL=webpack:///./app.ts?");

/***/ })

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ ERROR in ./app.ts
Module build failed: Error: ts-loader was supplied with an unexpected loader option: notRealOption

Please take a look at the options you are supplying; the following are valid options:
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules
silent / logLevel / logInfoToStdOut / instance / compiler / context / configFile / transpileOnly / ignoreDiagnostics / errorFormatter / colors / compilerOptions / appendTsSuffixTo / appendTsxSuffixTo / onlyCompileBundledFiles / happyPackMode / getCustomTransformers / reportFiles / experimentalWatchApi / allowTsInNodeModules / experimentalFileCaching

at validateLoaderOptions (dist\index.js:110:19)
at validateLoaderOptions (dist\index.js:111:19)
at getLoaderOptions (dist\index.js:72:5)
at Object.loader (dist\index.js:15:21)