-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Hoist RegExp Literals in SourceMapDevToolPlugin #15720
Copy link
Copy link
Closed
Labels
Description
Bug report
What is the current behavior?
The use of RegExp literals create unnecessary object references/allocations especially in iterated hot paths like SourceMapDevToolPlugin. We should hoist these to the top of file and give them useful names so people understand what the heck they do.
If the current behavior is a bug, please provide the steps to reproduce.
Some Examples:
Mapping over a RegExp literal:
webpack/lib/SourceMapDevToolPlugin.js
Line 419 in 3ad4fca
| pattern = contenthash.map(quoteMeta).join("|"); |
Big Async ForEach Statement with multiple RexExp Literal statements:
webpack/lib/SourceMapDevToolPlugin.js
Lines 384 to 551 in 3ad4fca
| asyncLib.each( | |
| tasks, | |
| (task, callback) => { | |
| const assets = Object.create(null); | |
| const assetsInfo = Object.create(null); | |
| const file = task.file; | |
| const chunk = fileToChunk.get(file); | |
| const sourceMap = task.sourceMap; | |
| const source = task.source; | |
| const modules = task.modules; | |
| reportProgress( | |
| 0.5 + (0.5 * taskIndex) / tasks.length, | |
| file, | |
| "attach SourceMap" | |
| ); | |
| const moduleFilenames = modules.map(m => | |
| moduleToSourceNameMapping.get(m) | |
| ); | |
| sourceMap.sources = moduleFilenames; | |
| if (options.noSources) { | |
| sourceMap.sourcesContent = undefined; | |
| } | |
| sourceMap.sourceRoot = options.sourceRoot || ""; | |
| sourceMap.file = file; | |
| const usesContentHash = | |
| sourceMapFilename && | |
| /\[contenthash(:\w+)?\]/.test(sourceMapFilename); | |
| // If SourceMap and asset uses contenthash, avoid a circular dependency by hiding hash in `file` | |
| if (usesContentHash && task.assetInfo.contenthash) { | |
| const contenthash = task.assetInfo.contenthash; | |
| let pattern; | |
| if (Array.isArray(contenthash)) { | |
| pattern = contenthash.map(quoteMeta).join("|"); | |
| } else { | |
| pattern = quoteMeta(contenthash); | |
| } | |
| sourceMap.file = sourceMap.file.replace( | |
| new RegExp(pattern, "g"), | |
| m => "x".repeat(m.length) | |
| ); | |
| } | |
| /** @type {string | false} */ | |
| let currentSourceMappingURLComment = sourceMappingURLComment; | |
| if ( | |
| currentSourceMappingURLComment !== false && | |
| /\.css($|\?)/i.test(file) | |
| ) { | |
| currentSourceMappingURLComment = | |
| currentSourceMappingURLComment.replace( | |
| /^\n\/\/(.*)$/, | |
| "\n/*$1*/" | |
| ); | |
| } | |
| const sourceMapString = JSON.stringify(sourceMap); | |
| if (sourceMapFilename) { | |
| let filename = file; | |
| const sourceMapContentHash = | |
| usesContentHash && | |
| /** @type {string} */ ( | |
| createHash(compilation.outputOptions.hashFunction) | |
| .update(sourceMapString) | |
| .digest("hex") | |
| ); | |
| const pathParams = { | |
| chunk, | |
| filename: options.fileContext | |
| ? relative( | |
| outputFs, | |
| `/${options.fileContext}`, | |
| `/${filename}` | |
| ) | |
| : filename, | |
| contentHash: sourceMapContentHash | |
| }; | |
| const { path: sourceMapFile, info: sourceMapInfo } = | |
| compilation.getPathWithInfo( | |
| sourceMapFilename, | |
| pathParams | |
| ); | |
| const sourceMapUrl = options.publicPath | |
| ? options.publicPath + sourceMapFile | |
| : relative( | |
| outputFs, | |
| dirname(outputFs, `/${file}`), | |
| `/${sourceMapFile}` | |
| ); | |
| /** @type {Source} */ | |
| let asset = new RawSource(source); | |
| if (currentSourceMappingURLComment !== false) { | |
| // Add source map url to compilation asset, if currentSourceMappingURLComment is set | |
| asset = new ConcatSource( | |
| asset, | |
| compilation.getPath( | |
| currentSourceMappingURLComment, | |
| Object.assign({ url: sourceMapUrl }, pathParams) | |
| ) | |
| ); | |
| } | |
| const assetInfo = { | |
| related: { sourceMap: sourceMapFile } | |
| }; | |
| assets[file] = asset; | |
| assetsInfo[file] = assetInfo; | |
| compilation.updateAsset(file, asset, assetInfo); | |
| // Add source map file to compilation assets and chunk files | |
| const sourceMapAsset = new RawSource(sourceMapString); | |
| const sourceMapAssetInfo = { | |
| ...sourceMapInfo, | |
| development: true | |
| }; | |
| assets[sourceMapFile] = sourceMapAsset; | |
| assetsInfo[sourceMapFile] = sourceMapAssetInfo; | |
| compilation.emitAsset( | |
| sourceMapFile, | |
| sourceMapAsset, | |
| sourceMapAssetInfo | |
| ); | |
| if (chunk !== undefined) | |
| chunk.auxiliaryFiles.add(sourceMapFile); | |
| } else { | |
| if (currentSourceMappingURLComment === false) { | |
| throw new Error( | |
| "SourceMapDevToolPlugin: append can't be false when no filename is provided" | |
| ); | |
| } | |
| /** | |
| * Add source map as data url to asset | |
| */ | |
| const asset = new ConcatSource( | |
| new RawSource(source), | |
| currentSourceMappingURLComment | |
| .replace(/\[map\]/g, () => sourceMapString) | |
| .replace( | |
| /\[url\]/g, | |
| () => | |
| `data:application/json;charset=utf-8;base64,${Buffer.from( | |
| sourceMapString, | |
| "utf-8" | |
| ).toString("base64")}` | |
| ) | |
| ); | |
| assets[file] = asset; | |
| assetsInfo[file] = undefined; | |
| compilation.updateAsset(file, asset); | |
| } | |
| task.cacheItem.store({ assets, assetsInfo }, err => { | |
| reportProgress( | |
| 0.5 + (0.5 * ++taskIndex) / tasks.length, | |
| task.file, | |
| "attached SourceMap" | |
| ); | |
| if (err) { | |
| return callback(err); | |
| } | |
| callback(); | |
| }); | |
| }, | |
| err => { | |
| reportProgress(1.0); | |
| callback(err); | |
| } | |
| ); |
What is the expected behavior?
Less memory allocation, less garbage collection triggered.
Happy to PR this. There are quite a few of these examples still lying around webpack that should be cleaned up but I'll start with this.
Reactions are currently unavailable