Merge pull request #73990 from CyrusNajmabadi/metadataWrites

Author: CyrusNajmabadi

src/VisualStudio/Core/Def/ProjectSystem/MetadataReferences/VisualStudioMetadataReferenceManager.cs

@@ -14,7 +14,6 @@
 using System.Runtime.InteropServices;
 using System.Threading;
 using Microsoft.CodeAnalysis;
-using Microsoft.CodeAnalysis.Collections.Internal;
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Shared.Collections;
 using Microsoft.VisualStudio.Shell.Interop;
@@ -48,9 +47,12 @@ internal sealed partial class VisualStudioMetadataReferenceManager : IWorkspaceS
     private readonly object _metadataCacheLock = new();
 
     /// <summary>
-    /// Access locked with <see cref="_metadataCacheLock"/>.
+    /// Access locked with <see cref="_metadataCacheLock"/>.  Maps from file path to metadata and the last time the
+    /// metadata was written to.  We keep this around until we see the file has changed on disk, at which point we'll
+    /// compute the new metadata and update this cache, allowing the old metadata to be released.  Note: this does mean
+    /// that metadata that is no longer used, will be kept around indefinitely.
     /// </summary>
-    private readonly Dictionary<FileKey, WeakReference<AssemblyMetadata>> _metadataCache = [];
+    private readonly Dictionary<string, (DateTime lastWriteTime, AssemblyMetadata metadata)> _metadataCache = new(StringComparer.OrdinalIgnoreCase);
 
     private readonly ImmutableArray<string> _runtimeDirectories;
     private readonly TemporaryStorageService _temporaryStorageService;
@@ -90,22 +92,26 @@ public void Dispose()
         }
     }
 
-    private bool TryGetMetadata(FileKey key, [NotNullWhen(true)] out AssemblyMetadata? metadata)
+    private bool TryGetMetadata(string filePath, DateTime lastWriteTime, [NotNullWhen(true)] out AssemblyMetadata? metadata)
     {
-        metadata = null;
         lock (_metadataCacheLock)
         {
-            return _metadataCache.TryGetValue(key, out var weakMetadata) &&
-                   weakMetadata.TryGetTarget(out metadata);
+            if (_metadataCache.TryGetValue(filePath, out var tuple) &&
+                tuple.lastWriteTime == lastWriteTime)
+            {
+                metadata = tuple.metadata;
+                return true;
+            }
         }
+
+        metadata = null;
+        return false;
     }
 
     public IReadOnlyList<TemporaryStorageStreamHandle>? GetStorageHandles(string fullPath, DateTime snapshotTimestamp)
     {
-        var key = new FileKey(fullPath, snapshotTimestamp);
-        // check existing metadata
-        if (TryGetMetadata(key, out var source) &&
-            s_metadataToStorageHandles.TryGetValue(source, out var handles))
+        if (TryGetMetadata(fullPath, snapshotTimestamp, out var metadata) &&
+            s_metadataToStorageHandles.TryGetValue(metadata, out var handles))
         {
             return handles;
         }
@@ -143,10 +149,8 @@ private static ImmutableArray<string> GetRuntimeDirectories()
     /// <exception cref="BadImageFormatException" />
     internal Metadata GetMetadata(string fullPath, DateTime snapshotTimestamp)
     {
-        var key = new FileKey(fullPath, snapshotTimestamp);
-
         // check existing metadata
-        if (!TryGetMetadata(key, out var metadata))
+        if (!TryGetMetadata(fullPath, snapshotTimestamp, out var metadata))
         {
             // wasn't in the cache.  create a new instance.
             metadata = GetMetadataWorker(fullPath);
@@ -157,16 +161,14 @@ internal Metadata GetMetadata(string fullPath, DateTime snapshotTimestamp)
                 // Now try to create and add the metadata to the cache. If we fail to add it (because some other thread
                 // beat us to this), then Dispose the metadata we just created and will return the existing metadata
                 // instead.
-                if (_metadataCache.TryGetValue(key, out var weakCachedMetadata) &&
-                    weakCachedMetadata.TryGetTarget(out var cachedMetadata))
+                if (TryGetMetadata(fullPath, snapshotTimestamp, out var cachedMetadata))
                 {
                     metadata.Dispose();
                     return cachedMetadata;
                 }
 
-                // don't use "Add" since key might already exist with already released metadata
-                _metadataCache[key] = new WeakReference<AssemblyMetadata>(metadata);
-                ClearReleasedMetadata_NoLock();
+                // don't use "Add" since key might already exist with stale metadata
+                _metadataCache[fullPath] = (snapshotTimestamp, metadata);
                 return metadata;
             }
         }
@@ -184,29 +186,6 @@ AssemblyMetadata GetMetadataWorker(string fullPath)
 
             return metadata;
         }
-
-        void ClearReleasedMetadata_NoLock()
-        {
-            Contract.ThrowIfFalse(Monitor.IsEntered(_metadataCacheLock));
-
-            var count = _metadataCache.Count;
-
-            // Cleanup when we hit powers of two.  This way we don't have to walk too often.  But only when we've really
-            // grown. the dictionary by a substantial amount since the last time we cleaned up.
-            if (count == (1 << SegmentedArraySortUtils.Log2((uint)count)))
-            {
-                using var keysToRemove = TemporaryArray<FileKey>.Empty;
-
-                foreach (var (fileKey, weakMetadata) in _metadataCache)
-                {
-                    if (!weakMetadata.TryGetTarget(out _))
-                        keysToRemove.Add(fileKey);
-                }
-
-                foreach (var key in keysToRemove)
-                    _metadataCache.Remove(key);
-            }
-        }
     }
 
     private static (ModuleMetadata metadata, TemporaryStorageStreamHandle storageHandle) GetMetadataFromTemporaryStorage(
@@ -362,9 +341,9 @@ private static (AssemblyMetadata assemblyMetadata, IReadOnlyList<TemporaryStorag
             assemblyDir ??= Path.GetDirectoryName(fullPath);
 
             // Suppression should be removed or addressed https://github.com/dotnet/roslyn/issues/41636
-            var moduleFileKey = PathUtilities.CombineAbsoluteAndRelativePaths(assemblyDir, moduleName)!;
+            var moduleFullPath = PathUtilities.CombineAbsoluteAndRelativePaths(assemblyDir, moduleName)!;
 
-            var (moduleMetadata, moduleHandle) = moduleMetadataFactory(moduleFileKey);
+            var (moduleMetadata, moduleHandle) = moduleMetadataFactory(moduleFullPath);
             modules.Add(moduleMetadata);
             handles.Add(moduleHandle);
         }