diff --git a/src/runtime/bake/dev_server/finalize.rs b/src/runtime/bake/dev_server/finalize.rs index c6d1fffee5e3..62362fb0fd0f 100644 --- a/src/runtime/bake/dev_server/finalize.rs +++ b/src/runtime/bake/dev_server/finalize.rs @@ -625,9 +625,6 @@ fn finalize_bundle_inner( } dev.index_failures()?; - dev.client_graph.ensure_stale_bit_capacity(false)?; - dev.server_graph.ensure_stale_bit_capacity(false)?; - dev.generation = dev.generation.wrapping_add(1); if Environment::ENABLE_LOGS { debug_log!( diff --git a/src/runtime/bake/dev_server/incremental_graph.rs b/src/runtime/bake/dev_server/incremental_graph.rs index 37b3b7e0f176..63628244890c 100644 --- a/src/runtime/bake/dev_server/incremental_graph.rs +++ b/src/runtime/bake/dev_server/incremental_graph.rs @@ -215,6 +215,16 @@ enum FreeCssMode { IgnoreCss, } +/// What `add_node` does to the file's bit in `stale_files`. +#[derive(Copy, Clone, Eq, PartialEq)] +enum StaleBit { + Set, + /// Set the bit of a new file. An existing file keeps its bit. + SetIfNew, + Clear, + Keep, +} + /// `Route` on the server must also be registered in `DevServer::route_lookup`. #[derive(Copy, Clone, Eq, PartialEq)] pub(crate) enum RouteKind { @@ -293,6 +303,7 @@ pub(crate) struct IncrementalGraph { /// Keys are absolute paths for the "file" namespace (owned). Index = `FileIndex`. pub(crate) bundled_files: StringArrayHashMap, /// Parallel to `bundled_files`; bit set = file is stale and must rebundle. + /// Every file has a bit. Only `add_node` grows it. pub(crate) stale_files: DynamicBitSetUnmanaged, /// Parallel to `bundled_files`; the heads of each file's lists in `edges`. pub(crate) edge_lists: Vec, @@ -448,17 +459,35 @@ impl IncrementalGraph { self.current_chunk_parts.clear(); } - /// Grows `stale_files` to cover all currently-known files, filling new - /// bits with `are_new_files_stale`. - pub(crate) fn ensure_stale_bit_capacity( + /// The one place that adds a file to `bundled_files`, `edge_lists` and + /// `stale_files`. Returns the index and whether the file already existed. + fn add_node( &mut self, - are_new_files_stale: bool, - ) -> Result<(), bun_alloc::AllocError> { - let want = self.bundled_files.count().max(self.stale_files.bit_length); - // Align forward to 8 usize words (8*64 bits). - const STEP: usize = core::mem::size_of::() * 8 * 8; - let aligned = want.div_ceil(STEP) * STEP; - self.stale_files.resize(aligned, are_new_files_stale) + key: &[u8], + stale: StaleBit, + ) -> Result<(usize, bool), bun_alloc::AllocError> { + let gop = self.bundled_files.get_or_put(key)?; + let idx = gop.index; + let found_existing = gop.found_existing; + if !found_existing { + *gop.key_ptr = Box::<[u8]>::from(key); + self.edge_lists.push(EdgeLists::default()); + if idx >= self.stale_files.bit_length { + // 512-bit steps. The ">512 files" tests in test/bake/dev depend on it. + const STEP: usize = core::mem::size_of::() * 8 * 8; + self.stale_files + .resize((idx + 1).div_ceil(STEP) * STEP, false)?; + } + } + match stale { + StaleBit::Set => self.stale_files.set(idx), + StaleBit::SetIfNew if !found_existing => self.stale_files.set(idx), + StaleBit::Clear => self.stale_files.unset(idx), + StaleBit::SetIfNew | StaleBit::Keep => {} + } + debug_assert!(self.stale_files.bit_length >= self.bundled_files.count()); + debug_assert_eq!(self.edge_lists.len(), self.bundled_files.count()); + Ok((idx, found_existing)) } /// `IncrementalGraph(side).freeFileContent` (client only). @@ -600,20 +629,8 @@ impl IncrementalGraph { } } - let gop = self.bundled_files.get_or_put(key)?; - let file_index = FileIndex::::init(gop.index as u32); - let found_existing = gop.found_existing; - if !found_existing { - *gop.key_ptr = Box::<[u8]>::from(key); - } - // Note: drop `gop` borrow before pushing to other Vecs / re-borrowing. - if !found_existing { - self.edge_lists.push(EdgeLists::default()); - } - - if self.stale_files.bit_length > file_index.get() as usize { - self.stale_files.unset(file_index.get() as usize); - } + let (idx, found_existing) = self.add_node(key, StaleBit::Clear)?; + let file_index = FileIndex::::init(idx as u32); *ctx.get_cached_index(SIDE, index) = CachedFileIndex::from(Some::>(file_index)); @@ -1362,21 +1379,10 @@ impl IncrementalGraph { Side::Client => graph == bake::Graph::Client, Side::Server => graph != bake::Graph::Client, }); - let gop = self.bundled_files.get_or_put(abs_path)?; - let idx = gop.index; - let found_existing = gop.found_existing; - if found_existing { - if matches!(SIDE, Side::Server) && route == RouteKind::Route { - gop.value_ptr.is_route = true; - } - } else { - *gop.key_ptr = Box::<[u8]>::from(abs_path); + let (idx, found_existing) = self.add_node(abs_path, StaleBit::Set)?; + if found_existing && matches!(SIDE, Side::Server) && route == RouteKind::Route { + self.bundled_files.values_mut()[idx].is_route = true; } - if !found_existing { - self.edge_lists.push(EdgeLists::default()); - } - self.ensure_stale_bit_capacity(true)?; - self.stale_files.set(idx); match SIDE { Side::Client => { @@ -1435,12 +1441,9 @@ impl IncrementalGraph { abs_path: &[u8], kind: FileKind, ) -> Result, bun_alloc::AllocError> { - let gop = self.bundled_files.get_or_put(abs_path)?; - let idx = gop.index; - let found_existing = gop.found_existing; + let (idx, found_existing) = self.add_node(abs_path, StaleBit::SetIfNew)?; if !found_existing { - *gop.key_ptr = Box::<[u8]>::from(abs_path); - *gop.value_ptr = match SIDE { + self.bundled_files.values_mut()[idx] = match SIDE { Side::Client => File { kind, content: match kind { @@ -1457,14 +1460,7 @@ impl IncrementalGraph { }, }; } - // Capture the interned-key fat ptr now so the `gop` borrow on - // `bundled_files` ends before `ensure_stale_bit_capacity` reborrows. - let key = bun_ptr::RawSlice::new(&**gop.key_ptr); - if !found_existing { - self.edge_lists.push(EdgeLists::default()); - self.ensure_stale_bit_capacity(true)?; - self.stale_files.set(idx); - } + let key = bun_ptr::RawSlice::new(&*self.bundled_files.keys()[idx]); Ok(InsertEmptyResult { index: FileIndex::init(idx as u32), key, @@ -1481,19 +1477,12 @@ impl IncrementalGraph { abs_path: &[u8], ) -> Result<(), crate::Error> { debug_assert!(matches!(SIDE, Side::Server)); - let gop = self.bundled_files.get_or_put(abs_path)?; - let file_index = FileIndex::::init(gop.index as u32); - let found_existing = gop.found_existing; - if !found_existing { - *gop.key_ptr = Box::<[u8]>::from(abs_path); - } - *gop.value_ptr = File { + let (idx, _) = self.add_node(abs_path, StaleBit::Keep)?; + let file_index = FileIndex::::init(idx as u32); + self.bundled_files.values_mut()[idx] = File { kind: FileKind::Css, ..Default::default() }; - if !found_existing { - self.edge_lists.push(EdgeLists::default()); - } *ctx.get_cached_index(Side::Server, index) = CachedFileIndex::from(Some::>(file_index)); Ok(()) @@ -1517,20 +1506,14 @@ impl IncrementalGraph { ) -> Result<(), bun_alloc::AllocError> { let (idx, found_existing, parsed_for) = match key { InsertFailureKey::AbsPath(abs_path, graph) => { - let gop = self.bundled_files.get_or_put(abs_path)?; - if !gop.found_existing { - *gop.key_ptr = Box::<[u8]>::from(abs_path); - } - let (i, fe) = (gop.index, gop.found_existing); - if !fe { - self.edge_lists.push(EdgeLists::default()); - } + let (i, fe) = self.add_node(abs_path, StaleBit::Set)?; (i, fe, Some(graph)) } - InsertFailureKey::Index(i) => (i as usize, true, None), + InsertFailureKey::Index(i) => { + self.stale_files.set(i as usize); + (i as usize, true, None) + } }; - self.ensure_stale_bit_capacity(true)?; - self.stale_files.set(idx); match SIDE { Side::Client => { diff --git a/src/runtime/bake/dev_server/lifecycle.rs b/src/runtime/bake/dev_server/lifecycle.rs index c27b934f6b22..dbe0b86fa35a 100644 --- a/src/runtime/bake/dev_server/lifecycle.rs +++ b/src/runtime/bake/dev_server/lifecycle.rs @@ -787,9 +787,6 @@ impl DevServer { route_lookup: &mut self.route_lookup, }), )?; - - self.server_graph.ensure_stale_bit_capacity(true)?; - self.client_graph.ensure_stale_bit_capacity(true)?; Ok(()) } diff --git a/test/bake/dev/css.test.ts b/test/bake/dev/css.test.ts index 9ce744ae8636..36e4d2d8acb0 100644 --- a/test/bake/dev/css.test.ts +++ b/test/bake/dev/css.test.ts @@ -818,3 +818,47 @@ function extractCssUrl(backgroundImage: string): string { } return url[2]; } + +// The stale bitset grows in steps of 512 bits. A css asset registered during +// the first bundle used to grow it with the new bits set, which marked every +// already received module past index 512 as stale. The next hot update then +// resent those modules even though they did not change. +devTest("first hot update does not resend unchanged imports in a graph with more than 512 files and css", { + files: (() => { + const count = 700; + const files: Record = { + "index.html": emptyHtmlFile({ scripts: ["main.ts"] }), + "main.ts": ` + import "./style.css"; + import { total } from "./gen/index.ts"; + console.log("total " + total); + import.meta.hot.accept(); + `, + "style.css": `body { margin: 0; }`, + "gen/h.ts": `export const h = 1;`, + "gen/last.ts": `import { h } from "./h.ts";\nexport const last = h; // v0\n`, + }; + let imports = `import { last } from "./last.ts";\n`; + const sum = ["last"]; + for (let i = 0; i < count; i++) { + files[`gen/g${i}.ts`] = `export const g${i} = ${i};\n`; + imports += `import { g${i} } from "./g${i}.ts";\n`; + sum.push(`g${i}`); + } + files["gen/index.ts"] = imports + `export const total = ${sum.join(" + ")};\n`; + return files; + })(), + async test(dev) { + await using c = await dev.client("/", { storeHotChunks: true }); + const total = "total " + (1 + (699 * 700) / 2); + await c.expectMessage(total); + + for (const v of ["v1", "v2"]) { + await dev.write("gen/last.ts", `import { h } from "./h.ts";\nexport const last = h; // ${v}\n`); + const chunk = await c.getMostRecentHmrChunk(); + const modules = [...chunk.matchAll(/"([^"]+\.ts)"\s*:/g)].map(m => m[1]); + expect(modules).toEqual(["gen/last.ts"]); + await c.expectMessage(total); + } + }, +}); diff --git a/test/bake/dev/hot.test.ts b/test/bake/dev/hot.test.ts index f0f230bce840..12c346989b26 100644 --- a/test/bake/dev/hot.test.ts +++ b/test/bake/dev/hot.test.ts @@ -1,7 +1,7 @@ // Hot tests ensure that the `import.meta.hot` interface is functional import { expect } from "bun:test"; import { renameSync, unlinkSync, writeFileSync } from "node:fs"; -import { devTest, emptyHtmlFile } from "../bake-harness"; +import { devTest, emptyHtmlFile, minimalFramework } from "../bake-harness"; devTest("import.meta.hot.accept basic", { files: { @@ -691,3 +691,121 @@ devTest("dev.write resolves only after the new module body has run", { expect(await c.js`globalThis.marker`).toBe("updated"); }, }); + +// The stale bitset of an incremental graph grows in steps of 512 files. In the +// tests below the first bundle adds files past the end of the set, and something +// else in that bundle grows the set. The bundled modules must not be bundled again. +const modulesPastOneBitsetStep = Array.from({ length: 600 }, (_, i) => `m${i}`); +const moduleFilesPastOneBitsetStep = Object.fromEntries( + modulesPastOneBitsetStep.map(name => [`modules/${name}.ts`, `export const ${name} = "${name}";`]), +); +const importModulesPastOneBitsetStep = (from: string) => + modulesPastOneBitsetStep.map(name => `import "${from}/modules/${name}";`).join("\n"); +// `onLoad` runs once for each file the dev server parses, and not for a file it has cached. +const countLoadsBySide = (filter: string) => ` + export default [ + { + name: "count-loads-by-side", + setup(build) { + build.onLoad({ filter: /${filter}/ }, args => { + const loads = (globalThis.moduleLoads ??= { client: 0, server: 0 }); + loads[args.side]++; + }); + }, + }, + ]; +`; +const countModuleLoads = countLoadsBySide("[\\\\/]modules[\\\\/]m\\d+\\.ts$"); +devTest("repairing a failed import does not parse the unchanged modules of a route again (over 512 modules)", { + framework: minimalFramework, + pluginFile: countModuleLoads, + files: { + "routes/index.ts": [ + importModulesPastOneBitsetStep(".."), + `import { marker } from "../a";`, + `export default function (req, meta) {`, + ` return Response.json({ marker, loads: globalThis.moduleLoads });`, + `}`, + ].join("\n"), + // The failed import is recorded after the modules are bundled. It grows the bitset of the server graph. + "a.ts": [importModulesPastOneBitsetStep("."), `import "./missing";`, `export const marker = "a1";`].join("\n"), + ...moduleFilesPastOneBitsetStep, + }, + async test(dev) { + expect((await dev.fetch("/")).status).toBe(500); + + await dev.write("a.ts", [importModulesPastOneBitsetStep("."), `export const marker = "a2";`].join("\n")); + await dev.fetch("/").equals({ marker: "a2", loads: { client: 0, server: modulesPastOneBitsetStep.length } }); + }, +}); +// The bitset of the client graph is empty until the first bundle when the framework has no client entry point. +devTest("editing a client component does not parse its unchanged import again in a route with a stylesheet", { + framework: minimalFramework, + pluginFile: countModuleLoads, + files: { + "routes/index.ts": ` + import "../style.css"; + import "../Component"; + + export default function (req, meta) { + return Response.json({ loads: globalThis.moduleLoads }); + } + `, + "Component.ts": ` + "use client"; + import { m0 } from "./modules/m0"; + export const marker = "c1" + m0; + `, + "modules/m0.ts": ` + export const m0 = "m0"; + `, + "style.css": ` + body { color: red; } + `, + }, + async test(dev) { + // The first bundle parses "modules/m0.ts" once for each graph. + await dev.fetch("/").equals({ loads: { client: 1, server: 1 } }); + + await dev.write( + "Component.ts", + ` + "use client"; + import { m0 } from "./modules/m0"; + export const marker = "c2" + m0; + `, + ); + await dev.fetch("/").equals({ loads: { client: 1, server: 1 } }); + }, +}); +// A css file that a server route imports is a node of the server graph. It is +// created after the css is bundled, so it starts out cached. +devTest("editing a route does not parse its unchanged stylesheet again on the server", { + framework: minimalFramework, + pluginFile: countLoadsBySide("\\.css$"), + files: { + "routes/index.ts": ` + import "../style.css"; + export default function (req, meta) { + return Response.json({ marker: "r1", loads: globalThis.moduleLoads }); + } + `, + "style.css": ` + body { color: red; } + `, + }, + async test(dev) { + await dev.fetch("/").equals({ marker: "r1", loads: { client: 0, server: 1 } }); + + await dev.write( + "routes/index.ts", + ` + import "../style.css"; + export default function (req, meta) { + return Response.json({ marker: "r2", loads: globalThis.moduleLoads }); + } + `, + ); + await dev.fetch("/").equals({ marker: "r2", loads: { client: 0, server: 1 } }); + }, +});