Repository navigation
perf: Hot reload no longer re-reads the compiled assembly for every method it patches - #3199
Merged
hatayama merged 3 commits intoOct 7, 2026
Conversation
Entry resolution and the unchanged-patch peel now each create one matcher per group and pass its Resolve on as a delegate. The matcher still reads the compiled image on every call, so the new tests for one read per image, answers after the file is deleted or replaced, the per-home loaded-assembly check, the loader contract and disposal fail until the next commit.
Every entry used to read the whole compiled assembly again and rebuild its type table, so the patch stage grew with the number of live patches in a file. The matcher now keeps each image it read successfully until it is disposed at the end of the group, while a missing or unreadable image is still looked for again by the next entry and the Mvid guard still runs for every entry. A disposed matcher refuses to resolve, and a loader that returns no image is reported instead of failing on a null read.
Existing-method entries are now resolved through the delegate the caller passes, so a test counts the calls to make sure the group's matcher is the one that answers. The peel's comments now say that only the first row resolved against an image reads it, which the name prefilter still spares along with the per-row matching.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (8)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughHotReloadMethodMatcher is now a disposable instance that caches successfully loaded assembly images by DLL path. Entry preparation, resolution, and patch reversion pass its resolver through existing-method matching. Tests cover cache behavior, retries, disposal, and updated call sites. ChangesHot reload method resolution
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant PrepareGroup
participant PrepareFile
participant ResolveEntries
participant HotReloadMethodMatcher
PrepareGroup->>HotReloadMethodMatcher: CreateReadingFromDisk
PrepareGroup->>PrepareFile: Pass matcher.Resolve
PrepareFile->>ResolveEntries: Pass resolveMethod
ResolveEntries->>HotReloadMethodMatcher: Resolve existing method
HotReloadMethodMatcher-->>ResolveEntries: Return match result
Merge Risk: ⚪ Minimal · up to This change speeds up hot-reload patching by reusing each compiled assembly read within a group. No merge-blocking risk was found in the supplied evidence. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The cache has an explicit, short lifetime, and every method still undergoes the existing assembly-identity check. No introduced security weakness was identified in the reviewed paths, although broader security coverage remains incomplete. 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)
✨ Finishing Touches 💡 1📝 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 |
hatayama
merged commit Oct 7, 2026
7950674
into
feature/hot-reload-large-project-feedback
5 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
User Impact
Measured on a fast Editor, right after a forced compile, with a temporary file of 200 instance methods (not committed). Every round finished within 13.1 s of the end of the compile, because a background Editor on macOS slows down from about 25 s after a compile and multiplies every phase (a separate issue).
PatchedTotalwas 200 in every run.Changes
HotReloadMethodMatcheris now an instance made for one run (IDisposable). It keeps each compiled image it read successfully, keyed by path, and resolves the rest of the run's methods from it. A missing image or a failed read is not kept, so the next method looks again, as before.usingand outside the file loop: group preparation (resolution) and the unchanged-method peel. Entry resolution receives the matcher'sResolveas a delegate, as the peel already did, so it does not own the matcher's lifetime. No matcher is kept in a field or a static, so no read outlives its run.ObjectDisposedException. A loader that returns no image throwsInvalidOperationException, and nothing is kept.Resolveis removed; tests create a matcher for each test.Input space
The axes come from the branches of
Resolve: whether this matcher has already read the path, the file's state at the call, the matcher's state, whether the type and the method are found, the Mvid against the loaded assembly, and the number of paths in one run.Tests, in
HotReloadMethodMatcherTestsunless noted:Resolve_SeveralEntriesOfOneImage_ReadsTheImageOnce(one of the three is a nested type)Resolve_TwoImages_ReadsEachOnceAndAnswersFromItsOwnResolve_MissingImage_IsLookedForAgainOnTheNextEntryResolve_ImageThatFailedToRead_IsReadAgainOnTheNextEntryResolve_ImageDeletedAfterTheFirstRead_StillAnswersFromThatReadResolve_ImageReplacedAfterTheFirstRead_SameMatcherKeepsItsRead_AndANewMatcherReadsTheNewImageResolve_AfterDispose_ThrowsResolve_UnknownType_ReturnsTypeNotFoundResolve_AssemblyNotLoaded_IsReportedForEveryEntry_WithOneReadHotReloadEntryResolutionTests.ResolveEntries_ResolvesEachExistingMethodEntryThroughTheGivenResolverResolve_SameImageUnderTwoHomes_ChecksTheLoadedAssemblyForEachHomeResolve_LoaderReturningNull_Throws_AndTheImageIsReadAgainOnTheNextEntryResolve_*(5), T1CompiledAssemblyNotFoundCompiledAssemblyNotFoundTypeNotFound, orStaleAssemblyif the Mvid differs)DisposeObjectDisposedExceptionTypeNotFound/MethodNotFoundResolve_ParameterTypeMismatch_ReturnsMethodNotFoundStaleAssembly/AssemblyNotLoaded, per methodResolveLoadedMethod_MvidMismatch_ReturnsStaleAssembly, T9AssemblyNotLoadedInvalidOperationException; nothing keptEntry resolution resolves each existing method through the resolver it is given (T10).
Why C7 and C8 may change: the base failure there protects one thing: a token read from an image that differs from the loaded assembly is never applied. Head takes the token and the Mvid from the same read and compares that Mvid with the loaded assembly for every method, so this still holds. The DLL on disk changes during a run (about a second from the first method to the last) only when a compile runs, and the domain reload after it removes every patch.
Resolution and the unchanged-method peel use separate matchers, so a run that does both reads the image twice, once each. If the image is replaced between the two, the peel skips the row with
StaleAssembly, as before.Where the matcher is created (outside the file loop) is not pinned by a test, so that group preparation needs no loader hook; please check it in the diff.
Mutations
Each mutation was applied to the committed tree,
HotReloadMethodMatcherTestsandHotReloadEntryResolutionTestswere run (26 tests), and the file was restored withgit checkout. No mutation survived.NullReferenceException)ResolveDisposedoes not mark the matcher disposedNullReferenceException)Verification (local)
Pull requests to the integration branch do not trigger the main PR CI, so these checks were run locally.
Resolvestill reading on every call): 17 tests inHotReloadMethodMatcherTests, 8 failed as predicted: T1 (3 reads), T2 (4 reads), T5 (CompiledAssemblyNotFound), T6 (TypeNotFound), T7 (no exception), T9 (2 reads), T11 (2 reads), T12 (NullReferenceException). T3, T4, T8 and the 6 existing tests passed.HotReloadMethodMatcherTestsandHotReloadEntryResolutionTests26/26.HotReloadMethodMatcherTests,HotReloadEntryResolutionTests,HotReloadUnchangedPatchPeelTests,HotReloadSpikeS4ArtifactPatchTests,HotReloadGroupProcessorTestsandHotReloadGroupProcessorLeaveOutTests102/102;HotReloadOrchestratorTests184/184;HotReloadIntroducedTypeActivationTests26/26;HotReloadIntroducedTypeResponseTests17/17.scripts/check-code-complexity.sh(fail on exceeded): 0 Go issues, no CA1502 finding above 15.scripts/check-file-length.sh(fail on exceeded): no file over 500 SLOC.scripts/check-dead-code.shwith the CI arguments: exit 0, PublicCandidate 36 (limit 37), and no symbol of the matcher is listed.uloop compile-check --all: 0 errors; the 7 warnings are all in existing test fixtures, none in a changed file.HotReloadMethodMatcher.Resolve(remains.CreateReadingFromDiskis called in the two production places and in tests only.int MNNN() { return N; }.--revert-all. Every hot reload used--compile-on-skip off.Not covered
PatchedTotal200 above), so the number of methods resolved per run is unchanged. Reducing that is a separate candidate.Resolvethat read a DLL with Cecil (the patch-target support and the compiled call-site cache) are unchanged.