Skip to content

fix: Hot reload's call-site cache is bounded by bytes instead of a count, so one run's assemblies stay cached - #3235

Merged
hatayama merged 4 commits into
feature/hot-reload-large-project-feedback-3from
fix/call-site-cache-byte-budget
Oct 7, 2026
Merged

hatayama merged 4 commits into
feature/hot-reload-large-project-feedback-3from
fix/call-site-cache-byte-budget

Conversation

@hatayama

@hatayama hatayama commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Hot reload's call-site scan keeps the assemblies it read in a cache between runs. The cache was capped at 64 entries, so in a project whose edited assembly is referenced by more assemblies than that, every run read the same assemblies again. The cache is now bounded by the dlls' bytes (256 MB by default), so all the assemblies one run touches stay cached for the next run.
  • When the end of a run still has to evict entries, a hot_reload_call_site_cache_evicted vibe entry records how many and how many bytes.

Why

  • In a large project trial, the caller scan took 2.0 to 2.4 s of every apply run. The edited assembly was referenced by 68 assemblies (42 MB). With a cap of 64 entries, every hold release evicted the same 9 assemblies (29 MB, the edited assembly among them), and the next run read all 9 again: the load count grew by 9 per warm run. The fingerprint checks took 1 to 3 ms per pass and were not the cause.
  • Dlls range from kilobytes to megabytes, so an entry count bounds neither memory nor what one run needs. The previous fix (8 to 64 entries) hit the same limit one size up.

Changes

  • HotReloadCompiledCallSiteCache takes a byte budget instead of a capacity and tracks the sum of the cached dlls' lengths. The sum is updated on add, on eviction, and when a stale entry is replaced.
  • Without a hold, a miss evicts least recently used entries until the new dll fits, stopping when the cache is empty, so a dll larger than the budget is still cached, alone.
  • When the outermost hold ends, least recently used entries are evicted until the cached bytes fit the budget, keeping at least one entry. The evicted count and bytes are recorded (LastHoldReleaseEvictedCount, LastHoldReleaseEvictedBytes) and, when non-zero, logged outside the cache lock with numbers only.
  • docs/vibe-logs.md describes the new vibe entry.
  • Tests: the fixtures size the cache with BudgetForEntries(n) (n times the fixture dll's length) instead of a count. Four tests are renamed from capacity to budget. New tests:
    • HoldEntriesForRun_Dispose_KeepsEveryEntryWithinBudget: a run within the budget evicts nothing, and the next run reads nothing again (the fix itself).
    • HoldEntriesForRun_Dispose_RecordsWhatItEvicted
    • HoldEntriesForRun_Dispose_KeepsTheLastEntryOverBudget
    • GetOrLoad_EntryLargerThanBudget_IsStillLoaded
    • GetOrLoad_ReplacedEntry_UpdatesCachedBytes
    • GetOrLoad_WithoutHold_StillEvictsBeforeAdding also checks the cached bytes.

Verification

  • Red first: with only the test changes, compile failed on the missing CachedBytes, LastHoldReleaseEvictedCount, and LastHoldReleaseEvictedBytes.
  • uloop compile: 0 errors.
  • uloop run-tests --filter-type regex --filter-value 'HotReloadCompiledCallSiteCacheTests': 25 passed, 0 failed.
  • uloop run-tests --filter-type regex --filter-value 'HotReloadCallSiteScannerTests|HotReloadOrchestratorTests|HotReloadOneShotCaller': 237 passed, 0 failed.
  • The vibe entry was confirmed in the development Editor with a 1-byte budget: one eviction logged with camelCase keys, the last entry kept over budget.
  • No DefaultCapacity, _capacity, OverCapacity, or DownToCapacity remains under Packages or Assets.
  • scripts/check-file-length.sh: no file exceeded the limit.
  • Mutations, applied to the committed code and reverted afterwards:
Mutation Result
m1: the end of a hold evicts down to 2 entries regardless of bytes Caught by HoldEntriesForRun_Dispose_KeepsEveryEntryWithinBudget
m2: eviction does not subtract the evicted dll's length Caught by 6 tests, including GetOrLoad_WithoutHold_StillEvictsBeforeAdding and HoldEntriesForRun_Dispose_RecordsWhatItEvicted
m3: the end of a hold does not record what it evicted Caught by HoldEntriesForRun_Dispose_RecordsWhatItEvicted
m4: replacing a stale entry does not subtract the old length Caught by GetOrLoad_ReplacedEntry_UpdatesCachedBytes
m5: the end of a hold may evict the last entry (Count > 1 to Count > 0) Caught by HoldEntriesForRun_Dispose_KeepsTheLastEntryOverBudget

This pull request targets an integration branch, so the pull request CI does not run on it; the checks above were run locally. The development project scans only a few assemblies per run, so the effect on the caller scan time is to be measured in the large project trial.

Not changed

  • The scan results and the hot reload response.
  • What HoldEntriesForRun means: nothing is evicted while a hold is open, and holds nest.
  • The dll fingerprint (length, write time, module version id).

A count cap of 64 let a run that scanned 68 assemblies evict 9 of them
at every hold release and read them again on the next run. The tests now
size the cache in bytes, pin that a run within the budget evicts nothing,
and pin the cached bytes across eviction, replacement, and an entry
larger than the budget.
A count cap of 64 let a run that scanned 68 assemblies (42 MB) evict
9 of them (29 MB, the edited assembly among them) at every hold release,
so every run read them again, about 2 s per run. The cache now keeps up to
256 MB of dll bytes, still keeps one entry larger than the budget, and
logs the evicted count and bytes when the end of a run evicts anything.
@coderabbitai

coderabbitai Bot commented Oct 7, 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: hatayama/unity-cli-loop/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 433e7c7e-ed71-4986-a643-140aaed6ccae
📥 Commits

Reviewing files that changed from the base of the PR and between ee6cc4b and 474e53c.

📒 Files selected for processing (4)
  • Assets/Tests/Editor/HotReload/HotReloadCompiledCallSiteCacheTests.cs
  • Packages/src/Editor/FirstPartyTools/HotReload/HotReloadCompiledCallSiteCache.cs
  • Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs
  • docs/vibe-logs.md

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


📝 Walkthrough

Walkthrough

The call-site cache now uses a 256 MiB DLL-byte budget instead of an entry-count limit. It tracks cached bytes, evicts least-recently-used entries, and reports eviction counts and byte totals when the outermost hold ends.

Changes

Call-site cache byte budgeting

Layer / File(s) Summary
Byte accounting and lookup eviction
Packages/src/Editor/FirstPartyTools/HotReload/HotReloadCompiledCallSiteCache.cs, Assets/Tests/Editor/HotReload/HotReloadCompiledCallSiteCacheTests.cs
The cache accepts a positive byte budget and tracks DLL lengths through replacement, clearing, and lookup eviction. Tests cover byte-based limits, oversized assemblies, stale-entry replacement, and cache accounting.
Hold-release eviction and reporting
Packages/src/Editor/FirstPartyTools/HotReload/HotReloadCompiledCallSiteCache.cs, Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs, Assets/Tests/Editor/HotReload/HotReloadCompiledCallSiteCacheTests.cs, docs/vibe-logs.md
Eviction remains deferred during nested holds. When the outermost hold ends, the cache evicts to the budget, records eviction metrics, and logs the eviction counts and byte totals. Tests cover hold-release behavior, and the log documentation describes the event fields.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 474e5

No actionable merge-blocking issue is established; the change is ready for normal merge checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 474e5

The change alters how long hot-reload data is retained, without an observed new privilege or external access path. The budget is not a hard memory limit, and some upstream-input and concurrent-lifetime behavior remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated effect is on retention and diagnostic activity within the owning Unity Editor process. Scoped caller inspection did not establish a new independently attackable service, tenant boundary, or privilege transition; complete upstream input reachability was not assessed.

Trust Boundaries and Controls

  • observed — The new eviction payload contains only evicted count, evicted bytes, cached bytes, and budget bytes. It does not serialize DLL paths, assembly identities, or retained module contents.

Resilience and Maintainability Implications

  • observed — Eviction and hold-depth updates complete before diagnostic logging begins, and file-save exceptions are caught by the existing logger. Normal filesystem logging failures therefore do not leave a run hold open or roll back completed eviction.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 3 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Title check ✅ Passed The title clearly and accurately summarizes the main change: the hot-reload call-site cache now uses a byte limit instead of an entry-count limit.
Description check ✅ Passed The description is directly related to the changeset and explains the byte budget, eviction behavior, logging, tests, and verification results.
Full details: Docstring Coverage

Explanation

Docstring coverage is 57.58% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 33 functions across 3 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · 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.

The hold release promises to keep at least one entry, but no test failed
when that guard was dropped.
@hatayama
hatayama merged commit d872b6d into feature/hot-reload-large-project-feedback-3 Oct 7, 2026
4 of 5 checks passed
@hatayama
hatayama deleted the fix/call-site-cache-byte-budget branch October 7, 2026 23:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant