This repository has been archived by the owner on Dec 5, 2019. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 179
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
perf(sourcemaps): reduce memory usage (#276)
- Loading branch information
1 parent
e94a401
commit 0b11134
Showing
1 changed file
with
27 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,11 +48,18 @@ class UglifyJsPlugin { | |
...uglifyOptions, | ||
}, | ||
}; | ||
this.sourceMapsCache = new WeakMap(); | ||
} | ||
|
||
static buildError(err, file, sourceMap, requestShortener) { | ||
buildError(err, file, inputSourceMap, requestShortener) { | ||
// Handling error which should have line, col, filename and message | ||
if (err.line) { | ||
const sourceMapCacheKey = { file }; | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
alexander-akait
Member
|
||
let sourceMap = this.sourceMapsCache.get(sourceMapCacheKey); | ||
if (!sourceMap) { | ||
sourceMap = new SourceMapConsumer(inputSourceMap); | ||
this.sourceMapsCache.set(sourceMapCacheKey, sourceMap); | ||
} | ||
const original = sourceMap && sourceMap.originalPositionFor({ | ||
line: err.line, | ||
column: err.col, | ||
|
@@ -67,11 +74,20 @@ class UglifyJsPlugin { | |
return new Error(`${file} from UglifyJs\n${err.message}`); | ||
} | ||
|
||
static buildWarning(warning, file, sourceMap, warningsFilter, requestShortener) { | ||
if (!file || !sourceMap) { | ||
buildWarning(warning, file, inputSourceMap, warningsFilter, requestShortener) { | ||
if (!file || !inputSourceMap) { | ||
return warning; | ||
} | ||
|
||
const sourceMapCacheKey = { file }; | ||
|
||
let sourceMap = this.sourceMapsCache.get(sourceMapCacheKey); | ||
|
||
if (!sourceMap) { | ||
sourceMap = new SourceMapConsumer(inputSourceMap); | ||
this.sourceMapsCache.set(sourceMapCacheKey, sourceMap); | ||
} | ||
|
||
const match = warningRegex.exec(warning); | ||
const line = +match[1]; | ||
const column = +match[2]; | ||
|
@@ -114,15 +130,14 @@ class UglifyJsPlugin { | |
.concat(compilation.additionalChunkAssets || []) | ||
.filter(ModuleFilenameHelpers.matchObject.bind(null, this.options)) | ||
.forEach((file) => { | ||
let sourceMap; | ||
let inputSourceMap; | ||
const asset = compilation.assets[file]; | ||
if (uglifiedAssets.has(asset)) { | ||
return; | ||
} | ||
|
||
try { | ||
let input; | ||
let inputSourceMap; | ||
|
||
if (this.options.sourceMap && asset.sourceAndMap) { | ||
const { source, map } = asset.sourceAndMap(); | ||
|
@@ -131,11 +146,8 @@ class UglifyJsPlugin { | |
|
||
if (utils.isSourceMap(map)) { | ||
inputSourceMap = map; | ||
sourceMap = new SourceMapConsumer(inputSourceMap); | ||
} else { | ||
inputSourceMap = map; | ||
sourceMap = null; | ||
|
||
compilation.warnings.push( | ||
new Error(`${file} contain invalid source map`), | ||
); | ||
|
@@ -157,7 +169,6 @@ class UglifyJsPlugin { | |
const task = { | ||
file, | ||
input, | ||
sourceMap, | ||
inputSourceMap, | ||
commentsFile, | ||
extractComments: this.options.extractComments, | ||
|
@@ -177,10 +188,10 @@ class UglifyJsPlugin { | |
tasks.push(task); | ||
} catch (error) { | ||
compilation.errors.push( | ||
UglifyJsPlugin.buildError( | ||
this.buildError( | ||
error, | ||
file, | ||
sourceMap, | ||
inputSourceMap, | ||
requestShortener, | ||
), | ||
); | ||
|
@@ -194,17 +205,17 @@ class UglifyJsPlugin { | |
} | ||
|
||
results.forEach((data, index) => { | ||
const { file, input, sourceMap, inputSourceMap, commentsFile } = tasks[index]; | ||
const { file, input, inputSourceMap, commentsFile } = tasks[index]; | ||
const { error, map, code, warnings, extractedComments } = data; | ||
|
||
// Handling results | ||
// Error case: add errors, and go to next file | ||
if (error) { | ||
compilation.errors.push( | ||
UglifyJsPlugin.buildError( | ||
this.buildError( | ||
error, | ||
file, | ||
sourceMap, | ||
inputSourceMap, | ||
requestShortener, | ||
), | ||
); | ||
|
@@ -264,10 +275,10 @@ class UglifyJsPlugin { | |
// Handling warnings | ||
if (warnings && warnings.length > 0) { | ||
warnings.forEach((warning) => { | ||
const builtWarning = UglifyJsPlugin.buildWarning( | ||
const builtWarning = this.buildWarning( | ||
warning, | ||
file, | ||
sourceMap, | ||
inputSourceMap, | ||
this.options.warningsFilter, | ||
requestShortener, | ||
); | ||
|
While trying to track down why the 1.2.5 release made our build 4x longer, we came across this change which appears to be the culprit.
This doesn't seem to be working as it was intended, since it creates a new
sourceMapCacheKey
object every time which results in a cache miss on every call. If I change this to a normalMap
keyed by thefile
string, our build time goes back to normal.I'm happy to submit a PR, but since this is unfamiliar territory I'd like some guidance/confirmation first: is it important that this remain a
WeakMap
, which would require a way to mapfile
to unique key objects, or is switching to aMap
keyed byfile
string sufficient? Thanks in advance.