[Segment Cache] Use LRU to manage cache data by acdlite · Pull Request #73486 · vercel/next.js

acdlite · 2024-12-03T22:38:30Z

Based on:

[Segment Cache] Interception routes #73434

This implements an LRU for data stored in the Segment Cache.

For now I've hardcoded in a cache size of 50mb, but as a follow up, we should look into creating a dynamic limit based on device memory constraints.

Segments and Route entries are managed by separate LRUs. We could combine them into a single LRU, but because they are separate types, we'd need to wrap each one in an extra LRU node (to maintain monomorphism, at the cost of additional memory).

ijjk · 2024-12-03T22:43:16Z

Tests Passed

ijjk · 2024-12-03T22:47:29Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
buildDuration	20.2s	17.5s	N/A
buildDurationCached	16.6s	14.6s	N/A
nodeModulesSize	409 MB	409 MB	⚠️ +244 kB
nextStartRea..uration (ms)	450ms	455ms	N/A

Client Bundles (main, webpack) Overall increase ⚠️

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
1187-HASH.js gzip	49.6 kB	50.1 kB	⚠️ +454 B
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.3 kB	5.3 kB	N/A
bccd1874-HASH.js gzip	53 kB	53 kB	N/A
framework-HASH.js gzip	57.5 kB	57.5 kB	N/A
main-app-HASH.js gzip	233 B	235 B	N/A
main-HASH.js gzip	33.7 kB	33.7 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	49.6 kB	50.1 kB	⚠️ +454 B

Legacy Client Bundles (polyfills)

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
polyfills-HASH.js gzip	39.4 kB	39.4 kB	✓
Overall change	39.4 kB	39.4 kB	✓

Client Pages

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	513 B	511 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	N/A
edge-ssr-HASH.js gzip	265 B	265 B	✓
head-HASH.js gzip	363 B	362 B	N/A
hooks-HASH.js gzip	393 B	392 B	N/A
image-HASH.js gzip	4.44 kB	4.43 kB	N/A
index-HASH.js gzip	268 B	268 B	✓
link-HASH.js gzip	2.35 kB	2.34 kB	N/A
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	323 B	326 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	1.75 kB	1.75 kB	✓

Client Build Manifests

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
_buildManifest.js gzip	746 B	747 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
index.html gzip	523 B	524 B	N/A
link.html gzip	537 B	537 B	✓
withRouter.html gzip	519 B	521 B	N/A
Overall change	537 B	537 B	✓

Edge SSR bundle Size Overall increase ⚠️

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
edge-ssr.js gzip	128 kB	128 kB	N/A
page.js gzip	200 kB	202 kB	⚠️ +1.86 kB
Overall change	200 kB	202 kB	⚠️ +1.86 kB

Middleware size

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
middleware-b..fest.js gzip	667 B	664 B	N/A
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.1 kB	31.1 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	844 B	844 B	✓

Next Runtimes Overall increase ⚠️

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
797-experime...dev.js gzip	322 B	322 B	✓
797.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	321 kB	322 kB	⚠️ +637 B
app-page-exp..prod.js gzip	126 kB	126 kB	⚠️ +412 B
app-page-tur..prod.js gzip	139 kB	139 kB	⚠️ +411 B
app-page-tur..prod.js gzip	134 kB	135 kB	⚠️ +406 B
app-page.run...dev.js gzip	312 kB	312 kB	⚠️ +627 B
app-page.run..prod.js gzip	122 kB	122 kB	⚠️ +409 B
app-route-ex...dev.js gzip	36.8 kB	36.8 kB	✓
app-route-ex..prod.js gzip	25 kB	25 kB	✓
app-route-tu..prod.js gzip	25 kB	25 kB	✓
app-route-tu..prod.js gzip	24.8 kB	24.8 kB	✓
app-route.ru...dev.js gzip	38.5 kB	38.5 kB	✓
app-route.ru..prod.js gzip	24.8 kB	24.8 kB	✓
pages-api-tu..prod.js gzip	9.56 kB	9.56 kB	✓
pages-api.ru...dev.js gzip	11.4 kB	11.4 kB	✓
pages-api.ru..prod.js gzip	9.56 kB	9.56 kB	✓
pages-turbo...prod.js gzip	21.3 kB	21.3 kB	✓
pages.runtim...dev.js gzip	27 kB	27 kB	✓
pages.runtim..prod.js gzip	21.3 kB	21.3 kB	✓
server.runti..prod.js gzip	916 kB	916 kB	✓
Overall change	2.35 MB	2.35 MB	⚠️ +2.9 kB

build cache Overall increase ⚠️

	vercel/next.js canary	acdlite/next.js segment-cache-lru	Change
0.pack gzip	2.04 MB	2.03 MB	N/A
index.pack gzip	145 kB	146 kB	⚠️ +350 B
Overall change	145 kB	146 kB	⚠️ +350 B

Diff details

Diff for 1187-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Diff for app-page.runtime.prod.js

Diff too large to display

Commit: 6ae540a

ztanner · 2024-12-05T20:42:51Z

test/e2e/app-dir/segment-cache/memory-pressure/segment-cache-memory-pressure.test.ts

+    skipDeployment: true,
+  })
+  if (isNextDev || skipped) {
+    test('ppr is disabled', () => {})


Suggested change

test('ppr is disabled', () => {})

test('disabled in development / deployment', () => {})

ztanner · 2024-12-05T20:45:06Z

test/lib/browsers/playwright.ts

+  getPage() {
+    return page
+  }


If you intended to introduce this as part of this changeset I think you'll also need to add it to the BrowserInterface. Though maybe better to delete for now until we use it in a test so we can validate it's plumbed correctly

yeah I don't need it until a later PR, I'll move it to that one

ztanner · 2024-12-05T20:59:30Z

packages/next/src/client/components/segment-cache/cache.ts

+
+// We use an LRU for memory management. We must update this whenever we add or
+// remove a new cache entry, or when an entry changes size.
+// TODO: I chose the max size somewhat arbitrarily. Consider setting this based
+// on navigator.deviceMemory, or some other heuristic. We should make this
+// customizable via the Next.js config, too.


I think this detached the comment about storing search params in the segment cache, as it used to live next to the creation of the segmentCacheMap

you mean this one on line 151

https://github.com/vercel/next.js/blob/59af01f4f8beee75c774c640779b9bc273f106b8/packages/next/src/client/components/segment-cache/cache.ts#L151C4-L152

yea, it felt like it was floating when it was above the LRU comment. I think it just needs to be moved to 164

This implements an LRU for data stored in the Segment Cache. For now I've hardcoded in a cache size of 50mb, but as a follow up, we should look into creating a dynamic limit based on device memory constraints. Segments and Route entries are managed by separate LRUs. We could combine them into a single LRU, but because they are separate types, we'd need to wrap each one in an extra LRU node (to maintain monomorphism, at the cost of additional memory).

Based on - #73434 - #73486 --- During a navigation, we should be able to skip the dynamic request if the prefetched data does not contain any dynamic holes. In the previous cache implementation, we only tracked this per route; in the Segment Cache, we must track this per segment. This updates the SegmentCacheEntry and CacheNodeSeedData types to include an `isPartial` field. The field is always false during a dynamic render, or when PPR is disabled. This PR does not change any behavior; it only adds the new field.

Based on: - #73434 - #73486 - #73528 --- Similar to #73528, but for the head, which is delivered separately from the segments. We can only skip the dynamic request if this value is `false`.

Based on: - #73434 --- This implements an LRU for data stored in the Segment Cache. For now I've hardcoded in a cache size of 50mb, but as a follow up, we should look into creating a dynamic limit based on device memory constraints. Segments and Route entries are managed by separate LRUs. We could combine them into a single LRU, but because they are separate types, we'd need to wrap each one in an extra LRU node (to maintain monomorphism, at the cost of additional memory).

Based on - #73434 - #73486 --- During a navigation, we should be able to skip the dynamic request if the prefetched data does not contain any dynamic holes. In the previous cache implementation, we only tracked this per route; in the Segment Cache, we must track this per segment. This updates the SegmentCacheEntry and CacheNodeSeedData types to include an `isPartial` field. The field is always false during a dynamic render, or when PPR is disabled. This PR does not change any behavior; it only adds the new field.

ijjk added created-by: Next.js team PRs by the Next.js team. tests type: next labels Dec 3, 2024

acdlite requested a review from ztanner December 4, 2024 04:13

This was referenced Dec 4, 2024

[Segment Cache] Add isPartial to segment prefetch #73528

Merged

[Segment Cache] Add isHeadPartial #73530

Merged

[Segment Cache] Skip dynamic request if possible #73540

Merged

ztanner reviewed Dec 5, 2024

View reviewed changes

acdlite force-pushed the segment-cache-lru branch 2 times, most recently from 3d823b2 to 86e39dc Compare December 6, 2024 16:35

acdlite force-pushed the segment-cache-lru branch from 86e39dc to 6ae540a Compare December 6, 2024 16:35

acdlite requested a review from ztanner December 6, 2024 16:36

ztanner approved these changes Dec 6, 2024

View reviewed changes

acdlite merged commit 144fd73 into vercel:canary Dec 6, 2024

acdlite mentioned this pull request Dec 6, 2024

[Segment Cache] Skip prefetched segments on server #73626

Merged

github-actions bot added the locked label Dec 21, 2024

github-actions bot locked as resolved and limited conversation to collaborators Dec 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Segment Cache] Use LRU to manage cache data #73486

[Segment Cache] Use LRU to manage cache data #73486
acdlite merged 1 commit intovercel:canaryfrom
acdlite:segment-cache-lru

acdlite commented Dec 3, 2024 •

edited

Loading

Uh oh!

ijjk commented Dec 3, 2024 •

edited

Loading

Uh oh!

ijjk commented Dec 3, 2024 •

edited

Loading

Uh oh!

ztanner Dec 5, 2024

Uh oh!

ztanner Dec 5, 2024

Uh oh!

acdlite Dec 6, 2024

Uh oh!

ztanner Dec 5, 2024

Uh oh!

acdlite Dec 6, 2024 •

edited

Loading

Uh oh!

ztanner Dec 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	test('ppr is disabled', () => {})
	test('disabled in development / deployment', () => {})

Conversation

acdlite commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ijjk commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tests Passed

Uh oh!

ijjk commented Dec 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stats from current PR

Uh oh!

ztanner Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

ztanner Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

acdlite Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

ztanner Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

acdlite Dec 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ztanner Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

acdlite commented Dec 3, 2024 •

edited

Loading

ijjk commented Dec 3, 2024 •

edited

Loading

ijjk commented Dec 3, 2024 •

edited

Loading

acdlite Dec 6, 2024 •

edited

Loading