From f07a49ff9b2326fd275e06b29073011410f939bb Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Sun, 26 Sep 2021 08:51:09 -0700 Subject: [PATCH 1/6] Stop removing from file state cache This was causing numerous unnecessary cache misses. --- src/Tasks/SystemState.cs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index 2992e07bc73..df59fad47b2 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -366,15 +366,6 @@ private FileState ComputeFileStateFromCachesAndDisk(string path) // If the process-wide cache contains an up-to-date FileState, always use it if (isProcessFileStateUpToDate) { - // If a FileState already exists in this instance cache due to deserialization, remove it; - // another instance has taken responsibility for serialization, and keeping this would - // result in multiple instances serializing the same data to disk - if (isCachedInInstance) - { - instanceLocalFileStateCache.Remove(path); - isDirty = true; - } - return cachedProcessFileState; } // If the process-wide FileState is missing or out-of-date, this instance owns serialization; From 0e7c6ea9ef7d62c365d7b826455a29b90567f8ee Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Sun, 26 Sep 2021 08:51:17 -0700 Subject: [PATCH 2/6] Tiny clarity tweak --- src/Tasks/AssemblyDependency/ReferenceTable.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/Tasks/AssemblyDependency/ReferenceTable.cs b/src/Tasks/AssemblyDependency/ReferenceTable.cs index 196a70b8747..3b214c55a9f 100644 --- a/src/Tasks/AssemblyDependency/ReferenceTable.cs +++ b/src/Tasks/AssemblyDependency/ReferenceTable.cs @@ -741,13 +741,8 @@ out string redistName /// private static void TryConvertToAssemblyName(string itemSpec, string fusionName, ref AssemblyNameExtension assemblyName) { - // FusionName is used if available. - string finalName = fusionName; - if (string.IsNullOrEmpty(finalName)) - { - // Otherwise, its itemSpec. - finalName = itemSpec; - } + // FusionName is used if available; otherwise use itemspec. + string finalName = string.IsNullOrEmpty(fusionName) ? itemSpec : fusionName; bool pathRooted = false; try From a970a3df29daa7d2a29a845d092543b36e61b83e Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Fri, 1 Oct 2021 08:54:48 -0700 Subject: [PATCH 3/6] Add to file cache --- src/Tasks/SystemState.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index df59fad47b2..b68cb5f8b5f 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -366,6 +366,10 @@ private FileState ComputeFileStateFromCachesAndDisk(string path) // If the process-wide cache contains an up-to-date FileState, always use it if (isProcessFileStateUpToDate) { + if (!isInstanceFileStateUpToDate) + { + instanceLocalFileStateCache[path] = cachedProcessFileState; + } return cachedProcessFileState; } // If the process-wide FileState is missing or out-of-date, this instance owns serialization; From 49e92636b4406f8147acaebe952f0a38289f57a4 Mon Sep 17 00:00:00 2001 From: Forgind Date: Fri, 1 Oct 2021 11:29:19 -0700 Subject: [PATCH 4/6] Update src/Tasks/SystemState.cs --- src/Tasks/SystemState.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index b68cb5f8b5f..f018d2044c7 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -369,6 +369,7 @@ private FileState ComputeFileStateFromCachesAndDisk(string path) if (!isInstanceFileStateUpToDate) { instanceLocalFileStateCache[path] = cachedProcessFileState; + isDirty = true; } return cachedProcessFileState; } From 749a96228c6dff19533a9c2707299261a05d3769 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Mon, 4 Oct 2021 09:26:46 -0700 Subject: [PATCH 5/6] Change comments --- src/Tasks/SystemState.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index b68cb5f8b5f..5656518499b 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -366,14 +366,13 @@ 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) { instanceLocalFileStateCache[path] = cachedProcessFileState; } return cachedProcessFileState; } - // If the process-wide FileState is missing or out-of-date, this instance owns serialization; - // sync the process-wide cache and signal other instances to avoid data duplication if (isInstanceFileStateUpToDate) { return s_processWideFileStateCache[path] = cachedInstanceFileState; From 739d9fdbef0912eac0f7a921d040d29f15efa530 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Mon, 11 Oct 2021 16:26:33 -0700 Subject: [PATCH 6/6] Lazily deserialize rar cache --- .../RARPrecomputedCache_Tests.cs | 6 +- .../ResolveAssemblyReference.cs | 24 +++--- src/Tasks/SystemState.cs | 77 ++++++++++++++----- 3 files changed, 70 insertions(+), 37 deletions(-) diff --git a/src/Tasks.UnitTests/RARPrecomputedCache_Tests.cs b/src/Tasks.UnitTests/RARPrecomputedCache_Tests.cs index 11c0a395500..1156b77ead8 100644 --- a/src/Tasks.UnitTests/RARPrecomputedCache_Tests.cs +++ b/src/Tasks.UnitTests/RARPrecomputedCache_Tests.cs @@ -86,7 +86,8 @@ public void StandardCacheTakesPrecedence() // When we read the state file, it should read from the caches produced in a normal build. In this case, // the normal cache does not have dll.dll, whereas the precomputed cache does, so it should not be // present when we read it. - rarReaderTask.ReadStateFile(p => true); + rarReaderTask.InitializeStateFile(p => true); + rarReaderTask._cache.InitializeSystemState(); rarReaderTask._cache.instanceLocalFileStateCache.ShouldNotContainKey(dllName); } } @@ -128,7 +129,8 @@ public void TestPreComputedCacheInputMatchesOutput() // At this point, the standard cache does not exist, so it defaults to reading the "precomputed" cache. // Then we verify that the information contained in that cache matches what we'd expect. - rarReaderTask.ReadStateFile(p => true); + rarReaderTask.InitializeStateFile(p => true); + rarReaderTask._cache.InitializeSystemState(); rarReaderTask._cache.instanceLocalFileStateCache.ShouldContainKey(dllName); SystemState.FileState assembly3 = rarReaderTask._cache.instanceLocalFileStateCache[dllName]; assembly3.Assembly.ShouldBeNull(); diff --git a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs index 2cea34c71bb..35b71ea6a17 100644 --- a/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs +++ b/src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs @@ -2005,20 +2005,9 @@ private void LogConflict(Reference reference, string fusionName, StringBuilder l /// /// Reads the state file (if present) into the cache. /// - internal void ReadStateFile(FileExists fileExists) + internal void InitializeStateFile(FileExists fileExists) { - _cache = SystemState.DeserializeCache(_stateFile, Log, typeof(SystemState)) as SystemState; - - // Construct the cache only if we can't find any caches. - if (_cache == null && AssemblyInformationCachePaths != null && AssemblyInformationCachePaths.Length > 0) - { - _cache = SystemState.DeserializePrecomputedCaches(AssemblyInformationCachePaths, Log, fileExists); - } - - if (_cache == null) - { - _cache = new SystemState(); - } + _cache = new SystemState(_stateFile, AssemblyInformationCachePaths, Log, fileExists); } /// @@ -2026,9 +2015,14 @@ internal void ReadStateFile(FileExists fileExists) /// internal void WriteStateFile() { + if (!_cache.deserializedFromCache) + { + // The cache is empty; don't bother serializing it. + return; + } if (!String.IsNullOrEmpty(AssemblyInformationCacheOutputPath)) { - _cache.SerializePrecomputedCache(AssemblyInformationCacheOutputPath, Log); + _cache.SerializePrecomputedCache(AssemblyInformationCacheOutputPath); } else if (!String.IsNullOrEmpty(_stateFile) && _cache.IsDirty) { @@ -2262,7 +2256,7 @@ ReadMachineTypeFromPEHeader readMachineTypeFromPEHeader } // Load any prior saved state. - ReadStateFile(fileExists); + InitializeStateFile(fileExists); _cache.SetGetLastWriteTime(getLastWriteTime); _cache.SetInstalledAssemblyInformation(installedAssemblyTableInfo); diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index 2c11513b244..8e722f4595a 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -101,6 +101,12 @@ internal sealed class SystemState : StateFileBase, ITranslatable /// private GetAssemblyRuntimeVersion getAssemblyRuntimeVersion; + internal bool deserializedFromCache = false; + private readonly string stateFile; + private readonly ITaskItem[] assemblyInformationCachePaths; + private readonly TaskLoggingHelper log; + private readonly FileExists fileExists; + /// /// Class that holds the current file state. /// @@ -217,6 +223,14 @@ public SystemState() { } + public SystemState(string stateFile, ITaskItem[] assemblyInformationCachePaths, TaskLoggingHelper log, FileExists fileExists) + { + this.stateFile = stateFile; + this.assemblyInformationCachePaths = assemblyInformationCachePaths; + this.log = log; + this.fileExists = fileExists; + } + public SystemState(ITranslator translator) { Translate(translator); @@ -357,30 +371,42 @@ private FileState GetFileState(string path) private FileState ComputeFileStateFromCachesAndDisk(string path) { DateTime lastModified = GetAndCacheLastModified(path); - bool isCachedInInstance = instanceLocalFileStateCache.TryGetValue(path, out FileState cachedInstanceFileState); bool isCachedInProcess = s_processWideFileStateCache.TryGetValue(path, out FileState cachedProcessFileState); - - 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 (isProcessFileStateUpToDate) { - // For the next build, we may be using a different process. Update the file cache. - if (!isInstanceFileStateUpToDate) + if (deserializedFromCache) { - instanceLocalFileStateCache[path] = cachedProcessFileState; - isDirty = true; + bool isCachedInInstance = instanceLocalFileStateCache.TryGetValue(path, out FileState cachedInstanceFileState); + bool isInstanceFileStateUpToDate = isCachedInInstance && lastModified == cachedInstanceFileState.LastModified; + // For the next build, we may be using a different process. Update the file cache. + if (!isInstanceFileStateUpToDate) + { + instanceLocalFileStateCache[path] = cachedProcessFileState; + isDirty = true; + } } + return cachedProcessFileState; } - if (isInstanceFileStateUpToDate) + else { - return s_processWideFileStateCache[path] = cachedInstanceFileState; - } + if (!deserializedFromCache) + { + InitializeSystemState(); + } + bool isCachedInInstance = instanceLocalFileStateCache.TryGetValue(path, out FileState cachedInstanceFileState); + bool isInstanceFileStateUpToDate = isCachedInInstance && lastModified == cachedInstanceFileState.LastModified; + if (isInstanceFileStateUpToDate) + { + return s_processWideFileStateCache[path] = cachedInstanceFileState; + } - // If no up-to-date FileState exists at this point, create one and take ownership - return InitializeFileState(path, lastModified); + // If no up-to-date FileState exists at this point, create one and take ownership + return InitializeFileState(path, lastModified); + } } private DateTime GetAndCacheLastModified(string path) @@ -515,20 +541,32 @@ out fileState.frameworkName frameworkName = fileState.frameworkName; } + internal void InitializeSystemState() + { + SystemState cache = SystemState.DeserializeCache(stateFile, log, typeof(SystemState)) as SystemState; + if (cache is null && assemblyInformationCachePaths?.Length > 0) + { + cache = DeserializePrecomputedCaches(); + } + + if (cache is not null) + { + instanceLocalFileStateCache = cache.instanceLocalFileStateCache; + } + deserializedFromCache = true; + } + /// /// Reads in cached data from stateFiles to build an initial cache. Avoids logging warnings or errors. /// - /// List of locations of caches on disk. - /// How to log - /// Whether a file exists /// A cache representing key aspects of file states. - internal static SystemState DeserializePrecomputedCaches(ITaskItem[] stateFiles, TaskLoggingHelper log, FileExists fileExists) + internal SystemState DeserializePrecomputedCaches() { SystemState retVal = new SystemState(); - retVal.isDirty = stateFiles.Length > 0; + retVal.isDirty = assemblyInformationCachePaths.Length > 0; HashSet assembliesFound = new HashSet(); - foreach (ITaskItem stateFile in stateFiles) + foreach (ITaskItem stateFile in assemblyInformationCachePaths) { // 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; @@ -560,8 +598,7 @@ internal static SystemState DeserializePrecomputedCaches(ITaskItem[] stateFiles, /// Modifies this object to be more portable across machines, then writes it to filePath. /// /// Path to which to write the precomputed cache - /// How to log - internal void SerializePrecomputedCache(string stateFile, TaskLoggingHelper log) + internal void SerializePrecomputedCache(string stateFile) { // 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.