Don't save data on immutable files to per-project disk cache (#8802)

Contributes to #8635

Context
With #8688 we no longer need to cache data on immutable framework files on disk as obtaining this information is cheap and does not require I/O.

Changes Made
Made SystemState (the RAR cache) not add such files to the per-instance dictionary. This dictionary is serialized to disk as <project-file>.AssemblyReference.cache so the change effectively makes the cache file smaller, saving on serialization and deserialization time.

Also refactored DeserializeCache into a generic method instead of taking a Type parameter.

For a simple ASP.NET Core app with one project reference and one package reference, the size of the cache file is reduced from 142 kB to 2 kB. Projects with no references other than the SDK will not have the cache file created at all.

Testing
Existing unit tests, manual measurements.

Notes
The actual "Don't read the file if the data is already in memory" change will be in a separate upcoming PR.

src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs

@@ -8618,6 +8618,9 @@ public void SDKReferencesAreResolvedWithoutIO()
             rar.ResolvedFiles.Length.ShouldBe(1);
             rar.ResolvedFiles[0].ItemSpec.ShouldBe(refPath);
             rar.ResolvedFiles[0].GetMetadata("FusionName").ShouldBe("System.Candy, Version=8.1.2.3, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a");
+
+            // The reference is not worth persisting in the per-instance cache.
+            rar._cache.IsDirty.ShouldBeFalse();
         }
 
         [Fact]

src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceCacheSerialization.cs

@@ -45,7 +45,7 @@ public void RoundTripEmptyState()
 
             systemState.SerializeCache(_rarCacheFile, _taskLoggingHelper);
 
-            var deserialized = SystemState.DeserializeCache(_rarCacheFile, _taskLoggingHelper, typeof(SystemState));
+            var deserialized = StateFileBase.DeserializeCache<SystemState>(_rarCacheFile, _taskLoggingHelper);
 
             deserialized.ShouldNotBeNull();
         }
@@ -63,7 +63,7 @@ public void CorrectFileVersion()
                 cacheStream.Close();
             }
 
-            var deserialized = SystemState.DeserializeCache(_rarCacheFile, _taskLoggingHelper, typeof(SystemState));
+            var deserialized = StateFileBase.DeserializeCache<SystemState>(_rarCacheFile, _taskLoggingHelper);
 
             deserialized.ShouldNotBeNull();
         }
@@ -81,7 +81,7 @@ public void WrongFileVersion()
                 cacheStream.Close();
             }
 
-            var deserialized = SystemState.DeserializeCache(_rarCacheFile, _taskLoggingHelper, typeof(SystemState));
+            var deserialized = StateFileBase.DeserializeCache<SystemState>(_rarCacheFile, _taskLoggingHelper);
 
             deserialized.ShouldBeNull();
         }
@@ -104,7 +104,7 @@ public void ValidateSerializationAndDeserialization()
             {
                 TransientTestFile file = env.CreateFile();
                 sysState.SerializeCache(file.Path, null);
-                sysState2 = SystemState.DeserializeCache(file.Path, null, typeof(SystemState)) as SystemState;
+                sysState2 = StateFileBase.DeserializeCache<SystemState>(file.Path, null);
             }
 
             Dictionary<string, SystemState.FileState> cache2 = sysState2.instanceLocalFileStateCache;

src/Tasks.UnitTests/AssemblyRegistrationCache_Tests.cs

@@ -40,7 +40,7 @@ public void ExerciseCacheSerialization()
             {
                 TransientTestFile file = env.CreateFile();
                 arc.SerializeCache(file.Path, null);
-                arc2 = StateFileBase.DeserializeCache(file.Path, null, typeof(AssemblyRegistrationCache)) as AssemblyRegistrationCache;
+                arc2 = StateFileBase.DeserializeCache<AssemblyRegistrationCache>(file.Path, null);
             }
 
             arc2._assemblies.Count.ShouldBe(arc._assemblies.Count);

src/Tasks.UnitTests/ResolveComReference_Tests.cs

@@ -74,7 +74,7 @@ public void TestSerializationAndDeserialization()
             {
                 TransientTestFile file = env.CreateFile();
                 cache.SerializeCache(file.Path, null);
-                cache2 = StateFileBase.DeserializeCache(file.Path, null, typeof(ResolveComReferenceCache)) as ResolveComReferenceCache;
+                cache2 = StateFileBase.DeserializeCache<ResolveComReferenceCache>(file.Path, null);
             }
 
             cache2.tlbImpLocation.ShouldBe(cache.tlbImpLocation);

src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs

@@ -2036,7 +2036,7 @@ private void LogConflict(Reference reference, string fusionName, StringBuilder l
         /// </summary>
         internal void ReadStateFile(FileExists fileExists)
         {
-            _cache = SystemState.DeserializeCache(_stateFile, Log, typeof(SystemState)) as SystemState;
+            _cache = SystemState.DeserializeCache<SystemState>(_stateFile, Log);
 
             // Construct the cache only if we can't find any caches.
             if (_cache == null && AssemblyInformationCachePaths != null && AssemblyInformationCachePaths.Length > 0)
@@ -2313,7 +2313,7 @@ internal bool Execute(
                         {
                             // We don't want to perform I/O to see what the actual timestamp on disk is so we return a fixed made up value.
                             // Note that this value makes the file exist per the check in SystemState.FileTimestampIndicatesFileExists.
-                            return DateTime.MaxValue;
+                            return SystemState.FileState.ImmutableFileLastModifiedMarker;
                         }
                         return getLastWriteTime(path);
                     });

src/Tasks/RegisterAssembly.cs

@@ -76,7 +76,7 @@ public override bool Execute()
 
             if ((AssemblyListFile?.ItemSpec.Length > 0))
             {
-                cacheFile = (AssemblyRegistrationCache)StateFileBase.DeserializeCache(AssemblyListFile.ItemSpec, Log, typeof(AssemblyRegistrationCache)) ??
+                cacheFile = StateFileBase.DeserializeCache<AssemblyRegistrationCache>(AssemblyListFile.ItemSpec, Log) ??
                             new AssemblyRegistrationCache();
             }
 

src/Tasks/ResGenDependencies.cs

@@ -196,7 +196,7 @@ internal override void SerializeCache(string stateFile, TaskLoggingHelper log)
         /// </summary>
         internal static ResGenDependencies DeserializeCache(string stateFile, bool useSourcePath, TaskLoggingHelper log)
         {
-            var retVal = (ResGenDependencies)DeserializeCache(stateFile, log, typeof(ResGenDependencies)) ?? new ResGenDependencies();
+            var retVal = DeserializeCache<ResGenDependencies>(stateFile, log) ?? new ResGenDependencies();
 
             // Ensure that the cache is properly initialized with respect to how resgen will 
             // resolve linked files within .resx files.  ResGen has two different

src/Tasks/ResolveComReference.cs

@@ -333,7 +333,7 @@ public override bool Execute()
             allProjectRefs = new List<ComReferenceInfo>();
             allDependencyRefs = new List<ComReferenceInfo>();
 
-            _timestampCache = (ResolveComReferenceCache)StateFileBase.DeserializeCache(StateFile, Log, typeof(ResolveComReferenceCache));
+            _timestampCache = StateFileBase.DeserializeCache<ResolveComReferenceCache>(StateFile, Log);
 
             if (_timestampCache?.ToolPathsMatchCachePaths(_tlbimpPath, _aximpPath) != true)
             {

src/Tasks/StateFileBase.cs

@@ -65,9 +65,9 @@ internal virtual void SerializeCache(string stateFile, TaskLoggingHelper log)
         /// <summary>
         /// Reads the specified file from disk into a StateFileBase derived object.
         /// </summary>
-        internal static StateFileBase DeserializeCache(string stateFile, TaskLoggingHelper log, Type requiredReturnType)
+        internal static T DeserializeCache<T>(string stateFile, TaskLoggingHelper log) where T : StateFileBase
         {
-            StateFileBase retVal = null;
+            T retVal = null;
 
             // First, we read the cache from disk if one exists, or if one does not exist, we create one.
             try
@@ -88,21 +88,20 @@ internal static StateFileBase DeserializeCache(string stateFile, TaskLoggingHelp
                             return null;
                         }
 
-                        var constructors = requiredReturnType.GetConstructors();
+                        var constructors = typeof(T).GetConstructors();
                         foreach (var constructor in constructors)
                         {
                             var parameters = constructor.GetParameters();
                             if (parameters.Length == 1 && parameters[0].ParameterType == typeof(ITranslator))
                             {
-                                retVal = constructor.Invoke(new object[] { translator }) as StateFileBase;
+                                retVal = constructor.Invoke(new object[] { translator }) as T;
                             }
                         }
 
-                        if (retVal == null || !requiredReturnType.IsInstanceOfType(retVal))
+                        if (retVal == null)
                         {
                             log.LogMessageFromResources("General.CouldNotReadStateFileMessage", stateFile,
                                 log.FormatResourceString("General.IncompatibleStateFileType"));
-                            retVal = null;
                         }
                     }
                 }

src/Tasks/SystemState.cs

@@ -208,6 +208,16 @@ internal FrameworkName FrameworkNameAttribute
                 get { return frameworkName; }
                 set { frameworkName = value; }
             }
+
+            /// <summary>
+            /// The last-modified value to use for immutable framework files which we don't do I/O on.
+            /// </summary>
+            internal static DateTime ImmutableFileLastModifiedMarker => DateTime.MaxValue;
+
+            /// <summary>
+            /// It is wasteful to persist entries for immutable framework files.
+            /// </summary>
+            internal bool IsWorthPersisting => lastModified != ImmutableFileLastModifiedMarker;
         }
 
         /// <summary>
@@ -256,7 +266,7 @@ public override void Translate(ITranslator translator)
         }
 
         /// <summary>
-        /// Flag that indicates
+        /// Flag that indicates that <see cref="instanceLocalFileStateCache"/> has been modified.
         /// </summary>
         /// <value></value>
         internal bool IsDirty
@@ -337,7 +347,7 @@ private 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
-            // XXXResolveAssemblyReferencesInput.cache files in the intermediate directory and also store it in an
+            // <ProjectFileName>.AssemblyReference.cache files in the intermediate directory and also store it in an
             // process-wide cache to share between successive builds.
             //
             // To determine if this information is up-to-date, we use the last modified date of the assembly, however,
@@ -366,8 +376,8 @@ private FileState ComputeFileStateFromCachesAndDisk(string path)
             // 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 (!isInstanceFileStateUpToDate)
+                // 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)
                 {
                     instanceLocalFileStateCache[path] = cachedProcessFileState;
                     isDirty = true;
@@ -397,9 +407,15 @@ private DateTime GetAndCacheLastModified(string path)
         private FileState InitializeFileState(string path, DateTime lastModified)
         {
             var fileState = new FileState(lastModified);
-            instanceLocalFileStateCache[path] = fileState;
+
+            // Dirty the instance-local cache only with entries that are worth persisting.
+            if (fileState.IsWorthPersisting)
+            {
+                instanceLocalFileStateCache[path] = fileState;
+                isDirty = true;
+            }
+
             s_processWideFileStateCache[path] = fileState;
-            isDirty = true;
 
             return fileState;
         }
@@ -448,7 +464,10 @@ private AssemblyNameExtension GetAssemblyName(string path)
                 {
                     fileState.Assembly = AssemblyNameExtension.UnnamedAssembly;
                 }
-                isDirty = true;
+                if (fileState.IsWorthPersisting)
+                {
+                    isDirty = true;
+                }
             }
 
             if (fileState.Assembly.IsUnnamedAssembly)
@@ -469,7 +488,10 @@ private string GetRuntimeVersion(string path)
             if (String.IsNullOrEmpty(fileState.RuntimeVersion))
             {
                 fileState.RuntimeVersion = getAssemblyRuntimeVersion(path);
-                isDirty = true;
+                if (fileState.IsWorthPersisting)
+                {
+                    isDirty = true;
+                }
             }
 
             return fileState.RuntimeVersion;
@@ -501,7 +523,10 @@ private void GetAssemblyMetadata(
                     out fileState.scatterFiles,
                     out fileState.frameworkName);
 
-                isDirty = true;
+                if (fileState.IsWorthPersisting)
+                {
+                    isDirty = true;
+                }
             }
 
             dependencies = fileState.dependencies;
@@ -525,7 +550,7 @@ internal static SystemState DeserializePrecomputedCaches(ITaskItem[] stateFiles,
             foreach (ITaskItem stateFile in stateFiles)
             {
                 // Verify that it's a real stateFile. Log message but do not error if not.
-                SystemState sysState = DeserializeCache(stateFile.ToString(), log, typeof(SystemState)) as SystemState;
+                SystemState sysState = DeserializeCache<SystemState>(stateFile.ToString(), log);
                 if (sysState == null)
                 {
                     continue;

src/Tasks/UnregisterAssembly.cs

@@ -53,7 +53,7 @@ public override bool Execute()
 
             if (AssemblyListFile != null)
             {
-                cacheFile = (AssemblyRegistrationCache)StateFileBase.DeserializeCache(AssemblyListFile.ItemSpec, Log, typeof(AssemblyRegistrationCache));
+                cacheFile = StateFileBase.DeserializeCache<AssemblyRegistrationCache>(AssemblyListFile.ItemSpec, Log);
 
                 // no cache file, nothing to do. In case there was a problem reading the cache file, we can't do anything anyway.
                 if (cacheFile == null)