From b67a7d1d6340adfd7610d066beb5b9ce741d55b6 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Mon, 19 Jun 2023 10:28:44 +0200 Subject: [PATCH 1/3] Make RAR disk cache deterministic (serialize only what's actually used) --- ...olveAssemblyReferenceCacheSerialization.cs | 84 +++++++++++++++++-- .../RARPrecomputedCache_Tests.cs | 15 +++- .../ResolveAssemblyReference.cs | 6 +- src/Tasks/ResGenDependencies.cs | 4 +- src/Tasks/StateFileBase.cs | 18 ++-- src/Tasks/SystemState.cs | 49 ++++++++--- 6 files changed, 146 insertions(+), 30 deletions(-) diff --git a/src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceCacheSerialization.cs b/src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceCacheSerialization.cs index 08e201918af..5607a413d21 100644 --- a/src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceCacheSerialization.cs +++ b/src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceCacheSerialization.cs @@ -20,6 +20,8 @@ public class ResolveAssemblyReferenceCacheSerialization : IDisposable private readonly string _rarCacheFile; private readonly TaskLoggingHelper _taskLoggingHelper; + private static readonly DateTime s_now = DateTime.Now; + public ResolveAssemblyReferenceCacheSerialization() { var tempPath = Path.GetTempPath(); @@ -38,12 +40,20 @@ public void Dispose() } } + private static DateTime GetLastWriteTime(string path) => path switch + { + "path1" => s_now, + "path2" => s_now, + "dllName" => s_now.AddSeconds(-10), + _ => throw new ArgumentException(), + }; + [Fact] public void RoundTripEmptyState() { SystemState systemState = new(); - systemState.SerializeCache(_rarCacheFile, _taskLoggingHelper); + systemState.SerializeCache(_rarCacheFile, _taskLoggingHelper, serializeEmptyState: true); var deserialized = StateFileBase.DeserializeCache(_rarCacheFile, _taskLoggingHelper); @@ -55,7 +65,7 @@ public void CorrectFileVersion() { SystemState systemState = new(); - systemState.SerializeCache(_rarCacheFile, _taskLoggingHelper); + systemState.SerializeCache(_rarCacheFile, _taskLoggingHelper, serializeEmptyState: true); using (var cacheStream = new FileStream(_rarCacheFile, FileMode.Open, FileAccess.ReadWrite)) { cacheStream.Seek(0, SeekOrigin.Begin); @@ -73,7 +83,7 @@ public void WrongFileVersion() { SystemState systemState = new(); - systemState.SerializeCache(_rarCacheFile, _taskLoggingHelper); + systemState.SerializeCache(_rarCacheFile, _taskLoggingHelper, serializeEmptyState: true); using (var cacheStream = new FileStream(_rarCacheFile, FileMode.Open, FileAccess.ReadWrite)) { cacheStream.Seek(0, SeekOrigin.Begin); @@ -90,15 +100,24 @@ public void WrongFileVersion() public void ValidateSerializationAndDeserialization() { Dictionary cache = new() { - { "path1", new SystemState.FileState(DateTime.Now) }, - { "path2", new SystemState.FileState(DateTime.Now) { Assembly = new AssemblyNameExtension("hi") } }, - { "dllName", new SystemState.FileState(DateTime.Now.AddSeconds(-10)) { + { "path1", new SystemState.FileState(GetLastWriteTime("path1")) }, + { "path2", new SystemState.FileState(GetLastWriteTime("path2")) { Assembly = new AssemblyNameExtension("hi") } }, + { "dllName", new SystemState.FileState(GetLastWriteTime("dllName")) { Assembly = null, RuntimeVersion = "v4.0.30319", FrameworkNameAttribute = new FrameworkName(".NETFramework", Version.Parse("4.7.2"), "Profile"), scatterFiles = new string[] { "first", "second" } } } }; SystemState sysState = new(); + sysState.SetGetLastWriteTime(GetLastWriteTime); sysState.instanceLocalFileStateCache = cache; + + // Get all FileState entries to make sure they are marked as having been used. + _ = sysState.GetFileState("path1"); + _ = sysState.GetFileState("path2"); + _ = sysState.GetFileState("dllName"); + + sysState.HasStateToSave.ShouldBe(true); + SystemState sysState2 = null; using (TestEnvironment env = TestEnvironment.Create()) { @@ -119,5 +138,58 @@ public void ValidateSerializationAndDeserialization() dll2.scatterFiles.Length.ShouldBe(dll.scatterFiles.Length); dll2.scatterFiles[1].ShouldBe(dll.scatterFiles[1]); } + + [Fact] + public void OutgoingCacheIsSmallerThanIncomingCache() + { + Dictionary cache = new() { + { "path1", new SystemState.FileState(GetLastWriteTime("path1")) }, + { "path2", new SystemState.FileState(GetLastWriteTime("path2")) } }; + SystemState sysState = new(); + sysState.SetGetLastWriteTime(GetLastWriteTime); + sysState.instanceLocalFileStateCache = cache; + + // Get only the first FileState entry. + _ = sysState.GetFileState("path1"); + + sysState.HasStateToSave.ShouldBe(true); + + SystemState sysState2 = null; + using (TestEnvironment env = TestEnvironment.Create()) + { + TransientTestFile file = env.CreateFile(); + sysState.SerializeCache(file.Path, null); + sysState2 = StateFileBase.DeserializeCache(file.Path, null); + } + + // The new cache has only the entry that was actually used. + Dictionary cache2 = sysState2.instanceLocalFileStateCache; + cache2.Count.ShouldBe(1); + cache2.ShouldContainKey("path1"); + } + + [Fact] + public void OutgoingCacheIsEmpty() + { + Dictionary cache = new() { + { "path1", new SystemState.FileState(GetLastWriteTime("path1")) }, + { "path2", new SystemState.FileState(GetLastWriteTime("path2")) } }; + SystemState sysState = new(); + sysState.SetGetLastWriteTime(GetLastWriteTime); + sysState.instanceLocalFileStateCache = cache; + + sysState.HasStateToSave.ShouldBe(false); + + SystemState sysState2 = null; + using (TestEnvironment env = TestEnvironment.Create()) + { + TransientTestFile file = env.CreateFile(); + sysState.SerializeCache(file.Path, null); + sysState2 = StateFileBase.DeserializeCache(file.Path, null); + } + + // The new cache was not written to disk at all because none of the entries were actually used. + sysState2.ShouldBeNull(); + } } } diff --git a/src/Tasks.UnitTests/RARPrecomputedCache_Tests.cs b/src/Tasks.UnitTests/RARPrecomputedCache_Tests.cs index 062eab76b12..d3c94688e18 100644 --- a/src/Tasks.UnitTests/RARPrecomputedCache_Tests.cs +++ b/src/Tasks.UnitTests/RARPrecomputedCache_Tests.cs @@ -21,14 +21,18 @@ public void TestPrecomputedCacheOutput() { using (TestEnvironment env = TestEnvironment.Create()) { + DateTime now = DateTime.Now; TransientTestFile standardCache = env.CreateFile(".cache"); ResolveAssemblyReference t = new ResolveAssemblyReference() { _cache = new SystemState() }; t._cache.instanceLocalFileStateCache = new Dictionary() { - { Path.Combine(standardCache.Path, "assembly1"), new SystemState.FileState(DateTime.Now) }, - { Path.Combine(standardCache.Path, "assembly2"), new SystemState.FileState(DateTime.Now) { Assembly = new Shared.AssemblyNameExtension("hi") } } }; + { Path.Combine(standardCache.Path, "assembly1"), new SystemState.FileState(now) }, + { Path.Combine(standardCache.Path, "assembly2"), new SystemState.FileState(now) { Assembly = new Shared.AssemblyNameExtension("hi") } } }; + t._cache.SetGetLastWriteTime(_ => now); + _ = t._cache.GetFileState("assembly1"); + _ = t._cache.GetFileState("assembly2"); t._cache.IsDirty = true; t.StateFile = standardCache.Path; t.WriteStateFile(); @@ -52,13 +56,18 @@ public void StandardCacheTakesPrecedence() { using (TestEnvironment env = TestEnvironment.Create()) { + DateTime now = DateTime.Now; TransientTestFile standardCache = env.CreateFile(".cache"); ResolveAssemblyReference rarWriterTask = new ResolveAssemblyReference() { _cache = new SystemState() }; - rarWriterTask._cache.instanceLocalFileStateCache = new Dictionary(); + rarWriterTask._cache.instanceLocalFileStateCache = new() { + { "path1", new SystemState.FileState(now) }, + }; + rarWriterTask._cache.SetGetLastWriteTime(_ => now); rarWriterTask.StateFile = standardCache.Path; + _ = rarWriterTask._cache.GetFileState("path1"); rarWriterTask._cache.IsDirty = true; // Write standard cache rarWriterTask.WriteStateFile(); diff --git a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs index 56e7d8ad826..45d87afb04c 100644 --- a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs +++ b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs @@ -2055,12 +2055,14 @@ internal void ReadStateFile(FileExists fileExists) /// internal void WriteStateFile() { - if (!String.IsNullOrEmpty(AssemblyInformationCacheOutputPath)) + if (!string.IsNullOrEmpty(AssemblyInformationCacheOutputPath)) { _cache.SerializePrecomputedCache(AssemblyInformationCacheOutputPath, Log); } - else if (!String.IsNullOrEmpty(_stateFile) && _cache.IsDirty) + else if (!string.IsNullOrEmpty(_stateFile) && (_cache.IsDirty || _cache.instanceLocalOutgoingFileStateCache.Count < _cache.instanceLocalFileStateCache.Count)) { + // Either the cache is dirty (we added or updated an item) or the number of items actually used is less than what + // we got by reading the state file prior to execution. Serialize the cache into the state file. if (FailIfNotIncremental) { Log.LogErrorFromResources("ResolveAssemblyReference.WritingCacheFile", _stateFile); diff --git a/src/Tasks/ResGenDependencies.cs b/src/Tasks/ResGenDependencies.cs index f441cedcfdc..706899188d7 100644 --- a/src/Tasks/ResGenDependencies.cs +++ b/src/Tasks/ResGenDependencies.cs @@ -185,9 +185,9 @@ internal void UpdatePortableLibrary(PortableLibraryFile library) /// /// Writes the contents of this object out to the specified file. /// - internal override void SerializeCache(string stateFile, TaskLoggingHelper log) + internal override void SerializeCache(string stateFile, TaskLoggingHelper log, bool serializeEmptyState = false) { - base.SerializeCache(stateFile, log); + base.SerializeCache(stateFile, log, serializeEmptyState); _isDirty = false; } diff --git a/src/Tasks/StateFileBase.cs b/src/Tasks/StateFileBase.cs index 061baf82ad9..f3e57ba44e4 100644 --- a/src/Tasks/StateFileBase.cs +++ b/src/Tasks/StateFileBase.cs @@ -28,10 +28,15 @@ internal abstract class StateFileBase // Version this instance is serialized with. private byte _serializedVersion = CurrentSerializationVersion; + /// + /// True if should create the state file and serialize ourselves, false otherwise. + /// + internal virtual bool HasStateToSave => true; + /// /// Writes the contents of this object out to the specified file. /// - internal virtual void SerializeCache(string stateFile, TaskLoggingHelper log) + internal virtual void SerializeCache(string stateFile, TaskLoggingHelper log, bool serializeEmptyState = false) { try { @@ -42,11 +47,14 @@ internal virtual void SerializeCache(string stateFile, TaskLoggingHelper log) File.Delete(stateFile); } - using (var s = new FileStream(stateFile, FileMode.CreateNew)) + if (serializeEmptyState || HasStateToSave) { - var translator = BinaryTranslator.GetWriteTranslator(s); - translator.Translate(ref _serializedVersion); - Translate(translator); + using (var s = new FileStream(stateFile, FileMode.CreateNew)) + { + var translator = BinaryTranslator.GetWriteTranslator(s); + translator.Translate(ref _serializedVersion); + Translate(translator); + } } } } diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index 8290c6611d9..3506abb9df4 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -31,10 +31,21 @@ internal sealed class SystemState : StateFileBase, ITranslatable private Dictionary upToDateLocalFileStateCache = new Dictionary(StringComparer.OrdinalIgnoreCase); /// - /// Cache at the SystemState instance level. It is serialized and reused between instances. - /// + /// Cache at the SystemState instance level. + /// + /// + /// Before starting execution, RAR attempts to populate this field by deserializing a per-project cache file. During execution, + /// objects that get actually used are inserted into . + /// After execution, is serialized and written to disk if it's different from + /// what we originally deserialized into this field. + /// internal Dictionary instanceLocalFileStateCache = new Dictionary(StringComparer.OrdinalIgnoreCase); + /// + /// Cache at the SystemState instance level. It is serialized to disk and reused between instances via . + /// + internal Dictionary instanceLocalOutgoingFileStateCache = new Dictionary(StringComparer.OrdinalIgnoreCase); + /// /// LastModified information is purely instance-local. It doesn't make sense to /// cache this for long periods of time since there's no way (without actually @@ -104,7 +115,6 @@ internal sealed class SystemState : StateFileBase, ITranslatable /// /// Class that holds the current file state. /// - [Serializable] internal sealed class FileState : ITranslatable { /// @@ -256,7 +266,7 @@ public override void Translate(ITranslator translator) } translator.TranslateDictionary( - ref instanceLocalFileStateCache, + ref (translator.Mode == TranslationDirection.WriteToStream) ? ref instanceLocalOutgoingFileStateCache : ref instanceLocalFileStateCache, StringComparer.OrdinalIgnoreCase, (ITranslator t) => new FileState(t)); @@ -265,6 +275,9 @@ public override void Translate(ITranslator translator) IsDirty = false; } + /// + internal override bool HasStateToSave => instanceLocalOutgoingFileStateCache.Count > 0; + /// /// Flag that indicates that has been modified. /// @@ -343,7 +356,7 @@ internal GetAssemblyRuntimeVersion CacheDelegate(GetAssemblyRuntimeVersion getAs return GetRuntimeVersion; } - private FileState GetFileState(string path) + internal FileState GetFileState(string path) { // Looking up an assembly to get its metadata can be expensive for projects that reference large amounts // of assemblies. To avoid that expense, we remember and serialize this information betweeen runs in @@ -373,19 +386,30 @@ private FileState ComputeFileStateFromCachesAndDisk(string path) bool isInstanceFileStateUpToDate = isCachedInInstance && lastModified == cachedInstanceFileState.LastModified; bool isProcessFileStateUpToDate = isCachedInProcess && lastModified == cachedProcessFileState.LastModified; - // If the process-wide cache contains an up-to-date FileState, always use it + // If the process-wide cache contains an up-to-date FileState, always use it. if (isProcessFileStateUpToDate) { // For the next build, we may be using a different process. Update the file cache if the entry is worth persisting. - if (!isInstanceFileStateUpToDate && cachedProcessFileState.IsWorthPersisting) + if (cachedProcessFileState.IsWorthPersisting) { - instanceLocalFileStateCache[path] = cachedProcessFileState; - isDirty = true; + if (!isInstanceFileStateUpToDate) + { + instanceLocalFileStateCache[path] = cachedProcessFileState; + isDirty = true; + } + + // Remember that this FileState was actually used by adding it to the outgoing dictionary. + instanceLocalOutgoingFileStateCache[path] = cachedProcessFileState; } return cachedProcessFileState; } if (isInstanceFileStateUpToDate) { + if (cachedInstanceFileState.IsWorthPersisting) + { + // Remember that this FileState was actually used by adding it to the outgoing dictionary. + instanceLocalOutgoingFileStateCache[path] = cachedInstanceFileState; + } return s_processWideFileStateCache[path] = cachedInstanceFileState; } @@ -412,6 +436,7 @@ private FileState InitializeFileState(string path, DateTime lastModified) if (fileState.IsWorthPersisting) { instanceLocalFileStateCache[path] = fileState; + instanceLocalOutgoingFileStateCache[path] = fileState; isDirty = true; } @@ -584,8 +609,8 @@ internal void SerializePrecomputedCache(string stateFile, TaskLoggingHelper log) { // Save a copy of instanceLocalFileStateCache so we can restore it later. SerializeCacheByTranslator serializes // instanceLocalFileStateCache by default, so change that to the relativized form, then change it back. - Dictionary oldFileStateCache = instanceLocalFileStateCache; - instanceLocalFileStateCache = instanceLocalFileStateCache.ToDictionary(kvp => FileUtilities.MakeRelative(Path.GetDirectoryName(stateFile), kvp.Key), kvp => kvp.Value); + Dictionary oldFileStateCache = instanceLocalOutgoingFileStateCache; + instanceLocalOutgoingFileStateCache = instanceLocalFileStateCache.ToDictionary(kvp => FileUtilities.MakeRelative(Path.GetDirectoryName(stateFile), kvp.Key), kvp => kvp.Value); try { @@ -597,7 +622,7 @@ internal void SerializePrecomputedCache(string stateFile, TaskLoggingHelper log) } finally { - instanceLocalFileStateCache = oldFileStateCache; + instanceLocalOutgoingFileStateCache = oldFileStateCache; } } From 328dac062afed59091613f1e7fa2d301f5a86fc9 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Tue, 20 Jun 2023 22:42:25 +0200 Subject: [PATCH 2/3] Update src/Tasks/SystemState.cs Co-authored-by: AR-May <67507805+AR-May@users.noreply.github.com> --- src/Tasks/SystemState.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index 3506abb9df4..53e38ce818a 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -607,7 +607,7 @@ internal static SystemState DeserializePrecomputedCaches(ITaskItem[] stateFiles, /// How to log internal void SerializePrecomputedCache(string stateFile, TaskLoggingHelper log) { - // Save a copy of instanceLocalFileStateCache so we can restore it later. SerializeCacheByTranslator serializes + // Save a copy of instanceLocalOutgoingFileStateCache so we can restore it later. SerializeCacheByTranslator serializes // instanceLocalFileStateCache by default, so change that to the relativized form, then change it back. Dictionary oldFileStateCache = instanceLocalOutgoingFileStateCache; instanceLocalOutgoingFileStateCache = instanceLocalFileStateCache.ToDictionary(kvp => FileUtilities.MakeRelative(Path.GetDirectoryName(stateFile), kvp.Key), kvp => kvp.Value); From 9e315996391df9e2c9d22b3d3e622a1c88dc97ad Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Tue, 20 Jun 2023 22:42:38 +0200 Subject: [PATCH 3/3] Update src/Tasks/SystemState.cs Co-authored-by: AR-May <67507805+AR-May@users.noreply.github.com> --- src/Tasks/SystemState.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index 53e38ce818a..8774ba6773b 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -608,7 +608,7 @@ internal static SystemState DeserializePrecomputedCaches(ITaskItem[] stateFiles, internal void SerializePrecomputedCache(string stateFile, TaskLoggingHelper log) { // Save a copy of instanceLocalOutgoingFileStateCache so we can restore it later. SerializeCacheByTranslator serializes - // instanceLocalFileStateCache by default, so change that to the relativized form, then change it back. + // instanceLocalOutgoingFileStateCache by default, so change that to the relativized form, then change it back. Dictionary oldFileStateCache = instanceLocalOutgoingFileStateCache; instanceLocalOutgoingFileStateCache = instanceLocalFileStateCache.ToDictionary(kvp => FileUtilities.MakeRelative(Path.GetDirectoryName(stateFile), kvp.Key), kvp => kvp.Value);