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

[Memory] Add options to reduce memory consumed by RawSource #155

Merged
merged 6 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 31 additions & 13 deletions lib/RawSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
"use strict";

const streamChunksOfRawSource = require("./helpers/streamChunksOfRawSource");
const {
internString,
isDualStringBufferCachingEnabled
} = require("./helpers/stringBufferUtils");
const Source = require("./Source");

class RawSource extends Source {
Expand All @@ -16,9 +20,14 @@ class RawSource extends Source {
throw new TypeError("argument 'value' must be either string or Buffer");
}
this._valueIsBuffer = !convertToString && isBuffer;
this._value = convertToString && isBuffer ? undefined : value;
this._value =
convertToString && isBuffer
? undefined
: typeof value === "string"
? internString(value)
: value;
this._valueAsBuffer = isBuffer ? value : undefined;
this._valueAsString = isBuffer ? undefined : value;
this._valueAsString = isBuffer ? undefined : internString(value);
mknichel marked this conversation as resolved.
Show resolved Hide resolved
}

isBuffer() {
Expand All @@ -27,14 +36,22 @@ class RawSource extends Source {

source() {
if (this._value === undefined) {
this._value = this._valueAsBuffer.toString("utf-8");
const value = internString(this._valueAsBuffer.toString("utf-8"));
if (isDualStringBufferCachingEnabled()) {
this._value = value;
mknichel marked this conversation as resolved.
Show resolved Hide resolved
}
return value;
}
return this._value;
}

buffer() {
if (this._valueAsBuffer === undefined) {
this._valueAsBuffer = Buffer.from(this._value, "utf-8");
const value = Buffer.from(this._value, "utf-8");
if (isDualStringBufferCachingEnabled()) {
this._valueAsBuffer = value;
}
return value;
}
return this._valueAsBuffer;
}
Expand All @@ -51,17 +68,21 @@ class RawSource extends Source {
* @returns {void}
*/
streamChunks(options, onChunk, onSource, onName) {
if (this._value === undefined) {
if (this._value === undefined && isDualStringBufferCachingEnabled()) {
this._value = Buffer.from(this._valueAsBuffer, "utf-8");
}
if (this._valueAsString === undefined) {
this._valueAsString =
let strValue = this._valueAsString;
if (strValue === undefined) {
strValue =
typeof this._value === "string"
? this._value
: this._value.toString("utf-8");
: internString(this._value.toString("utf-8"));
if (isDualStringBufferCachingEnabled()) {
this._valueAsString = strValue;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This looks like we want to call const strValue = this.source()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logic is a bit strange since source() can return a Buffer (when convertToString = false) but it looks a bit cleaner now.

return streamChunksOfRawSource(
this._valueAsString,
strValue,
onChunk,
onSource,
onName,
Expand All @@ -70,11 +91,8 @@ class RawSource extends Source {
}

updateHash(hash) {
if (this._valueAsBuffer === undefined) {
this._valueAsBuffer = Buffer.from(this._value, "utf-8");
}
hash.update("RawSource");
hash.update(this._valueAsBuffer);
hash.update(this.buffer());
}
}

Expand Down
107 changes: 107 additions & 0 deletions lib/helpers/stringBufferUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
MIT License http://www.opensource.org/licenses/mit-license.php
Author Mark Knichel @mknichel
*/

"use strict";

let dualStringBufferCaching = true;

/**
* @returns {boolean} Whether the optimization to cache copies of both the
* string and buffer version of source content is enabled. This is enabled by
* default to improve performance but can consume more memory since values are
* stored twice.
*/
function isDualStringBufferCachingEnabled() {
return dualStringBufferCaching;
}

/**
* Enables an optimization to save both string and buffer in memory to avoid
* repeat conversions between the two formats when they are requested. This
* is enabled by default. This option can improve performance but can consume
* additional memory since values are stored twice.
*
* @returns {void}
*/
function enableDualStringBufferCaching() {
dualStringBufferCaching = true;
}

/**
* Disables the optimization to save both string and buffer in memory. This
* may increase performance but should reduce memory usage in the Webpack
* compiler.
*
* @returns {void}
*/
function disableDualStringBufferCaching() {
dualStringBufferCaching = false;
}

const interningStringMap = new Map();

/**
* Saves the string in a map to ensure that only one copy of the string exists
* in memory at a given time. This is controlled by {@link enableStringInterning}
* and {@link disableStringInterning}. Callers are expect to manage the memory
* of the interned strings by calling {@link disableStringInterning} after the
* compiler no longer needs to save the interned memory.
*
* @param {string} str A string to be interned.
* @returns {string} The original string or a reference to an existing string
* of the same value if it has already been interned.
*/
function internString(str) {
if (!isStringInterningEnabled() || !str || typeof str !== "string") {
mknichel marked this conversation as resolved.
Show resolved Hide resolved
return str;
}
let internedString = interningStringMap.get(str);
if (internedString === undefined) {
internedString = str;
interningStringMap.set(str, internedString);
}
return internedString;
}

let enableStringInterningRefCount = 0;

function isStringInterningEnabled() {
return enableStringInterningRefCount > 0;
}

/**
* Enables a memory optimization to avoid repeat copies of the same string in
* memory by caching a single reference to the string. This can reduce memory
* usage if the same string is repeated many times in the compiler, such as
* when Webpack layers are used with the same files.
*
* @returns {void}
*/
function enableStringInterning() {
enableStringInterningRefCount++;
}

/**
* Disables string interning. This should be called to free the memory used by
* the interned strings after the compiler no longer needs to reuse the
* interned strings such as at the end of the compilation.
*
* @returns {void}
*/
function disableStringInterning() {
Copy link
Member

Choose a reason for hiding this comment

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

Disable sounds too much like it would stop interning unconditionally and doesn't mention the ref counting.
Maybe call it start/endStringInterningRange or something else

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Switched to enter/exitStringInterningRange

if (--enableStringInterningRefCount <= 0) {
interningStringMap.clear();
enableStringInterningRefCount = 0;
}
}

module.exports = {
disableDualStringBufferCaching,
disableStringInterning,
enableDualStringBufferCaching,
enableStringInterning,
internString,
isDualStringBufferCachingEnabled
};
1 change: 1 addition & 0 deletions lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ defineExport("ReplaceSource", () => require("./ReplaceSource"));
defineExport("PrefixSource", () => require("./PrefixSource"));
defineExport("SizeOnlySource", () => require("./SizeOnlySource"));
defineExport("CompatSource", () => require("./CompatSource"));
defineExport("stringBufferUtils", () => require("./helpers/stringBufferUtils"));
66 changes: 66 additions & 0 deletions test/RawSource.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
const RawSource = require("../").RawSource;
const {
enableDualStringBufferCaching,
enableStringInterning,
disableDualStringBufferCaching,
disableStringInterning
} = require("../lib/helpers/stringBufferUtils");

const CODE_STRING =
"console.log('test');\nconsole.log('test2');\nconsole.log('test22');\n";

describe("RawSource", () => {
it("converts to buffer correctly", () => {
const source = new RawSource(Buffer.from(CODE_STRING), true);
expect(source.isBuffer()).toEqual(false);
expect(source.buffer().toString("utf-8")).toEqual(CODE_STRING);
// The buffer conversion should be cached.
expect(source.buffer() === source.buffer()).toEqual(true);
});

it("stream chunks works correctly", () => {
const source = new RawSource(CODE_STRING, true);
source.streamChunks(null, (line, lineNum) => {
expect(line).toEqual(`console.log('test${"2".repeat(lineNum - 1)}');\n`);
});
expect.assertions(3);
});

describe("performance optimizations are disabled", () => {
beforeEach(() => {
disableDualStringBufferCaching();
disableStringInterning();
});

afterEach(() => {
enableDualStringBufferCaching();
enableStringInterning();
});

it("should create new buffers when caching is not enabled", () => {
const source = new RawSource(CODE_STRING, true);
expect(source.buffer().toString("utf-8")).toEqual(CODE_STRING);
// The buffer conversion should not be cached.
expect(source.buffer() === source.buffer()).toEqual(false);
});

it("should not create new buffers when original value is a buffer", () => {
const originalValue = Buffer.from(CODE_STRING);
const source = new RawSource(originalValue, true);
expect(source.buffer().toString("utf-8")).toEqual(CODE_STRING);
// The same buffer as the original value should always be returned.
expect(originalValue === source.buffer()).toEqual(true);
expect(source.buffer() === source.buffer()).toEqual(true);
});

it("stream chunks works correctly", () => {
const source = new RawSource(CODE_STRING, true);
source.streamChunks(null, (line, lineNum) => {
expect(line).toEqual(
`console.log('test${"2".repeat(lineNum - 1)}');\n`
);
});
expect.assertions(3);
});
});
});