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

mknichel · 2024-05-20T23:18:20Z

This PR adds two options to reduce memory used by RawSource: disable dual caching of string and buffer content and intern strings across source objects. These issues were observed while investigating memory usage of Next.js applications where Webpack had duplicate copies of strings/buffers in memory.

These options are optional so that they should not affect existing users but can be enabled for users who wish to reduce memory usage at the potential of a slight compilation time increase.

Disable dual caching of string and buffer contents

Upon creation, RawSource stores either a string or buffer. When requested, it will create the other type and then store it in memory for any future calls:

		if (this._valueAsBuffer === undefined) {
			this._valueAsBuffer = Buffer.from(this._value, "utf-8");
		}
		return this._valueAsBuffer;

This will cause the same content to be stored twice in memory, roughly doubling the memory consumption for RawSource objects (a buffer will be slightly bigger than the string, but not by too much).

This will save any future conversion that has to be done. However, these conversions aren't too expensive CPU wise (Buffer.from runs at 34000-36000 ops/sec on an Apple M1 Pro on a 150kb file), so it makes sense for large applications or memory constrained applications to disable this with only slight increases in compile times.

Intern strings across Raw Sources

Files that are repeated in multiple Webpack layers will be loaded into multiple RawSource objects and repeat the file contents in memory. Next.js App Router uses Webpack layers and therefore we are seeing file contents in next build appear duplicate times.

An option to intern strings is added in this pull request. With string interning, the same reference will be used across multiple RawSources so the memory isn't duplicated. v8 doesn't support string interning for dynamically allocated strings, but this can be implemented in JavaScript with a Map. The caller is responsible for deleting the contents of the string intern cache when it is no longer needed (such as at the end of a compilation).

Impact

I tested next build for 3 different Next.js applications with these changes: nodejs.org, and two private Next.js applications of medium and large sizes.

Nodejs.org would save ~20 MB of heap by enabling these optimizations out of ~315MB total used by the compilation and increase compile times by < 1 second
Internal medium sized Next.js application would save ~270MB of memory with these optimizations out of ~2.1GB heap total and would increase compile times by ~1-2 seconds.
Internal large Next.js application would save ~330MB of memory with these optimizations and would increase compile times by ~10 seconds.

linux-foundation-easycla · 2024-05-20T23:18:24Z

The committers listed above are authorized under a signed CLA.

✅ login: mknichel (766b87a, 9c7704d, dc41538, 3d6ab16, 1b29b35, a7a085f)

alexander-akait

I see, good catch, maybe we should review out codebase and try to only strings when it is possible, it can decrease build time for all too (feel free to do it), also can you accept CLA?

sokra · 2024-05-22T17:51:30Z

lib/helpers/stringBufferUtils.js

+ *
+ * @returns {void}
+ */
+function disableStringInterning() {


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

Done. Switched to enter/exitStringInterningRange

…s in webpack-sources (#66003) This PR adds a flag to Next.js to enable Webpack options to improve memory usage. See webpack/webpack-sources#155 for a full description of the changes and impact on memory. This PR adds a patch to `webpack-sources` temporarily that contains the fixes as the real changes are iterated on to merge upstream in the `webpack/webpack-sources` repository. After that is done, the patch will be reverted and the latest `webpack-sources` version will be updated in Next.js.

mknichel · 2024-05-23T23:01:44Z

I see, good catch, maybe we should review out codebase and try to only strings when it is possible, it can decrease build time for all too (feel free to do it), also can you accept CLA?

Thank you for the review. I signed the CLA and I applied the optimization to the other Source classes that cached copies of string and buffer. I didn't apply the string interning optimization to those classes since it can increase memory usage for some Source objects if they aren't stored for the entire Compilation so I wanted to limit it to just RawSource right now.

alexander-akait · 2024-06-07T15:10:04Z

/cc @sokra can you review it?

codecov · 2024-06-07T15:15:18Z

Codecov Report

Attention: Patch coverage is 95.45455% with 7 lines in your changes missing coverage. Please review.

Project coverage is 93.14%. Comparing base (19d4e8e) to head (dc41538).

Files	Patch %	Lines
lib/RawSource.js	83.33%	3 Missing and 1 partial ⚠️
lib/SourceMapSource.js	97.36%	2 Missing ⚠️
lib/CachedSource.js	94.11%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #155      +/-   ##
==========================================
- Coverage   93.48%   93.14%   -0.34%     
==========================================
  Files          23       24       +1     
  Lines        1703     1779      +76     
  Branches      539      566      +27     
==========================================
+ Hits         1592     1657      +65     
- Misses        104      112       +8     
- Partials        7       10       +3

Flag	Coverage Δ
integration	`93.14% <95.45%> (-0.34%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

alexander-akait · 2024-06-11T13:28:25Z

/cc @mknichel Hello, What is status?

mknichel · 2024-06-20T22:01:19Z

Hi @alexander-akait, Sorry I have been OOO and I believe @sokra has too. I pinged @sokra to take a look.

lib/helpers/stringBufferUtils.js

lib/RawSource.js

sokra · 2024-06-26T11:36:24Z

lib/RawSource.js

+		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;
+			}
 		}


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

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

sokra · 2024-06-26T11:37:02Z

lib/OriginalSource.js

@@ -9,6 +9,9 @@ const splitIntoLines = require("./helpers/splitIntoLines");
 const getGeneratedSourceInfo = require("./helpers/getGeneratedSourceInfo");
 const Source = require("./Source");
 const splitIntoPotentialTokens = require("./helpers/splitIntoPotentialTokens");
+const {
+	isDualStringBufferCachingEnabled
+} = require("./helpers/stringBufferUtils");

 class OriginalSource extends Source {
 	constructor(value, name) {


I think we want to internal OriginalSource value too?

I intentionally limited string interning just to RawSource for now since during my experiments, it seemed like memory may increase if applying the interning in other source objects (if it retains memory beyond the lifetime of the source object). I would need to go back and more rigorously determine the behavior for each class to understand where the memory is coming from, but at least for RawSource this was an improvement.

sokra · 2024-06-26T11:38:34Z

lib/SourceMapSource.js

@@ -8,6 +8,9 @@ const Source = require("./Source");
 const streamChunksOfSourceMap = require("./helpers/streamChunksOfSourceMap");
 const streamChunksOfCombinedSourceMap = require("./helpers/streamChunksOfCombinedSourceMap");
 const { getMap, getSourceAndMap } = require("./helpers/getFromStreamChunks");
+const {
+	isDualStringBufferCachingEnabled
+} = require("./helpers/stringBufferUtils");

 class SourceMapSource extends Source {
 	constructor(


I think we want to internal value and originalSource here?

Same comment as https://github.com/webpack/webpack-sources/pull/155/files#r1654650118.

alexander-akait · 2024-07-23T13:45:04Z

@sokra Do we need to make release?

…s in webpack-sources (#66003) This PR adds a flag to Next.js to enable Webpack options to improve memory usage. See webpack/webpack-sources#155 for a full description of the changes and impact on memory. This PR adds a patch to `webpack-sources` temporarily that contains the fixes as the real changes are iterated on to merge upstream in the `webpack/webpack-sources` repository. After that is done, the patch will be reverted and the latest `webpack-sources` version will be updated in Next.js.

mknichel added 2 commits May 20, 2024 15:40

[Memory] Add options to improve memory usage consumed by source objects

1b29b35

Add missing export

9c7704d

Remove webpack-sources-3.2.3.tgz

766b87a

mknichel mentioned this pull request May 20, 2024

[Memory] Add option to reduce memory usage caused by duplicate strings in webpack-sources vercel/next.js#66003

Merged

mknichel marked this pull request as ready for review May 20, 2024 23:44

alexander-akait reviewed May 22, 2024

View reviewed changes

update CLA

3d6ab16

sokra reviewed May 22, 2024

View reviewed changes

Address code review comments

dc41538

mknichel requested review from alexander-akait and sokra May 23, 2024 23:01

alexander-akait approved these changes Jun 7, 2024

View reviewed changes

sokra reviewed Jun 26, 2024

View reviewed changes

lib/helpers/stringBufferUtils.js Outdated Show resolved Hide resolved

sokra reviewed Jun 26, 2024

View reviewed changes

lib/RawSource.js Outdated Show resolved Hide resolved

sokra reviewed Jun 26, 2024

View reviewed changes

lib/RawSource.js Outdated Show resolved Hide resolved

sokra reviewed Jun 26, 2024

View reviewed changes

Address code review comments

a7a085f

mknichel requested a review from sokra July 17, 2024 23:22

mknichel mentioned this pull request Jul 18, 2024

Fix a bug in the webpack-sources patch that happens sometimes when using the webpack cache vercel/next.js#67928

Merged

sokra approved these changes Jul 22, 2024

View reviewed changes

sokra merged commit 88d0e04 into webpack:main Jul 22, 2024
19 of 22 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

mknichel commented May 20, 2024 •

edited

Loading

linux-foundation-easycla bot commented May 20, 2024 •

edited

Loading

alexander-akait left a comment

sokra May 22, 2024

mknichel May 23, 2024

mknichel commented May 23, 2024

alexander-akait commented Jun 7, 2024

codecov bot commented Jun 7, 2024 •

edited

Loading

alexander-akait commented Jun 11, 2024

mknichel commented Jun 20, 2024

sokra Jun 26, 2024

mknichel Jul 8, 2024

sokra Jun 26, 2024

mknichel Jul 8, 2024

sokra Jun 26, 2024

mknichel Jul 8, 2024

alexander-akait commented Jul 23, 2024

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

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

Conversation

mknichel commented May 20, 2024 • edited Loading

Disable dual caching of string and buffer contents

Intern strings across Raw Sources

Impact

linux-foundation-easycla bot commented May 20, 2024 • edited Loading

alexander-akait left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mknichel commented May 23, 2024

alexander-akait commented Jun 7, 2024

codecov bot commented Jun 7, 2024 • edited Loading

Codecov Report

alexander-akait commented Jun 11, 2024

mknichel commented Jun 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait commented Jul 23, 2024

mknichel commented May 20, 2024 •

edited

Loading

linux-foundation-easycla bot commented May 20, 2024 •

edited

Loading

codecov bot commented Jun 7, 2024 •

edited

Loading