Optimize memory usage for Next.js Image #23015
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shuding
commented
Mar 12, 2021
Comment on lines
+237
to
+245
| const etag = getHash([upstreamBuffer]) | ||
| sendResponse(req, res, upstreamType, etag, upstreamBuffer) | ||
| await writeToCacheDir( | ||
| hashDir, | ||
| upstreamType, | ||
| expireAt, | ||
| etag, | ||
| upstreamBuffer | ||
| ) |
There was a problem hiding this comment.
Moved etag outside to avoid duplicate calculations, since it can increase CPU usage when the image is large. Also moved sendResponse earlier for a better TTFB.
Stats from current PRDefault Server Mode (Increase detected
|
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| buildDuration | 13.5s | 13.9s | |
| nodeModulesSize | 42.8 MB | 42.8 MB |
Page Load Tests Overall increase ✓
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| / failed reqs | 0 | 0 | ✓ |
| / total time (seconds) | 2.496 | 2.451 | -0.04 |
| / avg req/sec | 1001.75 | 1019.88 | +18.13 |
| /error-in-render failed reqs | 0 | 0 | ✓ |
| /error-in-render total time (seconds) | 1.649 | 1.654 | 0 |
| /error-in-render avg req/sec | 1516.1 | 1511.55 |
Client Bundles (main, webpack, commons)
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| 677f882d2ed8..a2e7.js gzip | 13.4 kB | 13.4 kB | ✓ |
| framework.HASH.js gzip | 39 kB | 39 kB | ✓ |
| main-HASH.js gzip | 6.65 kB | 6.65 kB | ✓ |
| webpack-HASH.js gzip | 751 B | 751 B | ✓ |
| Overall change | 59.8 kB | 59.8 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31.3 kB | 31.3 kB | ✓ |
| Overall change | 31.3 kB | 31.3 kB | ✓ |
Client Pages
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| _app-fde3324..9dd1.js gzip | 1.28 kB | 1.28 kB | ✓ |
| _error-af59f..582f.js gzip | 3.46 kB | 3.46 kB | ✓ |
| amp-9716187d..0aa8.js gzip | 536 B | 536 B | ✓ |
| hooks-107e90..74c7.js gzip | 888 B | 888 B | ✓ |
| index-ac435c..ecf2.js gzip | 227 B | 227 B | ✓ |
| link-c0d2c96..de48.js gzip | 1.67 kB | 1.67 kB | ✓ |
| routerDirect..dc9d.js gzip | 303 B | 303 B | ✓ |
| withRouter-6..0e02.js gzip | 302 B | 302 B | ✓ |
| Overall change | 8.66 kB | 8.66 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 347 B | 347 B | ✓ |
| Overall change | 347 B | 347 B | ✓ |
Rendered Page Sizes
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| index.html gzip | 611 B | 611 B | ✓ |
| link.html gzip | 619 B | 619 B | ✓ |
| withRouter.html gzip | 607 B | 607 B | ✓ |
| Overall change | 1.84 kB | 1.84 kB | ✓ |
Serverless Mode
General Overall increase ⚠️
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| buildDuration | 15.7s | 15.5s | -175ms |
| nodeModulesSize | 42.8 MB | 42.8 MB |
Client Bundles (main, webpack, commons)
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| 677f882d2ed8..a2e7.js gzip | 13.4 kB | 13.4 kB | ✓ |
| framework.HASH.js gzip | 39 kB | 39 kB | ✓ |
| main-HASH.js gzip | 6.65 kB | 6.65 kB | ✓ |
| webpack-HASH.js gzip | 751 B | 751 B | ✓ |
| Overall change | 59.8 kB | 59.8 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31.3 kB | 31.3 kB | ✓ |
| Overall change | 31.3 kB | 31.3 kB | ✓ |
Client Pages
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| _app-fde3324..9dd1.js gzip | 1.28 kB | 1.28 kB | ✓ |
| _error-af59f..582f.js gzip | 3.46 kB | 3.46 kB | ✓ |
| amp-9716187d..0aa8.js gzip | 536 B | 536 B | ✓ |
| hooks-107e90..74c7.js gzip | 888 B | 888 B | ✓ |
| index-ac435c..ecf2.js gzip | 227 B | 227 B | ✓ |
| link-c0d2c96..de48.js gzip | 1.67 kB | 1.67 kB | ✓ |
| routerDirect..dc9d.js gzip | 303 B | 303 B | ✓ |
| withRouter-6..0e02.js gzip | 302 B | 302 B | ✓ |
| Overall change | 8.66 kB | 8.66 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 347 B | 347 B | ✓ |
| Overall change | 347 B | 347 B | ✓ |
Serverless bundles
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| _error.js | 1.02 MB | 1.02 MB | ✓ |
| 404.html | 2.67 kB | 2.67 kB | ✓ |
| 500.html | 2.65 kB | 2.65 kB | ✓ |
| amp.amp.html | 10.6 kB | 10.6 kB | ✓ |
| amp.html | 1.86 kB | 1.86 kB | ✓ |
| hooks.html | 1.92 kB | 1.92 kB | ✓ |
| index.js | 1.02 MB | 1.02 MB | ✓ |
| link.js | 1.08 MB | 1.08 MB | ✓ |
| routerDirect.js | 1.07 MB | 1.07 MB | ✓ |
| withRouter.js | 1.07 MB | 1.07 MB | ✓ |
| Overall change | 5.27 MB | 5.27 MB | ✓ |
Webpack 5 Mode (Decrease detected ✓)
General Overall increase ⚠️
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| buildDuration | 15.8s | 16.1s | |
| nodeModulesSize | 42.8 MB | 42.8 MB |
Page Load Tests Overall decrease ⚠️
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| / failed reqs | 0 | 0 | ✓ |
| / total time (seconds) | 2.478 | 2.38 | -0.1 |
| / avg req/sec | 1008.72 | 1050.39 | +41.67 |
| /error-in-render failed reqs | 0 | 0 | ✓ |
| /error-in-render total time (seconds) | 1.635 | 1.691 | |
| /error-in-render avg req/sec | 1528.99 | 1478.03 |
Client Bundles (main, webpack, commons)
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| 597-2bc2376a..203d.js gzip | 13.3 kB | 13.3 kB | ✓ |
| framework.HASH.js gzip | 39.3 kB | 39.3 kB | ✓ |
| main-HASH.js gzip | 6.59 kB | 6.59 kB | ✓ |
| webpack-HASH.js gzip | 954 B | 954 B | ✓ |
| Overall change | 60.2 kB | 60.2 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31.1 kB | 31.1 kB | ✓ |
| Overall change | 31.1 kB | 31.1 kB | ✓ |
Client Pages
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| _app-0c62a59..94b7.js gzip | 1.26 kB | 1.26 kB | ✓ |
| _error-97d24..ed28.js gzip | 3.38 kB | 3.38 kB | ✓ |
| amp-2926e4c2..9ccc.js gzip | 536 B | 536 B | ✓ |
| hooks-1ed65b..8908.js gzip | 902 B | 902 B | ✓ |
| index-6259b6..77d8.js gzip | 230 B | 230 B | ✓ |
| link-b722682..14a4.js gzip | 1.65 kB | 1.65 kB | ✓ |
| routerDirect..862a.js gzip | 306 B | 306 B | ✓ |
| withRouter-4..76fd.js gzip | 302 B | 302 B | ✓ |
| Overall change | 8.57 kB | 8.57 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 322 B | 322 B | ✓ |
| Overall change | 322 B | 322 B | ✓ |
Rendered Page Sizes
| vercel/next.js canary | shuding/next.js fix-optimize-image-memory | Change | |
|---|---|---|---|
| index.html gzip | 585 B | 585 B | ✓ |
| link.html gzip | 593 B | 593 B | ✓ |
| withRouter.html gzip | 579 B | 579 B | ✓ |
| Overall change | 1.76 kB | 1.76 kB | ✓ |
Diffs
Diff for index.html
@@ -43,7 +43,7 @@
"props": { "pageProps": {} },
"page": "/",
"query": {},
- "buildId": "gNZXdTDVqNEYPYNiOeH1m",
+ "buildId": "3ZtABPLs8K9pcllagbI85",
"isFallback": false,
"gip": true
}
@@ -77,11 +77,11 @@
async=""
></script>
<script
- src="/_next/static/gNZXdTDVqNEYPYNiOeH1m/_buildManifest.js"
+ src="/_next/static/3ZtABPLs8K9pcllagbI85/_buildManifest.js"
async=""
></script>
<script
- src="/_next/static/gNZXdTDVqNEYPYNiOeH1m/_ssgManifest.js"
+ src="/_next/static/3ZtABPLs8K9pcllagbI85/_ssgManifest.js"
async=""
></script>
</body>Diff for link.html
@@ -48,7 +48,7 @@
"props": { "pageProps": {} },
"page": "/link",
"query": {},
- "buildId": "gNZXdTDVqNEYPYNiOeH1m",
+ "buildId": "3ZtABPLs8K9pcllagbI85",
"isFallback": false,
"gip": true
}
@@ -82,11 +82,11 @@
async=""
></script>
<script
- src="/_next/static/gNZXdTDVqNEYPYNiOeH1m/_buildManifest.js"
+ src="/_next/static/3ZtABPLs8K9pcllagbI85/_buildManifest.js"
async=""
></script>
<script
- src="/_next/static/gNZXdTDVqNEYPYNiOeH1m/_ssgManifest.js"
+ src="/_next/static/3ZtABPLs8K9pcllagbI85/_ssgManifest.js"
async=""
></script>
</body>Diff for withRouter.html
@@ -43,7 +43,7 @@
"props": { "pageProps": {} },
"page": "/withRouter",
"query": {},
- "buildId": "gNZXdTDVqNEYPYNiOeH1m",
+ "buildId": "3ZtABPLs8K9pcllagbI85",
"isFallback": false,
"gip": true
}
@@ -77,11 +77,11 @@
async=""
></script>
<script
- src="/_next/static/gNZXdTDVqNEYPYNiOeH1m/_buildManifest.js"
+ src="/_next/static/3ZtABPLs8K9pcllagbI85/_buildManifest.js"
async=""
></script>
<script
- src="/_next/static/gNZXdTDVqNEYPYNiOeH1m/_ssgManifest.js"
+ src="/_next/static/3ZtABPLs8K9pcllagbI85/_ssgManifest.js"
async=""
></script>
</body>
Failing test suitesCommit: f7ecdca test/integration/image-optimizer/test/index.test.js
Expand output● Image Optimizer › dev support w/o next.config.js › should use cached image file when parameters are the same ● Image Optimizer › dev support w/o next.config.js › should use cached image file when parameters are the same for svg ● Image Optimizer › dev support w/o next.config.js › should use cached image file when parameters are the same for animated gif ● Image Optimizer › dev support w/o next.config.js › should proxy-pass unsupported image types and should not cache file ● Image Optimizer › dev support with next.config.js › should use cached image file when parameters are the same ● Image Optimizer › dev support with next.config.js › should use cached image file when parameters are the same for svg ● Image Optimizer › dev support with next.config.js › should proxy-pass unsupported image types and should not cache file ● Image Optimizer › Server support w/o next.config.js › should use cached image file when parameters are the same ● Image Optimizer › Server support w/o next.config.js › should use cached image file when parameters are the same for svg ● Image Optimizer › Server support w/o next.config.js › should proxy-pass unsupported image types and should not cache file ● Image Optimizer › Server support with next.config.js › should use cached image file when parameters are the same ● Image Optimizer › Server support with next.config.js › should use cached image file when parameters are the same for svg ● Image Optimizer › Server support with next.config.js › should proxy-pass unsupported image types and should not cache file ● Image Optimizer › Serverless support with next.config.js › should use cached image file when parameters are the same ● Image Optimizer › Serverless support with next.config.js › should use cached image file when parameters are the same for svg ● Image Optimizer › Serverless support with next.config.js › should use cached image file when parameters are the same for animated gif ● Image Optimizer › Serverless support with next.config.js › should proxy-pass unsupported image types and should not cache file |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
We recently noticed that #22253 introduced some high memory usage issue for image-optimizer. Here's a simple way that I'm using to reproduce it:
The memory usage increased from 19.3MB to 52.9MB (+274%), and most of the new allocated memory was taken by
ArrayBufferof ImageData, which should have been recollected after an image optimization request was done:When looking at the reference chain I noticed that they're hold by jest-worker (Farm.js) inside the worker thread. Which is very likely a memory leak.
Root Cause
We are currently using
jest-worker@24.9.0(code) and as suggested in the screenshot above, it keeps task arguments inside a linked list called_last(code) but there's no removal action for it. So even with GC, the current head of that linked list won't be recollected as it will always have a reference.After fixing that locally and with some further investigations, I noticed more memory related problems. The
onStartandonEndcallbacks here and...argswill combine together as closures, andtaskwill be passed to each worker (code) with another closure. Inside the worker there's another closure that keeps the previous values until_onProcessEnd(code).That means, the arguments (and potentially the return value, due to Promise related closures) of the task we are doing won't be recollected unless the worker receives a new task, so all function references can be replaced.
An estimation of leaked memory size will be ~
N ⨉ SizeOfOriginalImage ⨉ 2, where N is the number of workers (CPU cores), with some other overheads.Fix
I first checked if the latest version
jest-worker@nexthas fixed those issue, but unfortunately not. The latest version reimplemented the queue but still holds the reference in these closures internally. Although the same issue was roughly mentioned in the PR, the memory problem isn't fixed in my test app:It's not easy to get it fixed by doing more investigations and sending a patch to jest-worker, I suppose that Farm and Worker needs some non-trivial changes. And this problem will mostly only hurt us, because we are sending large objects as task arguments and results.
Given the priority of this issue inside the image optimizer, a work around is to pass some extra tasks to clear out the current queue head and replace those internal values inside each worker so the previous task can be GC'd completely. I added a no-op method and make sure it will be called N times after a Squoosh task.
With the fix the same app will not cause huge memory increase anymore (19.1MB → 19.9MB), and the ImageData and Buffer will be recollected successfully. There's small memory increase due to cache inside the HTTP module and extra loaded code.
Potentially fixes #22925.