Repository navigation
fix: Hot reload's call-site cache is bounded by bytes instead of a count, so one run's assemblies stay cached - #3235
Conversation
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.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe 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. ChangesCall-site cache byte budgeting
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue is established; the change is ready for normal merge checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to 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 Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
The hold release promises to keep at least one entry, but no test failed when that guard was dropped.
d872b6d
into
feature/hot-reload-large-project-feedback-3
Summary
hot_reload_call_site_cache_evictedvibe entry records how many and how many bytes.Why
Changes
HotReloadCompiledCallSiteCachetakes 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.LastHoldReleaseEvictedCount,LastHoldReleaseEvictedBytes) and, when non-zero, logged outside the cache lock with numbers only.docs/vibe-logs.mddescribes the new vibe entry.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_RecordsWhatItEvictedHoldEntriesForRun_Dispose_KeepsTheLastEntryOverBudgetGetOrLoad_EntryLargerThanBudget_IsStillLoadedGetOrLoad_ReplacedEntry_UpdatesCachedBytesGetOrLoad_WithoutHold_StillEvictsBeforeAddingalso checks the cached bytes.Verification
CachedBytes,LastHoldReleaseEvictedCount, andLastHoldReleaseEvictedBytes.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.DefaultCapacity,_capacity,OverCapacity, orDownToCapacityremains underPackagesorAssets.scripts/check-file-length.sh: no file exceeded the limit.HoldEntriesForRun_Dispose_KeepsEveryEntryWithinBudgetGetOrLoad_WithoutHold_StillEvictsBeforeAddingandHoldEntriesForRun_Dispose_RecordsWhatItEvictedHoldEntriesForRun_Dispose_RecordsWhatItEvictedGetOrLoad_ReplacedEntry_UpdatesCachedBytesCount > 1toCount > 0)HoldEntriesForRun_Dispose_KeepsTheLastEntryOverBudgetThis 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
HoldEntriesForRunmeans: nothing is evicted while a hold is open, and holds nest.