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

mknichel/dedupe strings in module layers #18340

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 81 additions & 0 deletions lib/ModuleLayerCache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
MIT License http://www.opensource.org/licenses/mit-license.php
Author @mknichel
*/

"use strict";

/**
* A class used to represent a key in a {@link WeakMap}. {@link Symbol} can not
* be used due to https://github.com/nodejs/node/issues/49135.
*/
class WeakMapKey {
/**
* @param {string} key The key to use for caching.
*/
constructor(key) {
this.key = key;
}
}

/**
* A cache for storing information that could be shared by modules across
* different layers. This can be useful for memory management since the same
* file that appears in multiple layers may duplicate large strings, such as
* the contents of the source file. The values in this cache can be garbage
* collected if there are no more retaining references.
*/
class ModuleLayerCache {
constructor() {
/** @type {Map<string, WeakMapKey>} */
this.weakMapKeys = new Map();
/** @type {WeakMap<WeakMapKey, string | Buffer>} */
this.cachedValues = new WeakMap();
mknichel marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Checks to see if there is an entry for the key in this cache. The key is
* typically {@link NormalModule#request}.
*
* @param {string} key The key to use for caching.
* @returns {boolean} Whether there is an entry for the key in this cache.
*/
has(key) {
return (
this.weakMapKeys.has(key) &&
this.cachedValues.has(this.weakMapKeys.get(key))
);
}

/**
* Returns the cached value value for the key in this cache if the value has
* not already been garbage collected.
*
* @param {string} key The key to use for caching.
* @returns {string | Buffer} The cached value for this key.
*/
get(key) {
if (!this.has(key)) {
return null;
}
const weakMapKey = this.weakMapKeys.get(key);
return this.cachedValues.get(weakMapKey);
}

/**
* Stores the value in this cache as a weakly held value (i.e. it can be
* garbage collected).
*
* @param {string} key The key to use for caching.
* @param {string | Buffer} value The value to store in the cache.
*/
set(key, value) {
const weakMapKey = this.weakMapKeys.has(key)
? this.weakMapKeys.get(key)
: new WeakMapKey(key);
this.weakMapKeys.set(key, weakMapKey);
this.cachedValues.set(weakMapKey, value);
}
}

module.exports = ModuleLayerCache;
29 changes: 29 additions & 0 deletions lib/NormalModule.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const Module = require("./Module");
const ModuleBuildError = require("./ModuleBuildError");
const ModuleError = require("./ModuleError");
const ModuleGraphConnection = require("./ModuleGraphConnection");
const ModuleLayerCache = require("./ModuleLayerCache");
const ModuleParseError = require("./ModuleParseError");
const { JAVASCRIPT_MODULE_TYPE_AUTO } = require("./ModuleTypeConstants");
const ModuleWarning = require("./ModuleWarning");
Expand Down Expand Up @@ -231,6 +232,9 @@ makeSerializable(
/** @type {WeakMap<Compilation, NormalModuleCompilationHooks>} */
const compilationHooksMap = new WeakMap();

/** @type {WeakMap<Object, ModuleLayerCache>} */
const cachedModuleLayerCaches = new WeakMap();

class NormalModule extends Module {
/**
* @param {Compilation} compilation the compilation
Expand Down Expand Up @@ -780,6 +784,31 @@ class NormalModule extends Module {
* @returns {Source} the created source
*/
createSource(context, content, sourceMap, associatedObjectForCache) {
// If this module exists in a layer, try to reuse the value for the
// source string so it is not duplicated in multiple modules.
if (this.layer && this.userRequest) {
mknichel marked this conversation as resolved.
Show resolved Hide resolved
let moduleLayerCache = cachedModuleLayerCaches.get(
associatedObjectForCache
);
if (moduleLayerCache) {
moduleLayerCache = new ModuleLayerCache();
cachedModuleLayerCaches.set(associatedObjectForCache, moduleLayerCache);
}
if (moduleLayerCache.has(this.userRequest)) {
mknichel marked this conversation as resolved.
Show resolved Hide resolved
const cachedSource = moduleLayerCache.get(this.userRequest);
mknichel marked this conversation as resolved.
Show resolved Hide resolved
if (
(Buffer.isBuffer(content) &&
Buffer.isBuffer(cachedSource) &&
content.equals(cachedSource)) ||
content === cachedSource
) {
content = cachedSource;
}
} else {
moduleLayerCache.set(this.userRequest, content);
}
}

if (Buffer.isBuffer(content)) {
return new RawSource(content);
}
Expand Down
2 changes: 1 addition & 1 deletion lib/json/JsonModulesPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class JsonModulesPlugin {
.tap(PLUGIN_NAME, parserOptions => {
validate(parserOptions);

return new JsonParser(parserOptions);
return new JsonParser(parserOptions, compilation);
});
normalModuleFactory.hooks.createGenerator
.for(JSON_MODULE_TYPE)
Expand Down
33 changes: 32 additions & 1 deletion lib/json/JsonParser.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

"use strict";

const ModuleLayerCache = require("../ModuleLayerCache");
const Parser = require("../Parser");
const JsonExportsDependency = require("../dependencies/JsonExportsDependency");
const memoize = require("../util/memoize");
Expand All @@ -19,13 +20,18 @@ const JsonData = require("./JsonData");

const getParseJson = memoize(() => require("json-parse-even-better-errors"));

/** @type {WeakMap<Object, ModuleLayerCache>} */
const cachedModuleLayerCaches = new WeakMap();

class JsonParser extends Parser {
/**
* @param {JsonModulesPluginParserOptions} options parser options
* @param {Object} associatedObjectForCache An object to associate cached data with.
*/
constructor(options) {
constructor(options, associatedObjectForCache) {
super();
this.options = options || {};
this.associatedObjectForCache = associatedObjectForCache;
}

/**
Expand Down Expand Up @@ -53,6 +59,31 @@ class JsonParser extends Parser {
} catch (e) {
throw new Error(`Cannot parse JSON: ${/** @type {Error} */ (e).message}`);
}

// If the module is associated with a layer, try to reuse cached data instead
// of duplicating the data multiple times.
const module = state.module;
if (module && module.userRequest && module.layer && Buffer.isBuffer(data)) {
let moduleLayerCache = cachedModuleLayerCaches.get(
this.associatedObjectForCache
);
if (moduleLayerCache) {
moduleLayerCache = new ModuleLayerCache();
cachedModuleLayerCaches.set(
this.associatedObjectForCache,
moduleLayerCache
);
}
if (moduleLayerCache.has(module.userRequest)) {
const cachedData = moduleLayerCache.get(module.userRequest);
if (Buffer.isBuffer(cachedData) && data.equals(cachedData)) {
data = cachedData;
}
} else {
moduleLayerCache.set(module.userRequest, data);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we move it to method, to avoid duplicate logic (for example to the compilation class)

Copy link
Author

Choose a reason for hiding this comment

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

Done. Sorry for the delay. Let me know if this is what you had in mind.


const jsonData = new JsonData(/** @type {Buffer | RawJsonData} */ (data));
const buildInfo = /** @type {BuildInfo} */ (state.module.buildInfo);
buildInfo.jsonData = jsonData;
Expand Down