Make RAR disk cache deterministic (serialize only what's actually used)

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<SystemState>(_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<string, SystemState.FileState> 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<string, SystemState.FileState> 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<SystemState>(file.Path, null);
+            }
+
+            // The new cache has only the entry that was actually used.
+            Dictionary<string, SystemState.FileState> cache2 = sysState2.instanceLocalFileStateCache;
+            cache2.Count.ShouldBe(1);
+            cache2.ShouldContainKey("path1");
+        }
+
+        [Fact]
+        public void OutgoingCacheIsEmpty()
+        {
+            Dictionary<string, SystemState.FileState> 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<SystemState>(file.Path, null);
+            }
+
+            // The new cache was not written to disk at all because none of the entries were actually used.
+            sysState2.ShouldBeNull();
+        }
     }
 }

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<string, SystemState.FileState>() {
-                    { 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<string, SystemState.FileState>();
+                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();

src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs

@@ -2055,12 +2055,14 @@ internal void ReadStateFile(FileExists fileExists)
         /// </summary>
         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);

src/Tasks/ResGenDependencies.cs

@@ -185,9 +185,9 @@ internal void UpdatePortableLibrary(PortableLibraryFile library)
         /// <summary>
         /// Writes the contents of this object out to the specified file.
         /// </summary>
-        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;
         }
 

src/Tasks/StateFileBase.cs

@@ -28,10 +28,15 @@ internal abstract class StateFileBase
         // Version this instance is serialized with.
         private byte _serializedVersion = CurrentSerializationVersion;
 
+        /// <summary>
+        /// True if <see cref="SerializeCache"/> should create the state file and serialize ourselves, false otherwise.
+        /// </summary>
+        internal virtual bool HasStateToSave => true;
+
         /// <summary>
         /// Writes the contents of this object out to the specified file.
         /// </summary>
-        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);
+                        }
                     }
                 }
             }

src/Tasks/SystemState.cs

@@ -31,10 +31,21 @@ internal sealed class SystemState : StateFileBase, ITranslatable
         private Dictionary<string, FileState> upToDateLocalFileStateCache = new Dictionary<string, FileState>(StringComparer.OrdinalIgnoreCase);
 
         /// <summary>
-        /// Cache at the SystemState instance level. It is serialized and reused between instances.
-        /// </summary>
+        /// Cache at the SystemState instance level.
+        /// </summary>
+        /// <remarks>
+        /// Before starting execution, RAR attempts to populate this field by deserializing a per-project cache file. During execution,
+        /// <see cref="FileState"/> objects that get actually used are inserted into <see cref="instanceLocalOutgoingFileStateCache"/>.
+        /// After execution, <see cref="instanceLocalOutgoingFileStateCache"/> is serialized and written to disk if it's different from
+        /// what we originally deserialized into this field.
+        /// </remarks>
         internal Dictionary<string, FileState> instanceLocalFileStateCache = new Dictionary<string, FileState>(StringComparer.OrdinalIgnoreCase);
 
+        /// <summary>
+        /// Cache at the SystemState instance level. It is serialized to disk and reused between instances via <see cref="instanceLocalFileStateCache"/>.
+        /// </summary>
+        internal Dictionary<string, FileState> instanceLocalOutgoingFileStateCache = new Dictionary<string, FileState>(StringComparer.OrdinalIgnoreCase);
+
         /// <summary>
         /// 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
         /// <summary>
         /// Class that holds the current file state.
         /// </summary>
-        [Serializable]
         internal sealed class FileState : ITranslatable
         {
             /// <summary>
@@ -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;
         }
 
+        /// <inheritdoc />
+        internal override bool HasStateToSave => instanceLocalOutgoingFileStateCache.Count > 0;
+
         /// <summary>
         /// Flag that indicates that <see cref="instanceLocalFileStateCache"/> has been modified.
         /// </summary>
@@ -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;
             }
 
@@ -582,10 +607,10 @@ internal static SystemState DeserializePrecomputedCaches(ITaskItem[] stateFiles,
         /// <param name="log">How to log</param>
         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<string, FileState> oldFileStateCache = instanceLocalFileStateCache;
-            instanceLocalFileStateCache = instanceLocalFileStateCache.ToDictionary(kvp => FileUtilities.MakeRelative(Path.GetDirectoryName(stateFile), kvp.Key), kvp => kvp.Value);
+            // Save a copy of instanceLocalOutgoingFileStateCache so we can restore it later. SerializeCacheByTranslator serializes
+            // instanceLocalOutgoingFileStateCache by default, so change that to the relativized form, then change it back.
+            Dictionary<string, FileState> 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;
             }
         }