From b3a99d5ba94d75bfbd87578bd6c7ac7273eb15a2 Mon Sep 17 00:00:00 2001 From: Forgind Date: Wed, 13 Oct 2021 12:12:36 -0700 Subject: [PATCH] [RAR] Stop removing from file cache just because an assembly is cached in process (#6891) Fixes #6844 ### Context RAR has several caches, including a process-wide cache and a (serialized) file cache. Previously, we had the process cache take priority (if it was up-to-date) and removed from the serialized cache, believing that meant some other process had taken "ownership" of serializing the assembly, but that doesn't actually make sense because that other process could be the same process currently looking at this assembly, and even if it were another process, that other process could have removed it from its own file cache. This removes that flawed logic so that we always write to the file cache any information we have. This improves incremental build times. ### Changes Made Stopped removing from file cache when process-wide cache has information. ### Testing Tried without this change and noted that the first few attempted cache lookups all missed. Tried with this change without killing processes and noted that they were cache hits; tried with this change after killing processes, and they were still cache hits. Also noted that without this change, there were 0 things in the instanceLocalFileStateCache at the beginning of the first RAR execution. After it, there were 122. All tests conducted with Ocelot repo. ### Notes I actually mentioned that I didn't think it made sense to have this code here in the "questions" section of my RAR pre-caching document [here](https://microsoft-my.sharepoint.com/:w:/p/nmytelka/EfJOiPG6XPJMsDQ12Nk5-HIBm957qVk2DPB0R5XSpRx1RA?e=6bVaU6) but never looked into it more. Ah, well. --- src/Tasks/AssemblyDependency/ReferenceTable.cs | 9 ++------- src/Tasks/SystemState.cs | 11 +++-------- 2 files changed, 5 insertions(+), 15 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 @@ ITaskItem referenceAssemblyName /// 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 diff --git a/src/Tasks/SystemState.cs b/src/Tasks/SystemState.cs index 2992e07bc73..2c11513b244 100644 --- a/src/Tasks/SystemState.cs +++ b/src/Tasks/SystemState.cs @@ -366,19 +366,14 @@ 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) + // For the next build, we may be using a different process. Update the file cache. + if (!isInstanceFileStateUpToDate) { - instanceLocalFileStateCache.Remove(path); + instanceLocalFileStateCache[path] = cachedProcessFileState; isDirty = true; } - 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;