Skip to content

dev server: clear the stale bit of a module that is bundled past the end of the stale bit set - #45455

Open
robobun wants to merge 1 commit into
mainfrom
robobun/42b18a5f/dev-server-stale-bit-on-insert
Open

robobun wants to merge 1 commit into
mainfrom
robobun/42b18a5f/dev-server-stale-bit-on-insert

Conversation

@robobun

@robobun robobun commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator

Related to #29418

Problem

  • A page with a stylesheet and over 510 modules reloads in full on the first edit of a self-accepting module: [Bun] Hot update was not accepted because it or its importers do not call import.meta.hot.accept.
  • receive_chunk (src/runtime/bake/dev_server/incremental_graph.rs:596) clears a file's stale bit only if stale_files covers it. A later insert_empty or insert_failure in that bundle fills those files with true.

Fix

  • receive_chunk grows the set to exactly the file it receives: resize(index + 1, false). A later grow writes only the bits after it.
  • The grow is exact because insert_empty and insert_css_file_on_server read what a 512-bit step fills.
  • Verified: three new tests in test/bake/dev/hot.test.ts fail on main. Also test/bake/ and the html tests in test/js/bun/http/.
  • Self-reviewed: 5 concerns raised, 5 addressed, one in part (no client-graph copy of the failed-import test).

Background

Downsides

Notes

Why not the two lines of #44340 (ensure_stale_bit_capacity, then write the bit)

Who reaches it

Measurements (main 5f5799e against the same commit with this change, linux x64. The page has 600 modules and a plain stylesheet)

  • ParseTasks after one edit of a.ts, 600 modules + stylesheet: 90 on main, 0 on PR (controls, 600 modules without the stylesheet and 400 with it: 0 and 0, unchanged). Debug builds, BUN_DEBUG_Bundle=1.
  • modules/ ids in the hot chunk after editing a.ts: 91 on main (page reloads), 0 on PR.
  • First load of 600 modules + stylesheet: resize calls on stale_files 6 -> 97, bit-set reallocs 2 -> 4 (gdb breakpoint counts, debug builds, main vs PR. Same with no stylesheet: 5 -> 96, 2 -> 4. 400 modules: 6 -> 6 and 1 -> 1).
  • In-range stale-bit step of receive_chunk, counted from the statement before it to the statement after it: client copy 27 -> 27 instructions, server copy (inlined into finalize_bundle) 32 -> 36, conditional branches 3 -> 3 and 4 -> 4, 0 calls, 0 allocations (objdump, release builds, main vs PR).
  • File received past the set: 3 -> 69 instructions and 1 resize call, once per file (gdb stepi, PR release build). The two files that start a new 64-bit word take 184 and 266, with one realloc each.
  • Release binary: +256 bytes text (receive_chunk client 2070 -> 2129 bytes, finalize_bundle 25241 -> 24982 bytes. size and nm, main vs PR). The stripped file is 106178312 bytes in both.
  • css onLoad runs for a css file that a framework route imports, over a load and two route edits: main 1, 2, 3 and PR 1, 2, 3 (identical, that bit is not touched). With 600 server modules: main 1, 1, 1 and PR 1, 1, 1.
  • Failed import, no stylesheet, 600 modules: 92 unchanged modules parsed again on main, 0 on PR.

The panic in Downsides

What this PR does not change

Tests

  • bun bd test test/bake/dev/hot.test.ts: 14 pass. The three new tests on a debug build of main:
    • Client exited while applying hot update: Unexpected reload
    • {"marker":"a2","loads":{"client":0,"server":691}}, expected "server":600
    • {"loads":{"client":2,"server":1}}, expected "client":1
  • The first test has a stylesheet with an @import, so it does not depend on the insert_empty call in Assets::replace_path.
  • The second test is the server graph and the insert_failure grower. There is no client-graph copy of it: it is a third 600-module run, and the grower and the new branch are the same generic code for both graphs.
  • Debug build, this branch: every file in test/bake/ and test/bake/dev/ passes. production.test.ts passes with --timeout 90000 (the timeout of the CI runner). With the default 5000 ms it times out on a debug build, on main too.
  • Also run: bun-serve-html.test.ts, bun-serve-html-hot-reload-drop.test.ts, bun-serve-html-405.test.ts, test/regression/issue/42989.test.ts, test/regression/issue/21792.test.ts.
  • The new tests take 6, 6 and 2 s on a debug build. 600 modules is what it takes to pass a 512-bit step.

…end of the stale bit set

`receive_chunk` cleared the stale bit of a file only when `stale_files`
already covered the index of the file. A file past the end got no bit.
A later `insert_empty` or `insert_failure` in the same bundle grew the
set with `true` over that file. The next bundle then parsed the file
again and sent it in the hot update.

`receive_chunk` now grows the set to exactly the file it receives, with
the bit cleared. The grow is exact because `insert_empty` and
`insert_css_file_on_server` read the bits that a 512-bit step fills.
@robobun

robobun commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:46 PM PT - Oct 15th, 2026

✅ @robobun, your commit 3955b3b45cd7cdb5825a4d341c5e17c3e64cada3 passed in Build #126388! 🎉


🧪   To try this PR locally:

bunx bun-pr 45455

That installs a local version of the PR into your bun-45455 executable, so you can run:

bun-45455 --bun

@robobun

robobun commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduced on a debug build of main (5f5799e).

To reproduce, check out this branch, restore src/ from main, and run bun bd test test/bake/dev/hot.test.ts. The three new tests fail:

  • Client exited while applying hot update: Unexpected reload
  • {"marker":"a2","loads":{"client":0,"server":691}}, expected "server":600
  • {"loads":{"client":2,"server":1}}, expected "client":1

With the change in src/, all 14 tests of the file pass.

PR: #45455

@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 5d6b4452-85cb-4ad4-b0f8-66dca7987e48
📥 Commits

Reviewing files that changed from the base of the PR and between af5743d and 3955b3b.

📒 Files selected for processing: 2
  • src/runtime/bake/dev_server/incremental_graph.rs
  • test/bake/dev/hot.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

Changes

The incremental graph now resizes its stale-file bitset when it receives a file index beyond the current length. Regression tests cover hot updates and incremental bundling with more than 512 modules.

Incremental graph tracking

Layer / File(s) Summary
Resize stale-file tracking for new chunks
src/runtime/bake/dev_server/incremental_graph.rs, test/bake/dev/hot.test.ts
The receive path grows the bitset for out-of-range file indices. Regression tests check hot-update chunk contents and confirm unchanged modules are not loaded again in the tested scenarios.

Suggested reviewers: dylan-conway

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 3955b

No actionable merge-blocking issue is identified; merge after normal checks.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies the main change: clearing stale bits for modules received beyond the current stale bit set. It is specific and directly related to the changes.
Description check Passed The description explains the problem, fix, limitations, testing, and verification results. Although it does not use the template headings exactly, it covers both required topics in sufficient detail.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread src/runtime/bake/dev_server/incremental_graph.rs
Comment thread src/runtime/bake/dev_server/incremental_graph.rs

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant