diff --git a/Assets/Tests/Editor/HotReload/HotReloadCompiledCallSiteCacheTests.cs b/Assets/Tests/Editor/HotReload/HotReloadCompiledCallSiteCacheTests.cs index 97a84f6ec..ded3f916e 100644 --- a/Assets/Tests/Editor/HotReload/HotReloadCompiledCallSiteCacheTests.cs +++ b/Assets/Tests/Editor/HotReload/HotReloadCompiledCallSiteCacheTests.cs @@ -15,7 +15,7 @@ namespace io.github.hatayama.UnityCliLoop.Tests.Editor.HotReload { /// - /// Invalidation, capacity, run hold, and call-site index contract of the compiled call-site + /// Invalidation, byte budget, run hold, and call-site index contract of the compiled call-site /// cache. Each test works on copies of this test assembly's dll in a private temp directory so /// that mutating the file never touches ScriptAssemblies. /// @@ -46,7 +46,7 @@ public void SetUp() Path.GetTempPath(), "uloop-call-site-cache-" + Guid.NewGuid().ToString("N")); Directory.CreateDirectory(_tempDirectory); - _cache = new HotReloadCompiledCallSiteCache(2); + _cache = new HotReloadCompiledCallSiteCache(BudgetForEntries(2)); } [TearDown] @@ -185,7 +185,7 @@ public void GetOrLoad_FileReplacedBetweenFingerprintAndRead_PublishesConsistentE File.SetLastWriteTimeUtc(dllPath, originalWriteTime); } }; - HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(2, probes); + HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(BudgetForEntries(2), probes); try { HotReloadCompiledCallSiteCache.Entry entry = cache.GetOrLoad(dllPath); @@ -227,7 +227,7 @@ public void GetOrLoad_FileChangesBeforeEveryRead_ThrowsWithoutPublishing() useOther = !useOther; } }; - HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(2, probes); + HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(BudgetForEntries(2), probes); try { Assert.Throws(() => cache.GetOrLoad(dllPath)); @@ -257,7 +257,7 @@ public void GetOrLoad_IndexBuildFails_DisposesAssemblyAndRethrows() throw injected; } }; - HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(2, probes); + HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(BudgetForEntries(2), probes); try { InvalidOperationException thrown = Assert.Throws(() => cache.GetOrLoad(dllPath)); @@ -295,11 +295,11 @@ public void GetOrLoad_DifferentPath_LoadsSeparately() } /// - /// What: loading beyond the capacity evicts the least recently used entry, so the count - /// never exceeds the cap and the evicted dll is read again on its next lookup. + /// What: loading beyond the budget evicts the least recently used entry, so the cached bytes + /// never exceed the budget and the evicted dll is read again on its next lookup. /// [Test] - public void GetOrLoad_OverCapacity_EvictsLeastRecentlyUsed() + public void GetOrLoad_OverBudget_EvictsLeastRecentlyUsed() { string pathA = CopyTestAssembly("a.dll"); string pathB = CopyTestAssembly("b.dll"); @@ -321,20 +321,20 @@ public void GetOrLoad_OverCapacity_EvictsLeastRecentlyUsed() } /// - /// What: a non-positive capacity is rejected, because the cache could never hold an entry. + /// What: a non-positive budget is rejected, because the cache could never hold an entry. /// [Test] - public void Constructor_NonPositiveCapacity_Throws() + public void Constructor_NonPositiveBudget_Throws() { Assert.Throws(() => new HotReloadCompiledCallSiteCache(0)); } /// - /// What: inside a hold each dll is read once even when a run touches more dlls than the - /// capacity, so a run never re-reads a dll it already read. + /// What: inside a hold each dll is read once even when a run touches more bytes than the + /// budget, so a run never re-reads a dll it already read. /// [Test] - public void GetOrLoad_InsideHold_ReadsEachDllOnce_EvenBeyondCapacity() + public void GetOrLoad_InsideHold_ReadsEachDllOnce_EvenBeyondBudget() { string pathA = CopyTestAssembly("a.dll"); string pathB = CopyTestAssembly("b.dll"); @@ -358,11 +358,11 @@ public void GetOrLoad_InsideHold_ReadsEachDllOnce_EvenBeyondCapacity() } /// - /// What: ending the hold evicts the least recently used entries down to the capacity, so + /// What: ending the hold evicts the least recently used entries down to the budget, so /// the dlls used last stay cached for the next run and the evicted one is read again. /// [Test] - public void HoldEntriesForRun_Dispose_EvictsDownToCapacity() + public void HoldEntriesForRun_Dispose_EvictsDownToBudget() { string pathA = CopyTestAssembly("a.dll"); string pathB = CopyTestAssembly("b.dll"); @@ -389,7 +389,7 @@ public void HoldEntriesForRun_Dispose_EvictsDownToCapacity() /// /// What: ending an inner hold keeps every entry while the outer hold is open; only the end - /// of the outer hold evicts down to the capacity. + /// of the outer hold evicts down to the budget. /// [Test] public void HoldEntriesForRun_Nested_KeepsEntriesUntilTheOuterHoldEnds() @@ -441,7 +441,7 @@ public void HoldEntriesForRun_DisposeTwice_IsIgnored() /// /// What: without a hold a miss still evicts the least recently used entry before it adds, - /// so lookups outside a run keep the cache within its capacity. + /// so lookups outside a run keep the cache within its budget. /// [Test] public void GetOrLoad_WithoutHold_StillEvictsBeforeAdding() @@ -452,6 +452,137 @@ public void GetOrLoad_WithoutHold_StillEvictsBeforeAdding() Assert.That(_cache.Count, Is.EqualTo(2)); Assert.That(_cache.LoadCount, Is.EqualTo(3)); + Assert.That(_cache.CachedBytes, Is.EqualTo(BudgetForEntries(2))); + } + + /// + /// What: when every dll a run touched fits the budget, ending the hold evicts nothing, so + /// the next run reads none of them again. A count cap smaller than the run evicted some. + /// + [Test] + public void HoldEntriesForRun_Dispose_KeepsEveryEntryWithinBudget() + { + HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(BudgetForEntries(3)); + try + { + string pathA = CopyTestAssembly("a.dll"); + string pathB = CopyTestAssembly("b.dll"); + string pathC = CopyTestAssembly("c.dll"); + using (cache.HoldEntriesForRun()) + { + cache.GetOrLoad(pathA); + cache.GetOrLoad(pathB); + cache.GetOrLoad(pathC); + } + + Assert.That(cache.Count, Is.EqualTo(3)); + Assert.That(cache.CachedBytes, Is.EqualTo(BudgetForEntries(3))); + Assert.That(cache.LastHoldReleaseEvictedCount, Is.EqualTo(0)); + + cache.GetOrLoad(pathA); + cache.GetOrLoad(pathB); + cache.GetOrLoad(pathC); + Assert.That(cache.LoadCount, Is.EqualTo(3), "The next run reads none of the dlls again."); + } + finally + { + cache.Clear(); + } + } + + /// + /// What: ending a hold over the budget records how many entries and bytes it evicted. + /// + [Test] + public void HoldEntriesForRun_Dispose_RecordsWhatItEvicted() + { + using (_cache.HoldEntriesForRun()) + { + _cache.GetOrLoad(CopyTestAssembly("a.dll")); + _cache.GetOrLoad(CopyTestAssembly("b.dll")); + _cache.GetOrLoad(CopyTestAssembly("c.dll")); + } + + Assert.That(_cache.Count, Is.EqualTo(2)); + Assert.That(_cache.CachedBytes, Is.EqualTo(BudgetForEntries(2))); + Assert.That(_cache.LastHoldReleaseEvictedCount, Is.EqualTo(1)); + Assert.That(_cache.LastHoldReleaseEvictedBytes, Is.EqualTo(TestAssemblyLength())); + } + + /// + /// What: ending a hold keeps the last entry even when it alone exceeds the budget, so the + /// end of a run never empties the cache. + /// + [Test] + public void HoldEntriesForRun_Dispose_KeepsTheLastEntryOverBudget() + { + HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(TestAssemblyLength() / 2); + try + { + using (cache.HoldEntriesForRun()) + { + cache.GetOrLoad(CopyTestAssembly("a.dll")); + cache.GetOrLoad(CopyTestAssembly("b.dll")); + } + + Assert.That(cache.Count, Is.EqualTo(1)); + Assert.That(cache.CachedBytes, Is.EqualTo(TestAssemblyLength())); + Assert.That(cache.LastHoldReleaseEvictedCount, Is.EqualTo(1)); + Assert.That(cache.LastHoldReleaseEvictedBytes, Is.EqualTo(TestAssemblyLength())); + } + finally + { + cache.Clear(); + } + } + + /// + /// What: a dll larger than the whole budget is still cached, alone, and the next dll + /// replaces it, so a large assembly never makes a lookup fail or loop. + /// + [Test] + public void GetOrLoad_EntryLargerThanBudget_IsStillLoaded() + { + HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(TestAssemblyLength() / 2); + try + { + cache.GetOrLoad(CopyTestAssembly("a.dll")); + Assert.That(cache.Count, Is.EqualTo(1)); + Assert.That(cache.CachedBytes, Is.EqualTo(TestAssemblyLength())); + + cache.GetOrLoad(CopyTestAssembly("b.dll")); + Assert.That(cache.Count, Is.EqualTo(1), "A was evicted to make room for B."); + Assert.That(cache.LoadCount, Is.EqualTo(2)); + } + finally + { + cache.Clear(); + } + } + + /// + /// What: replacing a stale entry counts the new file's length instead of adding it to the + /// old one, so the cached bytes match what is actually held. + /// + [Test] + public void GetOrLoad_ReplacedEntry_UpdatesCachedBytes() + { + string dllPath = CopyTestAssembly("a.dll"); + DateTime originalWriteTime = File.GetLastWriteTimeUtc(dllPath); + _cache.GetOrLoad(dllPath); + + // Why append: trailing bytes after the PE image keep the dll readable for Cecil while + // changing the length, as in GetOrLoad_SizeChanged_Reloads. + using (FileStream stream = new FileStream(dllPath, FileMode.Append)) + { + stream.WriteByte(0); + } + + File.SetLastWriteTimeUtc(dllPath, originalWriteTime); + _cache.GetOrLoad(dllPath); + + Assert.That(_cache.CachedBytes, Is.EqualTo(new FileInfo(dllPath).Length)); + Assert.That(_cache.Count, Is.EqualTo(1)); } /// @@ -503,7 +634,7 @@ public void GetOrLoad_ReleasesMethodBodiesAfterIndexing() { AfterAssemblyRead = assembly => captured = assembly }; - HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(2, probes); + HotReloadCompiledCallSiteCache cache = new HotReloadCompiledCallSiteCache(BudgetForEntries(2), probes); try { HotReloadCompiledCallSiteCache.Entry entry = cache.GetOrLoad(CopyTestAssembly("a.dll")); @@ -607,7 +738,7 @@ private static byte[] ReadOtherAssemblyPaddedTo(long length) return padded; } - private string CopyTestAssembly(string fileName) + private static string TestAssemblySourcePath() { string projectRoot = Path.GetFullPath(Path.Combine(Application.dataPath, "..")); string sourceDllPath = Path.Combine( @@ -616,6 +747,23 @@ private string CopyTestAssembly(string fileName) "ScriptAssemblies", TestAssemblyName + ".dll"); Assert.That(File.Exists(sourceDllPath), Is.True, "Test assembly dll missing: " + sourceDllPath); + return sourceDllPath; + } + + // Every fixture dll is a copy of the test assembly, so all of them have this length. + private static long TestAssemblyLength() + { + return new FileInfo(TestAssemblySourcePath()).Length; + } + + private static long BudgetForEntries(int count) + { + return count * TestAssemblyLength(); + } + + private string CopyTestAssembly(string fileName) + { + string sourceDllPath = TestAssemblySourcePath(); string destinationPath = Path.Combine(_tempDirectory, fileName); File.Copy(sourceDllPath, destinationPath); diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadCompiledCallSiteCache.cs b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadCompiledCallSiteCache.cs index 2e1fb85a9..ac61191ee 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadCompiledCallSiteCache.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/HotReloadCompiledCallSiteCache.cs @@ -8,6 +8,8 @@ using UnityEngine; +using io.github.hatayama.UnityCliLoop.ToolContracts; + namespace io.github.hatayama.UnityCliLoop.FirstPartyTools { /// @@ -20,12 +22,16 @@ namespace io.github.hatayama.UnityCliLoop.FirstPartyTools /// internal sealed class HotReloadCompiledCallSiteCache { - // Why 64: an entry keeps the dll's bytes (InMemory) and its metadata only, because the - // methods' instruction lists are released once the call sites are collected. In a large - // project one run scanned the target assembly plus the 20 assemblies that reference it, - // and a cap of 8 made every scan read all 21 again. Within a run nothing is evicted - // whatever the cap (HoldEntriesForRun); the cap bounds what stays cached between runs. - internal const int DefaultCapacity = 64; + // Why a byte budget: an entry costs about its dll's length, because it keeps the dll's + // bytes (InMemory) and its metadata only; the methods' instruction lists are released + // once the call sites are collected. Dlls range from kilobytes to megabytes, so a count + // bounds neither memory nor a run: with a cap of 64, a run that scanned 68 assemblies + // (42 MB) evicted 9 of them (29 MB, the edited assembly among them) at every hold release + // and read them again on the next run, about 2 s per run. 256 MB keeps every assembly + // such a run touches and still bounds the Editor's memory. Within a run nothing is + // evicted whatever the budget (HoldEntriesForRun); the budget bounds what stays cached + // between runs. The budget counts file lengths, not the exact in-memory size. + internal const long DefaultBudgetBytes = 256L * 1024 * 1024; /// /// One compiled instruction that may reference a target method. @@ -151,25 +157,29 @@ internal sealed class LoadProbes private const int MaxConsistentLoadAttempts = 2; public static HotReloadCompiledCallSiteCache Shared { get; } = - new HotReloadCompiledCallSiteCache(DefaultCapacity); + new HotReloadCompiledCallSiteCache(DefaultBudgetBytes); private readonly object _gate = new object(); - private readonly int _capacity; + private readonly long _budgetBytes; private readonly LoadProbes _probes; private readonly Dictionary _entries = new Dictionary(StringComparer.Ordinal); private long _accessSequence; private int _loadCount; + // Sum of the cached entries' dll lengths; kept in step with _entries under _gate. + private long _cachedBytes; + private int _lastHoldReleaseEvictedCount; + private long _lastHoldReleaseEvictedBytes; // Number of open holds; touched only under _gate. private int _holdDepth; - public HotReloadCompiledCallSiteCache(int capacity, LoadProbes probes = null) + public HotReloadCompiledCallSiteCache(long budgetBytes, LoadProbes probes = null) { - if (capacity <= 0) + if (budgetBytes <= 0) { - throw new ArgumentOutOfRangeException(nameof(capacity), "capacity must be positive."); + throw new ArgumentOutOfRangeException(nameof(budgetBytes), "budgetBytes must be positive."); } - _capacity = capacity; + _budgetBytes = budgetBytes; _probes = probes; } @@ -201,6 +211,49 @@ public int LoadCount } } + /// + /// Sum of the dll lengths of the entries currently held. It exceeds the budget only while + /// a hold is open, or when a single entry is larger than the budget. + /// + public long CachedBytes + { + get + { + lock (_gate) + { + return _cachedBytes; + } + } + } + + /// + /// Number of entries evicted when the outermost hold last ended; 0 before any hold ended. + /// + public int LastHoldReleaseEvictedCount + { + get + { + lock (_gate) + { + return _lastHoldReleaseEvictedCount; + } + } + } + + /// + /// Sum of the dll lengths of the entries evicted when the outermost hold last ended. + /// + public long LastHoldReleaseEvictedBytes + { + get + { + lock (_gate) + { + return _lastHoldReleaseEvictedBytes; + } + } + } + /// /// Returns the cached view of while the file's length, last /// write time, and module version id (MVID) all match the cached entry; otherwise reads the @@ -229,6 +282,7 @@ public Entry GetOrLoad(string dllPath) } _entries.Remove(fullPath); + _cachedBytes -= existing.Fingerprint.Length; existing.Dispose(); } @@ -238,18 +292,19 @@ public Entry GetOrLoad(string dllPath) // eviction inside the run turns one read per dll into one read per scan. if (_holdDepth == 0) { - EvictLeastRecentlyUsedWhileOverCapacity(); + EvictLeastRecentlyUsedUntilFits(loaded.Fingerprint.Length); } _entries[fullPath] = loaded; + _cachedBytes += loaded.Fingerprint.Length; return loaded; } } /// /// Keeps every entry until the returned hold is disposed, however many dlls are read in the - /// meantime; when the outermost hold ends, the least recently used entries are evicted down - /// to the capacity. Holds nest, and disposing one hold twice releases it once. + /// meantime; when the outermost hold ends, the least recently used entries are evicted until + /// the cached bytes fit the budget, keeping at least one entry. Holds nest, and disposing one hold twice releases it once. /// public IDisposable HoldEntriesForRun() { @@ -274,6 +329,7 @@ public void Clear() } _entries.Clear(); + _cachedBytes = 0; } } @@ -298,11 +354,43 @@ public void Dispose() return; } - owner.ReleaseHold(); + HoldRelease release = owner.ReleaseHold(); + if (release.EvictedCount == 0) + { + return; + } + + // Why outside the lock: the vibe log writes a file, which must not block lookups. + // Only numbers are logged; dll paths may name the user's project. + VibeLogger.LogInfo( + HotReloadConstants.VibeLogCallSiteCacheEvicted, + "Hot reload call-site cache evicted entries at the end of a run.", + new + { + evictedCount = release.EvictedCount, + evictedBytes = release.EvictedBytes, + cachedBytes = release.CachedBytes, + budgetBytes = owner._budgetBytes + }); + } + } + + // What ending a hold evicted, read under the lock and logged after it is released. + private readonly struct HoldRelease + { + public readonly int EvictedCount; + public readonly long EvictedBytes; + public readonly long CachedBytes; + + public HoldRelease(int evictedCount, long evictedBytes, long cachedBytes) + { + EvictedCount = evictedCount; + EvictedBytes = evictedBytes; + CachedBytes = cachedBytes; } } - private void ReleaseHold() + private HoldRelease ReleaseHold() { lock (_gate) { @@ -310,29 +398,40 @@ private void ReleaseHold() _holdDepth--; if (_holdDepth > 0) { - return; + return new HoldRelease(0, 0, _cachedBytes); } - EvictLeastRecentlyUsedDownToCapacity(); + EvictLeastRecentlyUsedDownToBudget(); + return new HoldRelease(_lastHoldReleaseEvictedCount, _lastHoldReleaseEvictedBytes, _cachedBytes); } } - private void EvictLeastRecentlyUsedWhileOverCapacity() + private void EvictLeastRecentlyUsedUntilFits(long incomingBytes) { - // The new entry is added after this call, so make room for it. - while (_entries.Count >= _capacity) + // The new entry is added after this call, so make room for it. Stops once the cache is + // empty: an entry larger than the budget is still added, alone. + while (_entries.Count > 0 && _cachedBytes + incomingBytes > _budgetBytes) { EvictEntry(FindLeastRecentlyUsedPath()); } } - private void EvictLeastRecentlyUsedDownToCapacity() + private void EvictLeastRecentlyUsedDownToBudget() { - // Nothing is added after this call, so keep entries up to the capacity itself. - while (_entries.Count > _capacity) + // Nothing is added after this call, so evict down to the budget itself, but keep the + // last entry even when it alone exceeds the budget, as a lookup without a hold would. + int evictedCount = 0; + long evictedBytes = 0; + while (_cachedBytes > _budgetBytes && _entries.Count > 1) { - EvictEntry(FindLeastRecentlyUsedPath()); + string path = FindLeastRecentlyUsedPath(); + evictedBytes += _entries[path].Fingerprint.Length; + EvictEntry(path); + evictedCount++; } + + _lastHoldReleaseEvictedCount = evictedCount; + _lastHoldReleaseEvictedBytes = evictedBytes; } private string FindLeastRecentlyUsedPath() @@ -355,6 +454,7 @@ private void EvictEntry(string path) { Entry evicted = _entries[path]; _entries.Remove(path); + _cachedBytes -= evicted.Fingerprint.Length; evicted.Dispose(); } diff --git a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs index ed68369eb..a6c24ce69 100644 --- a/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs +++ b/Packages/src/Editor/FirstPartyTools/HotReload/Shared/HotReloadConstants.cs @@ -650,6 +650,7 @@ public static bool IsPublicizableProjectAssemblyFileName(string fileNameWithoutE public const string VibeLogRevertFailed = "hot_reload_revert_failed"; public const string VibeLogApplySummary = "hot_reload_apply_summary"; public const string VibeLogTimingDetail = "hot_reload_timing_detail"; + public const string VibeLogCallSiteCacheEvicted = "hot_reload_call_site_cache_evicted"; // Steps of an apply run outside the response's Timing phases, as named in the // hot_reload_timing_detail vibe entry. Steps that run once per group add up. diff --git a/docs/vibe-logs.md b/docs/vibe-logs.md index 960dc5ad8..0d59cfb84 100644 --- a/docs/vibe-logs.md +++ b/docs/vibe-logs.md @@ -35,6 +35,10 @@ command ran against: each step outside the response's `Timing` phases (`steps`), the phases themselves, and `unaccountedMs` — the part of `otherMs` no step covers. A negative `unaccountedMs` means a step overlaps a phase. + When the end of a run evicted entries from the call-site cache, it also writes one + `hot_reload_call_site_cache_evicted` with the evicted count and bytes, the bytes still cached, + and the budget — a sign that the assemblies one run scans do not fit the cache and are read + again on the next run. - CLI side: only when the `ULOOP_DEBUG` environment variable is set to a value other than empty, `0`, or `false` (`cli/common/vibelog/cli_vibe.go`). - A missing line is evidence only when the define was set and the code path logs at all.