Skip to content

Commit 9b0277f

Browse files
authored
Turbopack: Pass asset_suffix_path as Vc (#89899)
## Summary - Pass `asset_suffix_path` as `Vc<Option<RcStr>>` instead of eagerly resolving it to `Option<RcStr>` in the chunking context option structs - Add filesystem cache size growth assertions to e2e tests to detect unbounded cache growth regressions ## Why Previously, `css_url_suffix` was eagerly awaited in `project.rs` before being passed into `ClientChunkingContextOptions`, `ServerChunkingContextOptions`, and `EdgeChunkingContextOptions`. This caused a new chunking context `Vc` to be created for every build, duplicating the entire build in cache and recompiling it. By keeping it as a `Vc`, the chunking context identity is stable across builds, preventing unnecessary cache duplication. ## How - Changed `css_url_suffix` field from `Option<RcStr>` to `Vc<Option<RcStr>>` in all three chunking context option structs - Removed `.owned().await?.clone()` in `project.rs` (3 call sites), passing the `Vc` directly - Added `.owned().await?` at the point of use in the 5 context builder functions - Added cache size measurements to `filesystem-cache.test.ts`: normal changes limited to 10% growth, renames allow up to 50% due to dead cache entries
1 parent 6627734 commit 9b0277f

File tree

5 files changed

+78
-23
lines changed

5 files changed

+78
-23
lines changed

crates/next-api/src/project.rs

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,12 +1440,7 @@ impl Project {
14401440
pub(super) async fn client_chunking_context(
14411441
self: Vc<Self>,
14421442
) -> Result<Vc<Box<dyn ChunkingContext>>> {
1443-
let css_url_suffix = self
1444-
.next_config()
1445-
.asset_suffix_path()
1446-
.owned()
1447-
.await?
1448-
.clone();
1443+
let css_url_suffix = self.next_config().asset_suffix_path();
14491444
Ok(get_client_chunking_context(ClientChunkingContextOptions {
14501445
mode: self.next_mode(),
14511446
root_path: self.project_root_path().owned().await?,
@@ -1474,12 +1469,7 @@ impl Project {
14741469
self: Vc<Self>,
14751470
client_assets: bool,
14761471
) -> Result<Vc<NodeJsChunkingContext>> {
1477-
let css_url_suffix = self
1478-
.next_config()
1479-
.asset_suffix_path()
1480-
.owned()
1481-
.await?
1482-
.clone();
1472+
let css_url_suffix = self.next_config().asset_suffix_path();
14831473
let options = ServerChunkingContextOptions {
14841474
mode: self.next_mode(),
14851475
root_path: self.project_root_path().owned().await?,
@@ -1513,12 +1503,7 @@ impl Project {
15131503
self: Vc<Self>,
15141504
client_assets: bool,
15151505
) -> Result<Vc<Box<dyn ChunkingContext>>> {
1516-
let css_url_suffix = self
1517-
.next_config()
1518-
.asset_suffix_path()
1519-
.owned()
1520-
.await?
1521-
.clone();
1506+
let css_url_suffix = self.next_config().asset_suffix_path();
15221507
let options = EdgeChunkingContextOptions {
15231508
mode: self.next_mode(),
15241509
root_path: self.project_root_path().owned().await?,

crates/next-core/src/next_client/context.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ pub struct ClientChunkingContextOptions {
449449
pub nested_async_chunking: Vc<bool>,
450450
pub debug_ids: Vc<bool>,
451451
pub should_use_absolute_url_references: Vc<bool>,
452-
pub css_url_suffix: Option<RcStr>,
452+
pub css_url_suffix: Vc<Option<RcStr>>,
453453
}
454454

455455
#[turbo_tasks::function]
@@ -475,6 +475,7 @@ pub async fn get_client_chunking_context(
475475
should_use_absolute_url_references,
476476
css_url_suffix,
477477
} = options;
478+
let css_url_suffix = css_url_suffix.owned().await?;
478479

479480
let next_mode = mode.await?;
480481
let asset_prefix = asset_prefix.owned().await?;

crates/next-core/src/next_edge/context.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ pub struct EdgeChunkingContextOptions {
223223
pub nested_async_chunking: Vc<bool>,
224224
pub client_root: FileSystemPath,
225225
pub asset_prefix: RcStr,
226-
pub css_url_suffix: Option<RcStr>,
226+
pub css_url_suffix: Vc<Option<RcStr>>,
227227
}
228228

229229
/// Like `get_edge_chunking_context` but all assets are emitted as client assets (so `/_next`)
@@ -249,6 +249,7 @@ pub async fn get_edge_chunking_context_with_client_assets(
249249
asset_prefix,
250250
css_url_suffix,
251251
} = options;
252+
let css_url_suffix = css_url_suffix.owned().await?;
252253
let output_root = node_root.join("server/edge")?;
253254
let next_mode = mode.await?;
254255
let mut builder = BrowserChunkingContext::builder(
@@ -326,6 +327,7 @@ pub async fn get_edge_chunking_context(
326327
asset_prefix,
327328
css_url_suffix,
328329
} = options;
330+
let css_url_suffix = css_url_suffix.owned().await?;
329331
let output_root = node_root.join("server/edge")?;
330332
let next_mode = mode.await?;
331333
let mut builder = BrowserChunkingContext::builder(

crates/next-core/src/next_server/context.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1035,7 +1035,7 @@ pub struct ServerChunkingContextOptions {
10351035
pub debug_ids: Vc<bool>,
10361036
pub client_root: FileSystemPath,
10371037
pub asset_prefix: RcStr,
1038-
pub css_url_suffix: Option<RcStr>,
1038+
pub css_url_suffix: Vc<Option<RcStr>>,
10391039
}
10401040

10411041
/// Like `get_server_chunking_context` but all assets are emitted as client assets (so `/_next`)
@@ -1062,6 +1062,7 @@ pub async fn get_server_chunking_context_with_client_assets(
10621062
asset_prefix,
10631063
css_url_suffix,
10641064
} = options;
1065+
let css_url_suffix = css_url_suffix.owned().await?;
10651066

10661067
let next_mode = mode.await?;
10671068
// TODO(alexkirsz) This should return a trait that can be implemented by the
@@ -1159,6 +1160,7 @@ pub async fn get_server_chunking_context(
11591160
asset_prefix,
11601161
css_url_suffix,
11611162
} = options;
1163+
let css_url_suffix = css_url_suffix.owned().await?;
11621164
let next_mode = mode.await?;
11631165
// TODO(alexkirsz) This should return a trait that can be implemented by the
11641166
// different server chunking contexts. OR the build chunking context should

test/e2e/filesystem-cache/filesystem-cache.test.ts

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,29 @@
11
/* eslint-disable jest/no-standalone-expect */
22
import { nextTestSetup, isNextDev } from 'e2e-utils'
33
import { waitFor } from 'next-test-utils'
4+
import fs from 'fs/promises'
5+
import path from 'path'
6+
7+
async function getDirectorySize(dirPath: string): Promise<number> {
8+
try {
9+
await fs.access(dirPath)
10+
} catch {
11+
return 0
12+
}
13+
let totalSize = 0
14+
const entries = await fs.readdir(dirPath, {
15+
recursive: true,
16+
withFileTypes: true,
17+
})
18+
for (const entry of entries) {
19+
if (entry.isFile()) {
20+
const filePath = path.join(entry.parentPath ?? entry.path, entry.name)
21+
const stat = await fs.stat(filePath)
22+
totalSize += stat.size
23+
}
24+
}
25+
return totalSize
26+
}
427

528
for (const cacheEnabled of [false, true]) {
629
describe(`filesystem-caching with cache ${cacheEnabled ? 'enabled' : 'disabled'}`, () => {
@@ -46,9 +69,11 @@ for (const cacheEnabled of [false, true]) {
4669
;(next as any).handleDevWatchDelayAfterChange = () => {}
4770
})
4871

49-
async function restartCycle() {
72+
async function restartCycle(): Promise<number> {
5073
await stop()
74+
const cacheSize = await getCacheSize()
5175
await start()
76+
return cacheSize
5277
}
5378

5479
async function stop() {
@@ -66,6 +91,12 @@ for (const cacheEnabled of [false, true]) {
6691
await next.start()
6792
}
6893

94+
async function getCacheSize(): Promise<number> {
95+
return getDirectorySize(
96+
path.join(next.testDir, '.next', 'cache', 'turbopack')
97+
)
98+
}
99+
69100
// Very flakey with Webpack enabled
70101
;(process.env.IS_TURBOPACK_TEST ? it : it.skip)(
71102
'should cache or not cache loaders',
@@ -95,7 +126,7 @@ for (const cacheEnabled of [false, true]) {
95126
expect(pagesTimestamp).toMatch(/Timestamp = \d+$/)
96127
await browser.close()
97128
}
98-
await restartCycle()
129+
const initialCacheSize = await restartCycle()
99130

100131
{
101132
const browser = await next.browser('/')
@@ -141,6 +172,18 @@ for (const cacheEnabled of [false, true]) {
141172
}
142173
await browser.close()
143174
}
175+
176+
if (cacheEnabled) {
177+
const finalCacheSize = await restartCycle()
178+
const increasePercent = (
179+
(finalCacheSize / initialCacheSize - 1) *
180+
100
181+
).toFixed(2)
182+
console.log(
183+
`Cache size: ${(initialCacheSize / 1024 / 1024).toFixed(2)} MB -> ${(finalCacheSize / 1024 / 1024).toFixed(2)} MB (${increasePercent}%)`
184+
)
185+
expect(finalCacheSize).toBeLessThanOrEqual(initialCacheSize * 1.1)
186+
}
144187
}
145188
)
146189

@@ -171,6 +214,7 @@ for (const cacheEnabled of [false, true]) {
171214
withChange(previous: () => Promise<void>): Promise<void>
172215
checkChanged(): Promise<void>
173216
fullInvalidation?: boolean
217+
largeCacheIncrease?: boolean
174218
}
175219
const POTENTIAL_CHANGES: Record<string, Change> = {
176220
'RSC change': {
@@ -199,6 +243,7 @@ for (const cacheEnabled of [false, true]) {
199243
}
200244
},
201245
checkChanged: makeTextCheck('/add-me', 'hello world'),
246+
largeCacheIncrease: true,
202247
},
203248
// TODO fix this case with Turbopack
204249
...(isTurbopack
@@ -265,11 +310,15 @@ for (const cacheEnabled of [false, true]) {
265310
`should allow to change files while stopped (${name})`,
266311
async () => {
267312
let fullInvalidation = !cacheEnabled
313+
let largeCacheIncrease = false
268314
for (const change of changes) {
269315
await change.checkInitial()
270316
if (change.fullInvalidation) {
271317
fullInvalidation = true
272318
}
319+
if (change.largeCacheIncrease) {
320+
largeCacheIncrease = true
321+
}
273322
}
274323

275324
let unchangedTimestamp: string
@@ -294,6 +343,7 @@ for (const cacheEnabled of [false, true]) {
294343
}
295344

296345
await stop()
346+
const initialCacheSize = await getCacheSize()
297347

298348
async function inner() {
299349
await start()
@@ -313,6 +363,21 @@ for (const cacheEnabled of [false, true]) {
313363
}
314364
await current()
315365

366+
if (cacheEnabled) {
367+
const finalCacheSize = await getCacheSize()
368+
const maxIncrease = largeCacheIncrease ? 1.5 : 1.1
369+
const increasePercent = (
370+
(finalCacheSize / initialCacheSize - 1) *
371+
100
372+
).toFixed(2)
373+
console.log(
374+
`Cache size (${name}): ${(initialCacheSize / 1024 / 1024).toFixed(2)} MB -> ${(finalCacheSize / 1024 / 1024).toFixed(2)} MB (${increasePercent}%)`
375+
)
376+
expect(finalCacheSize).toBeLessThanOrEqual(
377+
initialCacheSize * maxIncrease
378+
)
379+
}
380+
316381
await start()
317382
for (const change of changes) {
318383
await change.checkInitial()

0 commit comments

Comments
 (0)