Fix issue where generateStaticParams, combined with parallel routes, can cause JS Out of Memory during build #73871

icyJoseph · 2024-12-12T22:44:54Z

TODO

Preferred way to test this one? I feel like I need a suggestion from core maintainers
Explore a better way to ensure unique pages/segments

What?

92b3dd2 Introduced some changes that, accidentally cause the number of parentsParams, to grow exponentially.

When generateStaticParams, returns about 29 elements, the build barely passes. Adding one more, causes JS to crash Out of Memory.

With 39 elements returned from generateStaticParams, you can see quickly, parentParams cross up to millions of elements. I've dumped more info into #73793

Why?

Having a build ran out of memory because generateStaticParams return 30 elements, is not acceptable. In certain applications, these functions return several more elements.

How?

The problem is that the segments, keep adding the same segment, three times over, once per parallel route, if I am not mistaken.

Tip

See how the [slug]/layout.tsx bit is added twice (in the screenshot)
You can long this info at packages/next/src/build/utils.ts, within the buildAppStaticPaths function, at the builtRouteParams callback, by logging parentsParams within the callback scope.

By doing this:

    if (name === PAGE_SEGMENT_KEY) {
      currentSegments.forEach((seg) => {
        if (
          segments.some(
            (other) =>
              other.name === seg.name &&
              other.filePath === seg.filePath &&
              other.param === seg.param
          )
        ) {
          return
        }
        segments.push(seg)
      })
    }

As a means to ensure uniqueness of the segments. Of course this is a "poor" mans solution, but it seems to work well so far. Ideally, we should have a cache, or set, to quickly check if a segment as already been added.

Fixes #73793

ijjk · 2024-12-12T22:45:18Z

Allow CI Workflow Run

approve CI run for commit: 5c975b6

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

ijjk · 2024-12-12T22:45:18Z

Allow CI Workflow Run

approve CI run for commit: 5c975b6

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

…ial-page-growth

ijjk · 2024-12-13T14:54:52Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall increase ⚠️

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
buildDuration	18.7s	15.6s	N/A
buildDurationCached	14.7s	12.4s	N/A
nodeModulesSize	410 MB	410 MB	⚠️ +3.01 kB
nextStartRea..uration (ms)	473ms	478ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
1187-HASH.js gzip	50.8 kB	50.8 kB	N/A
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.36 kB	5.36 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	232 B	235 B	N/A
main-HASH.js gzip	34 kB	34 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	0 B	0 B	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	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	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	512 B	510 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	✓
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.49 kB	4.49 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	3.59 kB	3.59 kB	✓

Client Build Manifests

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
_buildManifest.js gzip	749 B	746 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
index.html gzip	525 B	523 B	N/A
link.html gzip	539 B	537 B	N/A
withRouter.html gzip	520 B	521 B	N/A
Overall change	0 B	0 B	✓

Edge SSR bundle Size

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
edge-ssr.js gzip	128 kB	128 kB	N/A
page.js gzip	203 kB	203 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
middleware-b..fest.js gzip	671 B	667 B	N/A
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.2 kB	31.2 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	844 B	844 B	✓

Next Runtimes

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
523-experime...dev.js gzip	322 B	322 B	✓
523.runtime.dev.js gzip	314 B	314 B	✓
app-page-exp...dev.js gzip	323 kB	323 kB	✓
app-page-exp..prod.js gzip	127 kB	127 kB	✓
app-page-tur..prod.js gzip	140 kB	140 kB	✓
app-page-tur..prod.js gzip	135 kB	135 kB	✓
app-page.run...dev.js gzip	313 kB	313 kB	✓
app-page.run..prod.js gzip	123 kB	123 kB	✓
app-route-ex...dev.js gzip	37.3 kB	37.3 kB	✓
app-route-ex..prod.js gzip	25.3 kB	25.3 kB	✓
app-route-tu..prod.js gzip	25.4 kB	25.4 kB	✓
app-route-tu..prod.js gzip	25.2 kB	25.2 kB	✓
app-route.ru...dev.js gzip	38.9 kB	38.9 kB	✓
app-route.ru..prod.js gzip	25.2 kB	25.2 kB	✓
pages-api-tu..prod.js gzip	9.67 kB	9.67 kB	✓
pages-api.ru...dev.js gzip	11.6 kB	11.6 kB	✓
pages-api.ru..prod.js gzip	9.67 kB	9.67 kB	✓
pages-turbo...prod.js gzip	21.7 kB	21.7 kB	✓
pages.runtim...dev.js gzip	27.4 kB	27.4 kB	✓
pages.runtim..prod.js gzip	21.7 kB	21.7 kB	✓
server.runti..prod.js gzip	916 kB	916 kB	✓
Overall change	2.36 MB	2.36 MB	✓

build cache Overall increase ⚠️

	vercel/next.js canary	icyJoseph/next.js fix/generate-static-params-causes-exponential-page-growth	Change
0.pack gzip	2.05 MB	2.05 MB	⚠️ +1.57 kB
index.pack gzip	72.1 kB	72 kB	N/A
Overall change	2.05 MB	2.05 MB	⚠️ +1.57 kB

Diff details

Diff for main-HASH.js

Diff too large to display

Commit: fa176c2

ijjk · 2024-12-13T15:06:48Z

Failing test suites

Commit: fa176c2

__NEXT_EXPERIMENTAL_PPR=true pnpm test-dev test/e2e/app-dir/actions/app-action.test.ts (PPR)

app-dir action handling > should forward action request to a worker that contains the action handler (edge)

Expand output

● app-dir action handling › should forward action request to a worker that contains the action handler (edge)

page.waitForSelector: Timeout 60000ms exceeded.
Call log:
  - waiting for locator('#other-page')

  423 |     return this.chain(() => {
  424 |       return page
> 425 |         .waitForSelector(selector, { timeout, state: 'attached' })
      |          ^
  426 |         .then(async (el) => {
  427 |           // it seems selenium waits longer and tests rely on this behavior
  428 |           // so we wait for the load event fire before returning

  at waitForSelector (lib/browsers/playwright.ts:425:10)
  at e2e/app-dir/actions/app-action.test.ts:862:7

Read more about building and testing Next.js in contributing.md.

ztanner · 2024-12-13T15:34:05Z

Thank you for digging deep on this @icyJoseph! If it's alright with you, I've went ahead and refactored this function further in #73908, including the deduping adjustment which has been implemented in a map with composite cache keys.

icyJoseph · 2024-12-13T15:35:32Z

Yeah that's a better approach, no worries! Should we close this PR then?

@icyJoseph

This refactors `collectAppPageSegments` to be more efficient especially when dealing with multiple parallel routes, where the number of combinations of segments can drastically increase how many segments are returned per route. Previously this function only considered the `children` parallel route. When it was updated to consider all possible parallel routes in #73358, this had the unintended side effect of increasing the amount of duplicate segments this function would have to process. We only need to return unique segments discovered from a particular loader tree (representing a page), as opposed to the same segment as many times as it might appear across all parallel routes. This PR uses a map to track unique segments with a composite key used to identify the discovered segments. Additionally, this refactors the function to be iterative rather than recursive. We keep track of a queue of segments to be processed (a tuple of `[loaderTree, segments]`), and as we discover new parallel route paths, we process the queue further. Fixes #73793 Closes #73871 (h/t to @icyJoseph for identifying the cause of the memory consumption)

@icyJoseph

This refactors `collectAppPageSegments` to be more efficient especially when dealing with multiple parallel routes, where the number of combinations of segments can drastically increase how many segments are returned per route. Previously this function only considered the `children` parallel route. When it was updated to consider all possible parallel routes in #73358, this had the unintended side effect of increasing the amount of duplicate segments this function would have to process. We only need to return unique segments discovered from a particular loader tree (representing a page), as opposed to the same segment as many times as it might appear across all parallel routes. This PR uses a map to track unique segments with a composite key used to identify the discovered segments. Additionally, this refactors the function to be iterative rather than recursive. We keep track of a queue of segments to be processed (a tuple of `[loaderTree, segments]`), and as we discover new parallel route paths, we process the queue further. Fixes #73793 Closes #73871 (h/t to @icyJoseph for identifying the cause of the memory consumption)

fix: Ensure unique segments, w/ a manual unoptimized search

5c975b6

ijjk added the type: next label Dec 12, 2024

icyJoseph changed the title ~~fix: Ensure unique segments, w/ a manual unoptimized search~~ generateStaticParams, combined with parallel routes, can cause JS Out of Memory during build Dec 12, 2024

icyJoseph changed the title ~~generateStaticParams, combined with parallel routes, can cause JS Out of Memory during build~~ Fix issue where generateStaticParams, combined with parallel routes, can cause JS Out of Memory during build Dec 12, 2024

Merge branch 'canary' into fix/generate-static-params-causes-exponent…

fa176c2

…ial-page-growth

ztanner mentioned this pull request Dec 13, 2024

refactor: collectAppPageSegments #73908

Merged

icyJoseph closed this Dec 13, 2024

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue where generateStaticParams, combined with parallel routes, can cause JS Out of Memory during build #73871

Fix issue where generateStaticParams, combined with parallel routes, can cause JS Out of Memory during build #73871

icyJoseph commented Dec 12, 2024 •

edited

Loading

ijjk commented Dec 12, 2024

ijjk commented Dec 12, 2024

ijjk commented Dec 13, 2024

ijjk commented Dec 13, 2024

ztanner commented Dec 13, 2024 •

edited

Loading

icyJoseph commented Dec 13, 2024

Fix issue where generateStaticParams, combined with parallel routes, can cause JS Out of Memory during build #73871

Fix issue where generateStaticParams, combined with parallel routes, can cause JS Out of Memory during build #73871

Conversation

icyJoseph commented Dec 12, 2024 • edited Loading

TODO

What?

Why?

How?

ijjk commented Dec 12, 2024

ijjk commented Dec 12, 2024

ijjk commented Dec 13, 2024

Stats from current PR

ijjk commented Dec 13, 2024

Failing test suites

ztanner commented Dec 13, 2024 • edited Loading

icyJoseph commented Dec 13, 2024

icyJoseph commented Dec 12, 2024 •

edited

Loading

ztanner commented Dec 13, 2024 •

edited

Loading