From 76a57a9181ae8bba85f60ab2702c08be6244d48b Mon Sep 17 00:00:00 2001 From: Valerio Pipolo Date: Mon, 2 Nov 2020 17:34:27 +0100 Subject: [PATCH 1/5] Use WeakMaps to keep track of webpack and typescript compiler instances --- src/index.ts | 14 ++++++++++---- src/instances.ts | 11 ++++++----- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/index.ts b/src/index.ts index 37af8a480..6b8cc30b7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,7 +27,11 @@ import { isReferencedFile, } from './utils'; -const webpackInstances: webpack.Compiler[] = []; +// WeakMaps are not enumerable, so we need to keep track of how many webpack instances +// were used, this index should be incremented whenever a new instance is used. +let lastWebpackIndex = 0; +// Using a WeakMap prevents compiler instances from being kept in memory forever. +const webpackIndexMap: WeakMap = new WeakMap(); const loaderOptionsCache: LoaderOptionsCache = {}; /** @@ -175,9 +179,11 @@ function getOptionsHash(loaderOptions: LoaderOptions) { */ function getLoaderOptions(loaderContext: webpack.loader.LoaderContext) { // differentiate the TypeScript instance based on the webpack instance - let webpackIndex = webpackInstances.indexOf(loaderContext._compiler); - if (webpackIndex === -1) { - webpackIndex = webpackInstances.push(loaderContext._compiler) - 1; + let webpackIndex = webpackIndexMap.get(loaderContext._compiler); + if (!webpackIndex) { + webpackIndex = lastWebpackIndex; + webpackIndexMap.set(loaderContext._compiler, lastWebpackIndex); + lastWebpackIndex++; } const loaderOptions = diff --git a/src/instances.ts b/src/instances.ts index 0078a5aaa..2a1a02aa8 100644 --- a/src/instances.ts +++ b/src/instances.ts @@ -33,7 +33,7 @@ import { } from './utils'; import { makeWatchRun } from './watch-run'; -const instances = new Map(); +const instances = new WeakMap(); const instancesBySolutionBuilderConfigs = new Map(); /** @@ -47,7 +47,7 @@ export function getTypeScriptInstance( loaderOptions: LoaderOptions, loader: webpack.loader.LoaderContext ): { instance?: TSInstance; error?: WebpackError } { - const existing = instances.get(loaderOptions.instance); + const existing = instances.get(loaderOptions); if (existing) { if (!existing.initialSetupPending) { ensureProgram(existing); @@ -141,7 +141,7 @@ function successfulTypeScriptInstance( const existing = getExistingSolutionBuilderHost(configFileKey); if (existing) { // Reuse the instance if config file for project references is shared. - instances.set(loaderOptions.instance, existing); + instances.set(loaderOptions, existing); return { instance: existing }; } } @@ -226,7 +226,8 @@ function successfulTypeScriptInstance( log, filePathKeyMapper, }; - instances.set(loaderOptions.instance, transpileInstance); + + instances.set(loaderOptions, transpileInstance); return { instance: transpileInstance }; } @@ -278,7 +279,7 @@ function successfulTypeScriptInstance( log, filePathKeyMapper, }; - instances.set(loaderOptions.instance, instance); + instances.set(loaderOptions, instance); return { instance }; } From 7ea0ea12e8fec72a98acedab6a6d5f8aa55fc04a Mon Sep 17 00:00:00 2001 From: Valerio Pipolo Date: Tue, 3 Nov 2020 10:52:15 +0100 Subject: [PATCH 2/5] Cache TS instances by webpack compiler and instance name --- src/instances.ts | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/instances.ts b/src/instances.ts index 2a1a02aa8..95e83c065 100644 --- a/src/instances.ts +++ b/src/instances.ts @@ -33,9 +33,19 @@ import { } from './utils'; import { makeWatchRun } from './watch-run'; -const instances = new WeakMap(); +const compilerMap = new WeakMap>(); const instancesBySolutionBuilderConfigs = new Map(); +function addTSInstanceToCache( + key: webpack.Compiler, + instanceName: string, + instance: TSInstance +) { + const instances = compilerMap.get(key) ?? new Map(); + instances.set(instanceName, instance); + compilerMap.set(key, instances); +} + /** * The loader is executed once for each file seen by webpack. However, we need to keep * a persistent instance of TypeScript that contains all of the files in the program @@ -47,7 +57,13 @@ export function getTypeScriptInstance( loaderOptions: LoaderOptions, loader: webpack.loader.LoaderContext ): { instance?: TSInstance; error?: WebpackError } { - const existing = instances.get(loaderOptions); + let instances = compilerMap.get(loader._compiler); + if (!instances) { + instances = new Map(); + compilerMap.set(loader._compiler, instances); + } + + const existing = instances.get(loaderOptions.instance); if (existing) { if (!existing.initialSetupPending) { ensureProgram(existing); @@ -141,7 +157,7 @@ function successfulTypeScriptInstance( const existing = getExistingSolutionBuilderHost(configFileKey); if (existing) { // Reuse the instance if config file for project references is shared. - instances.set(loaderOptions, existing); + addTSInstanceToCache(loader._compiler, loaderOptions.instance, existing); return { instance: existing }; } } @@ -227,7 +243,11 @@ function successfulTypeScriptInstance( filePathKeyMapper, }; - instances.set(loaderOptions, transpileInstance); + addTSInstanceToCache( + loader._compiler, + loaderOptions.instance, + transpileInstance + ); return { instance: transpileInstance }; } @@ -279,7 +299,8 @@ function successfulTypeScriptInstance( log, filePathKeyMapper, }; - instances.set(loaderOptions, instance); + + addTSInstanceToCache(loader._compiler, loaderOptions.instance, instance); return { instance }; } From cac25b2dac626328dab772b273309d2fdbc7eaef Mon Sep 17 00:00:00 2001 From: Valerio Pipolo Date: Tue, 3 Nov 2020 11:31:43 +0100 Subject: [PATCH 3/5] Remove webpack instance cache --- src/index.ts | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/src/index.ts b/src/index.ts index 6b8cc30b7..95e53e301 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,11 +27,6 @@ import { isReferencedFile, } from './utils'; -// WeakMaps are not enumerable, so we need to keep track of how many webpack instances -// were used, this index should be incremented whenever a new instance is used. -let lastWebpackIndex = 0; -// Using a WeakMap prevents compiler instances from being kept in memory forever. -const webpackIndexMap: WeakMap = new WeakMap(); const loaderOptionsCache: LoaderOptionsCache = {}; /** @@ -178,14 +173,6 @@ function getOptionsHash(loaderOptions: LoaderOptions) { * or creates them, adds them to the cache and returns */ function getLoaderOptions(loaderContext: webpack.loader.LoaderContext) { - // differentiate the TypeScript instance based on the webpack instance - let webpackIndex = webpackIndexMap.get(loaderContext._compiler); - if (!webpackIndex) { - webpackIndex = lastWebpackIndex; - webpackIndexMap.set(loaderContext._compiler, lastWebpackIndex); - lastWebpackIndex++; - } - const loaderOptions = loaderUtils.getOptions(loaderContext) || ({} as LoaderOptions); @@ -193,9 +180,7 @@ function getLoaderOptions(loaderContext: webpack.loader.LoaderContext) { // If no instance name is given in the options, use the hash of the loader options // In this way, if different options are given the instances will be different const instanceName = - webpackIndex + - '_' + - (loaderOptions.instance || 'default_' + getOptionsHash(loaderOptions)); + loaderOptions.instance || 'default_' + getOptionsHash(loaderOptions); if (!loaderOptionsCache.hasOwnProperty(instanceName)) { loaderOptionsCache[instanceName] = new WeakMap(); From c7d8ce78028989d2143ec76e5b3f3d92c4c4f8b3 Mon Sep 17 00:00:00 2001 From: Valerio Pipolo Date: Tue, 3 Nov 2020 11:53:13 +0100 Subject: [PATCH 4/5] Rename compilerMap to instanceCache and add small comment --- src/instances.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/instances.ts b/src/instances.ts index 95e83c065..69008844e 100644 --- a/src/instances.ts +++ b/src/instances.ts @@ -33,7 +33,10 @@ import { } from './utils'; import { makeWatchRun } from './watch-run'; -const compilerMap = new WeakMap>(); +// Each TypeScript instance is based on the webpack instance (key of the WeakMap) +// and also the name that was generated or passed via the options (string key of the +// internal Map) +const instanceCache = new WeakMap>(); const instancesBySolutionBuilderConfigs = new Map(); function addTSInstanceToCache( @@ -41,9 +44,9 @@ function addTSInstanceToCache( instanceName: string, instance: TSInstance ) { - const instances = compilerMap.get(key) ?? new Map(); + const instances = instanceCache.get(key) ?? new Map(); instances.set(instanceName, instance); - compilerMap.set(key, instances); + instanceCache.set(key, instances); } /** @@ -57,10 +60,10 @@ export function getTypeScriptInstance( loaderOptions: LoaderOptions, loader: webpack.loader.LoaderContext ): { instance?: TSInstance; error?: WebpackError } { - let instances = compilerMap.get(loader._compiler); + let instances = instanceCache.get(loader._compiler); if (!instances) { instances = new Map(); - compilerMap.set(loader._compiler, instances); + instanceCache.set(loader._compiler, instances); } const existing = instances.get(loaderOptions.instance); From 7eb34d0f128a08c5a2a8d0231fdc370e49dab1a1 Mon Sep 17 00:00:00 2001 From: Valerio Pipolo Date: Tue, 3 Nov 2020 20:31:01 +0100 Subject: [PATCH 5/5] Add to changelog and bump version --- CHANGELOG.md | 3 +++ package.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5eead78f8..b396a94e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## v8.0.8 +* [Fixed memory leak when using multiple webpack instances](https://github.com/TypeStrong/ts-loader/pull/1205) - thanks @valerio + ## v8.0.7 * [Speeds up project reference build and doesnt store the result in memory](https://github.com/TypeStrong/ts-loader/pull/1202) - thanks @sheetalkamat diff --git a/package.json b/package.json index f9ffda999..e5e48f498 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ts-loader", - "version": "8.0.7", + "version": "8.0.8", "description": "TypeScript loader for webpack", "main": "index.js", "types": "dist",