From f303a8349cf799f07f53dbed368ba35e213606eb Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Mon, 22 May 2023 12:08:29 +0200 Subject: [PATCH 1/3] Make StateFileBase.DeserializeCache generic --- .../ResolveAssemblyReferenceCacheSerialization.cs | 8 ++++---- .../AssemblyRegistrationCache_Tests.cs | 2 +- src/Tasks.UnitTests/ResolveComReference_Tests.cs | 2 +- .../AssemblyDependency/ResolveAssemblyReference.cs | 2 +- src/Tasks/RegisterAssembly.cs | 2 +- src/Tasks/ResGenDependencies.cs | 2 +- src/Tasks/ResolveComReference.cs | 2 +- src/Tasks/StateFileBase.cs | 11 +++++------ src/Tasks/SystemState.cs | 2 +- src/Tasks/UnregisterAssembly.cs | 2 +- 10 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceCacheSerialization.cs b/src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceCacheSerialization.cs index 8c5291e2f7a..08e201918af 100644 --- a/src/Tasks.UnitTests/AssemblyDependency/ResolveAssemblyReferenceCacheSerialization.cs +++ b/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(_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(_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(_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(file.Path, null); } Dictionary cache2 = sysState2.instanceLocalFileStateCache; diff --git a/src/Tasks.UnitTests/AssemblyRegistrationCache_Tests.cs b/src/Tasks.UnitTests/AssemblyRegistrationCache_Tests.cs index 8cf97318e7a..58e3021af56 100644 --- a/src/Tasks.UnitTests/AssemblyRegistrationCache_Tests.cs +++ b/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(file.Path, null); } arc2._assemblies.Count.ShouldBe(arc._assemblies.Count); diff --git a/src/Tasks.UnitTests/ResolveComReference_Tests.cs b/src/Tasks.UnitTests/ResolveComReference_Tests.cs index f71b3383a34..df44657497f 100644 --- a/src/Tasks.UnitTests/ResolveComReference_Tests.cs +++ b/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(file.Path, null); } cache2.tlbImpLocation.ShouldBe(cache.tlbImpLocation); diff --git a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs index 8e265e6e0c4..14375c70dea 100644 --- a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs +++ b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs @@ -2036,7 +2036,7 @@ private void LogConflict(Reference reference, string fusionName, StringBuilder l /// internal void ReadStateFile(FileExists fileExists) { - _cache = SystemState.DeserializeCache(_stateFile, Log, typeof(SystemState)) as SystemState; + _cache = SystemState.DeserializeCache(_stateFile, Log); // Construct the cache only if we can't find any caches. if (_cache == null && AssemblyInformationCachePaths != null && AssemblyInformationCachePaths.Length > 0) diff --git a/src/Tasks/RegisterAssembly.cs b/src/Tasks/RegisterAssembly.cs index 3ce50fcde06..0fb8e616bb3 100644 --- a/src/Tasks/RegisterAssembly.cs +++ b/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(AssemblyListFile.ItemSpec, Log) ?? new AssemblyRegistrationCache(); } diff --git a/src/Tasks/ResGenDependencies.cs b/src/Tasks/ResGenDependencies.cs index 2a3c042cff0..68555db9b53 100644 --- a/src/Tasks/ResGenDependencies.cs +++ b/src/Tasks/ResGenDependencies.cs @@ -200,7 +200,7 @@ internal override void SerializeCache(string stateFile, TaskLoggingHelper log) /// internal static ResGenDependencies DeserializeCache(string stateFile, bool useSourcePath, TaskLoggingHelper log) { - var retVal = (ResGenDependencies)DeserializeCache(stateFile, log, typeof(ResGenDependencies)) ?? new ResGenDependencies(); + var retVal = DeserializeCache(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 diff --git a/src/Tasks/ResolveComReference.cs b/src/Tasks/ResolveComReference.cs index 0c7348a73ac..0a3eb949eef 100644 --- a/src/Tasks/ResolveComReference.cs +++ b/src/Tasks/ResolveComReference.cs @@ -333,7 +333,7 @@ public override bool Execute() allProjectRefs = new List(); allDependencyRefs = new List(); - _timestampCache = (ResolveComReferenceCache)StateFileBase.DeserializeCache(StateFile, Log, typeof(ResolveComReferenceCache)); + _timestampCache = StateFileBase.DeserializeCache(StateFile, Log); if (_timestampCache?.ToolPathsMatchCachePaths(_tlbimpPath, _aximpPath) != true) { diff --git a/src/Tasks/StateFileBase.cs b/src/Tasks/StateFileBase.cs index 6b18c794e8f..d24b23dbab8 100644 --- a/src/Tasks/StateFileBase.cs +++ b/src/Tasks/StateFileBase.cs @@ -67,9 +67,9 @@ internal virtual void SerializeCache(string stateFile, TaskLoggingHelper log) /// /// Reads the specified file from disk into a StateFileBase derived object. /// - internal static StateFileBase DeserializeCache(string stateFile, TaskLoggingHelper log, Type requiredReturnType) + internal static T DeserializeCache(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 @@ -90,21 +90,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; } } } diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index 9e174d054ca..f01288b7c9a 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -527,7 +527,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(stateFile.ToString(), log); if (sysState == null) { continue; diff --git a/src/Tasks/UnregisterAssembly.cs b/src/Tasks/UnregisterAssembly.cs index fb74aec9202..4c297df27fd 100644 --- a/src/Tasks/UnregisterAssembly.cs +++ b/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(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) From a0187fedf936d1ac195ef242cb69db3f97b6271f Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Mon, 22 May 2023 13:55:49 +0200 Subject: [PATCH 2/3] Don't persist FileState of immutable files to disk --- .../ResolveAssemblyReference.cs | 2 +- src/Tasks/SystemState.cs | 43 +++++++++++++++---- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs index 14375c70dea..548c14c4efb 100644 --- a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs +++ b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs @@ -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); }); diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index f01288b7c9a..ba56a2017ab 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -210,6 +210,16 @@ internal FrameworkName FrameworkNameAttribute get { return frameworkName; } set { frameworkName = value; } } + + /// + /// The last-modified value to use for immutable framework files which we don't do I/O on. + /// + internal static DateTime ImmutableFileLastModifiedMarker => DateTime.MaxValue; + + /// + /// It is wasteful to persist entries for immutable framework files. + /// + internal bool IsWorthPersisting => lastModified != ImmutableFileLastModifiedMarker; } /// @@ -258,7 +268,7 @@ public override void Translate(ITranslator translator) } /// - /// Flag that indicates + /// Flag that indicates that has been modified. /// /// internal bool IsDirty @@ -339,7 +349,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 + // .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, @@ -368,8 +378,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; @@ -399,9 +409,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; } @@ -450,7 +466,10 @@ private AssemblyNameExtension GetAssemblyName(string path) { fileState.Assembly = AssemblyNameExtension.UnnamedAssembly; } - isDirty = true; + if (fileState.IsWorthPersisting) + { + isDirty = true; + } } if (fileState.Assembly.IsUnnamedAssembly) @@ -471,7 +490,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; @@ -503,7 +525,10 @@ private void GetAssemblyMetadata( out fileState.scatterFiles, out fileState.frameworkName); - isDirty = true; + if (fileState.IsWorthPersisting) + { + isDirty = true; + } } dependencies = fileState.dependencies; From 49d73e4b098867bcd3f77d1727c2174498bee4a0 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 26 May 2023 14:51:02 +0200 Subject: [PATCH 3/3] Add test assert --- src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs b/src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs index f747e27ebb0..b700ec64db3 100644 --- a/src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs +++ b/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]