Repository navigation
perf: Hot reload asks the compilation pipeline for the assembly list once per domain - #3240
Conversation
Hot reload asks CompilationPipeline.GetAssemblies() on every run, which takes hundreds of milliseconds on a project with several hundred assemblies. These tests describe a memo that asks once, keeps a non-empty answer until it is invalidated, and never keeps the empty answer Unity gives during a compile.
It asks Unity once and keeps the first non-empty answer until it is invalidated. An empty answer is not kept because GetAssemblies() returns nothing while a compile is in flight.
Input resolution, new-source membership checks, changed-file detection, the startup snapshot capture, and the call-site reference graph each called CompilationPipeline.GetAssemblies(), which costs 340-620 ms on a project with several hundred assemblies. They now share one memo that is dropped when a compile starts (a failed compile keeps the domain alive) and when the project changes (an import does not always compile).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (3)
📒 Files selected for processing (8)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughHot reload code now obtains compilation assemblies through a shared cache. The cache reuses non-empty results, retries empty results, supports name lookup, and invalidates on compilation start or project change. EditMode tests cover cache and lookup behavior. ChangesCompilation assembly access
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant HotReloadCallSiteScanner
participant HotReloadCompilationAssemblies
participant HotReloadCompilationAssemblyCache
participant CompilationPipeline
HotReloadCallSiteScanner->>HotReloadCompilationAssemblies: Current()
HotReloadCompilationAssemblies->>HotReloadCompilationAssemblyCache: Current()
HotReloadCompilationAssemblyCache->>CompilationPipeline: Fetch compilation assemblies
CompilationPipeline-->>HotReloadCompilationAssemblyCache: Compilation assemblies
HotReloadCompilationAssemblyCache-->>HotReloadCompilationAssemblies: Cached assembly list
HotReloadCompilationAssemblies-->>HotReloadCallSiteScanner: Assembly list
Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue is established; the change is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change preserves existing assembly-identity and membership controls without adding a new external entrypoint. The main uncertainty is whether shared assembly data remains fresh after compilation failures and imports, especially when a lookup occurs during compilation. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 3 | ❌ 1 | ❓ 1❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 lookup tests passed for a prefix match, a case-insensitive match, and a loop that skips the first assembly. Nothing pinned that the per-domain memo is shared or that its invalidation is subscribed to compile start and project change, so dropping either subscription went unnoticed.
731c2f7
into
feature/hot-reload-large-project-feedback-3
Summary
Why
CompilationPipeline.GetAssemblies(): once per input file while resolving inputs, once more when--filesis omitted, and again for new-source membership checks and the call-site reference graph.resolve_inputsstep (337-366 ms) was almost entirely that one call, and it was the largest share of the run's non-analysis time.Design
IReadOnlyList, so they can only read the shared array.GetAssemblies()returns nothing while a compile is in flight, and keeping that would hide every assembly for the rest of the domain.CompilationPipeline.compilationStarted(a failed compile keeps the domain alive) and onEditorApplication.projectChanged(an import does not always compile, for example in Play Mode with "Recompile After Finished Playing"). A domain reload clears it with the other statics.GetAssemblies()call now goes through the shared memo.Changes
HotReloadCompilationAssemblyCache(the memo, testable with an injected fetch) andHotReloadCompilationAssemblies(the per-domain instance and its two event handlers).FindCompilationAssemblyhelpers are gone.Verification
This pull request targets an integration branch, so the pull request CI does not run on it; the checks below were run locally in the Editor.
uloop run-tests --filter-type regexoverHotReloadCompilationAssemblyCacheTests|HotReloadCompilationAssembliesTests|HotReloadPatchTargetSupport|HotReloadNewSourceMembership|HotReloadChangedFileAggregatorTests|HotReloadSnapshotAssemblyEnumerationTests|HotReloadSourceSnapshot|HotReloadCallSiteScannerTests: 120 passed, 0 failed.HotReloadCompilationAssemblyCacheTests|HotReloadCompilationAssembliesTests:Current_EmptyAnswer_IsNotMemoized,Current_NonEmptyAfterEmpty_IsMemoized)Invalidate()does nothingCurrent_AfterInvalidate_FetchesAgainAndReturnsNewAnswer)Current()always fetchesCurrent_CalledTwice_FetchesOnceAndReturnsSameInstance,Current_NonEmptyAfterEmpty_IsMemoized,FindByName_KnownName_ReturnsAssemblyAndFetchesOnce)FindByNamematches by prefix (StartsWith)FindByName_SimilarNames_ReturnsExactMatchOnly)FindByNameignores caseFindByName_SimilarNames_ReturnsExactMatchOnly)FindByNamestarts at index 1FindByName_SimilarNames_ReturnsExactMatchOnly)compilationStartedsubscriptionStaticConstructor_SubscribesInvalidationToCompilationStartedAndProjectChanged)projectChangedsubscriptionStaticConstructor_SubscribesInvalidationToCompilationStartedAndProjectChanged)Current()callCurrent_CalledTwice_ReturnsSameInstance)resolve_inputsfromhot_reload_timing_detail, three runs in a row on the same file:compilationStarted: after a compile that failed on a syntax error (domain kept), the next run took 249 ms and the one after 4 ms. Without the handler nothing drops the memo, so that first run would also be fast (not run as a mutation).projectChanged: after importing a non-script asset withAssetDatabase.Refresh()(no compile logged), the next run took 61 ms, then 1 ms. After deleting it and refreshing again: 474 ms, then 1 ms. A controlexecute-dynamic-codewithout a refresh left the next run at 0 ms.Not covered
compilationStartedorprojectChanged; that the events fire when expected (a failed compile, a non-script import) is covered only by the Editor checks above.sibling_detectand other remaining run costs are separate work.