webpack / webpack-sources

Source code handling classes for webpack
MIT License
261 stars 71 forks source link

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

Closed mknichel closed 2 months ago

mknichel commented 4 months ago

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.

linux-foundation-easycla[bot] commented 4 months ago

CLA Signed

The committers listed above are authorized under a signed CLA.

mknichel commented 4 months ago

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 commented 3 months ago

/cc @sokra can you review it?

codecov[bot] commented 3 months ago

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 :warning:
lib/SourceMapSource.js 97.36% 2 Missing :warning:
lib/CachedSource.js 94.11% 1 Missing :warning:
Additional details and impacted files ```diff @@ 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](https://app.codecov.io/gh/webpack/webpack-sources/pull/155/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webpack) | Coverage Δ | | |---|---|---| | [integration](https://app.codecov.io/gh/webpack/webpack-sources/pull/155/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webpack) | `93.14% <95.45%> (-0.34%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=webpack#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

alexander-akait commented 3 months ago

/cc @mknichel Hello, What is status?

mknichel commented 3 months ago

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

alexander-akait commented 2 months ago

@sokra Do we need to make release?