Repository navigation
perf: Hot reload's source snapshot capture no longer asks the Package Manager for each assembly and source - #3282
Merged
hatayama merged 4 commits intoOct 11, 2026
Conversation
…e Package Manager inside it The capture now takes its project root, compile start, package roots and assembly list as values read on the main thread, and classifies each assembly and names each suspect source from those package roots instead of asking the Package Manager per assembly and per file. This is the first step toward running the capture off the main thread; it still runs synchronously here. - HotReloadSnapshotCaptureInputs.ReadOnMainThread reads the values; the assembly list still comes from GetAssemblies until the PDB listing lands. - The PDB comparison is split so the capture can call the stage that takes an already-built lookup path; the existing entry keeps asking the Package Manager for the other callers. - The folder-boundary match is shared with the PDB source map. - Two tests cross-check the new answers against the Package Manager on every assembly and source of this project.
…able No capture test covered the immutable skip, since a temporary root registers no package; the classification now arrives as a value, so a test can hand it in directly.
…the Package Manager per assembly The capture now classifies assemblies from package roots read once with its inputs, so packageLookupMs is close to 0, and totalMs no longer covers reading the compile start and the package roots, which captureMs still does.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No identified issue currently blocks merging the hot-reload snapshot change after normal checks. Security Architecture Review
Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 77.46% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 19 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 Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Title check The title clearly and concisely describes the main performance change: removing per-assembly and per-source Package Manager queries during hot-reload snapshot capture. Description check The description is directly related to the changeset. It explains the performance change, implementation details, user impact, verification results, and scope limitations.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 77.46% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 19 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
-
-
📝 Generate docstrings 💡
-
-
-
- Commit to this branch
- Create a new PR
-
-
🧪 Generate unit tests (beta)
-
-
- 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.
Comment @coderabbitai help to get the list of available commands.
… behind its package path Every test that built the check from a real dll and PDB used Assets sources, which the lookup returns unchanged, so passing the asset path or ignoring the package roots went unnoticed. The new test checks an embedded package's source, whose PDB records it under the package's folder relative to the compiling project.
hatayama
merged commit Oct 11, 2026
f79a2a0
into
perf/hot-reload-capture-off-main-thread
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
Changes
HotReloadSnapshotCaptureInputs.ReadOnMainThread()reads the project root, the compile start, the registered package roots, the path comparison of the platform, and the compilation assemblies. Each assembly is classified as read-only or editable from the package roots, by its first source, the same rule as before. The assembly list still comes fromGetAssembliesin this PR; a follow-up replaces it with a listing of the compiled dlls and their PDBs and removes it from the inputs.HotReloadSourceSnapshotter.CaptureAfterDomainReload(inputs)andCaptureAssemblies(inputs, documentIndex)take the inputs. The capture reads the read-only flag instead of asking the Package Manager.ShouldSkipImmutablePackageSourcesstays for the default file selection of a run and for the existing cross-check test.HotReloadSourceBaseline.CompareWithCompiledDocumentis split. The newCompareWithCompiledDocumentAtLookupPathtakes a path already named the way the PDB records it and does not touch the Package Manager; the existing method builds that path with the Package Manager and delegates. Its other callers (a run's baseline, the pause-point port) are unchanged.HotReloadSnapshotLookupPath.ToPdbLookupPathbuilds that path from the package roots, through the existingScriptPathNormalizer.ToPhysicalProjectRelative.HotReloadSnapshotSourceChecktakes the package roots and the path comparison and uses it.HotReloadPackageFolderMatch, and the new read-only classification uses the same match.docs/vibe-logs.md:packageLookupMsis now close to 0, andtotalMscovers listing the assemblies plus the per-assembly loop. Reading the compile start and the package roots is not intotalMs;captureMsin the captured entry still covers it.Verification
Local, Unity 2022.3 Editor via the dev
uloopbinary.uloop compile: ErrorCount 0, WarningCount 0.scripts/check-file-length.shandscripts/check-code-complexity.sh: no findings.HotReloadSnapshotCaptureInputsTests.ToCaptureAssembly_FromPackageRoots_MatchesThePackageManagerAnswerForEveryAssembly: 112 assemblies with sources (48 read-only package assemblies), 0 mismatches againstShouldSkipImmutablePackageSources.HotReloadSnapshotLookupPathTests.PdbLookupPath_FromPackageRoots_MatchesThePackageManagerAnswerForEverySource: 5595 sources (4601 of them behind a package folder), 0 mismatches againstScriptPackageRoots.ToPhysicalPath.HotReloadSourceSnapshotTests,HotReloadSourceSnapshotterTests,HotReloadSourceSnapshotCaptureTests,HotReloadSnapshotAssemblyEnumerationTests,HotReloadIncrementalSnapshotCaptureTests,HotReloadSnapshotEditedDuringCompileTests,HotReloadCompiledSourceCrossCheckTests,HotReloadCompiledSourceMapTests,PausePointScriptPathFormTests,HotReloadOrchestratorTests,HotReloadDefaultFilesTests, and the newHotReloadSnapshotCaptureInputsTestsandHotReloadSnapshotLookupPathTests.CaptureAssemblies_ImmutablePackageAssembly_CountsItAndWritesNothingfailed. No capture test covered that skip before, because a temporary project root registers no package.HotReloadSnapshotEditedDuringCompileTests.CaptureAtomically_APackageSourceWrittenSinceTheStartThatMatchesThePdb_RecordsAnUnmarkedStampfailed each time, and only it. Every earlier test that built the check from a real dll and PDB used Assets sources, which the lookup returns unchanged. The PDB records an embedded package's sources relative to the compiling project (./Packages/<folder>/...), so the test moves the package root under its temporary project root, as the real one lies under the real project.HotReloadSnapshotEditedDuringCompileTests: 5 passed.