Revert "ResolveAssemblyReference CPU optimizations (#8916)" (#9037)

This reverts commit 1ff019a.

Fixes #
VS Insertion, C++ Project System - PR: VC.ProjectSystem.VCProject.CachedProjectAutomation fails since these changes.
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_git/VS/pullRequest/483186#1689247828

So far I know the problem is in 1ff019a#diff-20c3f77300d596d35264f7f9351652e233cb3455c3f7a7262df4842d437fe8efR1407-R1414 but let's revert for now to unblock insertions. Thank you!

Author: JaynieBai

src/Build.OM.UnitTests/Instance/ProjectItemInstance_Tests.cs

@@ -95,49 +95,6 @@ public void AccessorsWithMetadata()
             Assert.Equal("v2", item.GetMetadataValue("m2"));
         }
 
-        /// <summary>
-        /// Basic ProjectItemInstance with metadata added using ImportMetadata
-        /// </summary>
-        [Fact]
-        public void AccessorsWithImportedMetadata()
-        {
-            ProjectItemInstance item = GetItemInstance();
-
-            ((IMetadataContainer)item).ImportMetadata(new Dictionary<string, string>
-            {
-                { "m1", "v1" },
-                { "m2", "v2" },
-            });
-
-            Assert.Equal("m1", item.GetMetadata("m1").Name);
-            Assert.Equal("m2", item.GetMetadata("m2").Name);
-            Assert.Equal("v1", item.GetMetadataValue("m1"));
-            Assert.Equal("v2", item.GetMetadataValue("m2"));
-        }
-
-        /// <summary>
-        /// ImportMetadata adds and overwrites metadata, does not delete existing metadata
-        /// </summary>
-        [Fact]
-        public void ImportMetadataAddsAndOverwrites()
-        {
-            ProjectItemInstance item = GetItemInstance();
-
-            item.SetMetadata("m1", "v1");
-            item.SetMetadata("m2", "v0");
-
-            ((IMetadataContainer) item).ImportMetadata(new Dictionary<string, string>
-            {
-                { "m2", "v2" },
-                { "m3", "v3" },
-            });
-
-            // m1 was not deleted, m2 was overwritten, m3 was added
-            Assert.Equal("v1", item.GetMetadataValue("m1"));
-            Assert.Equal("v2", item.GetMetadataValue("m2"));
-            Assert.Equal("v3", item.GetMetadataValue("m3"));
-        }
-
         /// <summary>
         /// Get metadata not present
         /// </summary>

src/Build/Instance/ProjectItemInstance.cs

@@ -521,8 +521,6 @@ IDictionary ITaskItem2.CloneCustomMetadataEscaped()
 
         IEnumerable<KeyValuePair<string, string>> IMetadataContainer.EnumerateMetadata() => _taskItem.EnumerateMetadata();
 
-        void IMetadataContainer.ImportMetadata(IEnumerable<KeyValuePair<string, string>> metadata) => _taskItem.ImportMetadata(metadata);
-
         #region IMetadataTable Members
 
         /// <summary>
@@ -1036,19 +1034,6 @@ public IEnumerable<KeyValuePair<string, string>> EnumerateMetadata()
                 }
             }
 
-            /// <summary>
-            /// Sets the given metadata.
-            /// Equivalent to calling <see cref="SetMetadata(string,string)"/> for each item in <paramref name="metadata"/>.
-            /// </summary>
-            /// <param name="metadata">The metadata to set.</param>
-            public void ImportMetadata(IEnumerable<KeyValuePair<string, string>> metadata)
-            {
-                ProjectInstance.VerifyThrowNotImmutable(_isImmutable);
-
-                _directMetadata ??= new CopyOnWritePropertyDictionary<ProjectMetadataInstance>();
-                _directMetadata.ImportProperties(metadata.Select(kvp => new ProjectMetadataInstance(kvp.Key, kvp.Value, allowItemSpecModifiers: true)));
-            }
-
             /// <summary>
             /// Used to return metadata from another AppDomain. Can't use yield return because the
             /// generated state machine is not marked as [Serializable], so we need to allocate.
@@ -1386,7 +1371,9 @@ public void CopyMetadataTo(ITaskItem destinationItem, bool addOriginalItemSpec)
                     originalItemSpec = destinationItem.GetMetadata("OriginalItemSpec");
                 }
 
-                if (destinationItem is TaskItem destinationAsTaskItem && destinationAsTaskItem._directMetadata == null)
+                TaskItem destinationAsTaskItem = destinationItem as TaskItem;
+
+                if (destinationAsTaskItem != null && destinationAsTaskItem._directMetadata == null)
                 {
                     ProjectInstance.VerifyThrowNotImmutable(destinationAsTaskItem._isImmutable);
 
@@ -1404,14 +1391,6 @@ public void CopyMetadataTo(ITaskItem destinationItem, bool addOriginalItemSpec)
                         destinationAsTaskItem._itemDefinitions.AddRange(_itemDefinitions);
                     }
                 }
-                else if (destinationItem is IMetadataContainer destinationItemAsMetadataContainer)
-                {
-                    // The destination implements IMetadataContainer so we can use the ImportMetadata bulk-set operation.
-                    IEnumerable<ProjectMetadataInstance> metadataEnumerable = MetadataCollection;
-                    destinationItemAsMetadataContainer.ImportMetadata(metadataEnumerable
-                        .Where(metadatum => string.IsNullOrEmpty(destinationItem.GetMetadata(metadatum.Name)))
-                        .Select(metadatum => new KeyValuePair<string, string>(metadatum.Name, GetMetadataEscaped(metadatum.Name))));
-                }
                 else
                 {
                     // OK, most likely the destination item was a Microsoft.Build.Utilities.TaskItem.

src/Framework/IMetadataContainer.cs

@@ -20,17 +20,5 @@ internal interface IMetadataContainer
         /// in the binary logger.
         /// </summary>
         IEnumerable<KeyValuePair<string, string>> EnumerateMetadata();
-
-        /// <summary>
-        /// Sets the given metadata. The operation is equivalent to calling
-        /// <see cref="ITaskItem.SetMetadata"/> on all metadata, but takes
-        /// advantage of a faster bulk-set operation where applicable. The
-        /// implementation may not perform the same parameter validation
-        /// as SetMetadata.
-        /// </summary>
-        /// <param name="metadata">The metadata to set. The keys are assumed
-        /// to be unique and values are assumed to be escaped.
-        /// </param>
-        void ImportMetadata(IEnumerable<KeyValuePair<string, string>> metadata);
     }
 }

src/Framework/TaskItemData.cs

@@ -49,9 +49,6 @@ public TaskItemData(ITaskItem original)
 
         IEnumerable<KeyValuePair<string, string>> IMetadataContainer.EnumerateMetadata() => Metadata;
 
-        void IMetadataContainer.ImportMetadata(IEnumerable<KeyValuePair<string, string>> metadata)
-            => throw new InvalidOperationException($"{nameof(TaskItemData)} does not support write operations");
-
         public int MetadataCount => Metadata.Count;
 
         public ICollection MetadataNames => (ICollection)Metadata.Keys;

src/Shared/TaskParameter.cs

@@ -856,14 +856,6 @@ private IEnumerable<KeyValuePair<string, string>> EnumerateMetadataLazy()
                     yield return unescaped;
                 }
             }
-
-            public void ImportMetadata(IEnumerable<KeyValuePair<string, string>> metadata)
-            {
-                foreach (KeyValuePair<string, string> kvp in metadata)
-                {
-                    SetMetadata(kvp.Key, kvp.Value);
-                }
-            }
         }
     }
 }

src/Tasks/AssemblyDependency/ReferenceTable.cs

@@ -852,7 +852,8 @@ private static AssemblyNameExtension GetAssemblyNameFromItemMetadata(ITaskItem i
                 name = item.GetMetadata(FileUtilities.ItemSpecModifiers.Filename);
             }
 
-            return new AssemblyNameExtension($"{name}, Version={version}, Culture=neutral, PublicKeyToken={publicKeyToken}");
+            AssemblyName assemblyName = new AssemblyName($"{name}, Version={version}, Culture=neutral, PublicKeyToken={publicKeyToken}");
+            return new AssemblyNameExtension(assemblyName);
         }
 
         /// <summary>
@@ -2676,9 +2677,36 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem>
             // Set up the main item.
             TaskItem referenceItem = new TaskItem();
             referenceItem.ItemSpec = reference.FullPath;
+            referenceItem.SetMetadata(ItemMetadataNames.resolvedFrom, reference.ResolvedSearchPath);
 
-            IMetadataContainer referenceItemAsMetadataContainer = referenceItem;
-            referenceItemAsMetadataContainer.ImportMetadata(EnumerateCommonMetadata());
+            // Set the CopyLocal metadata.
+            referenceItem.SetMetadata(ItemMetadataNames.copyLocal, reference.IsCopyLocal ? "true" : "false");
+
+            // Set the Redist name metadata.
+            if (!String.IsNullOrEmpty(reference.RedistName))
+            {
+                referenceItem.SetMetadata(ItemMetadataNames.redist, reference.RedistName);
+            }
+
+            if (Reference.IsFrameworkFile(reference.FullPath, _frameworkPaths) || (_installedAssemblies?.FrameworkAssemblyEntryInRedist(assemblyName) == true))
+            {
+                if (!IsAssemblyRemovedFromDotNetFramework(assemblyName, reference.FullPath, _frameworkPaths, _installedAssemblies))
+                {
+                    referenceItem.SetMetadata(ItemMetadataNames.frameworkFile, "true");
+                }
+            }
+
+            if (!String.IsNullOrEmpty(reference.ImageRuntime))
+            {
+                referenceItem.SetMetadata(ItemMetadataNames.imageRuntime, reference.ImageRuntime);
+            }
+
+            // The redist root is "null" when there was no IsRedistRoot flag in the Redist XML
+            // (or there was no redist XML at all for this item).
+            if (reference.IsRedistRoot != null)
+            {
+                referenceItem.SetMetadata(ItemMetadataNames.isRedistRoot, (bool)reference.IsRedistRoot ? "true" : "false");
+            }
 
             // If there was a primary source item, then forward metadata from it.
             // It's important that the metadata from the primary source item
@@ -2852,45 +2880,13 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem>
             // nonForwardableMetadata should be null here if relatedFileExtensions, satellites, serializationAssemblyFiles, and scatterFiles were all empty.
             if (nonForwardableMetadata != null)
             {
-                referenceItemAsMetadataContainer.ImportMetadata(nonForwardableMetadata);
-            }
-
-            return referenceItem;
-
-            // Enumerate common metadata with an iterator to allow using a more efficient bulk-set operation.
-            IEnumerable<KeyValuePair<string, string>> EnumerateCommonMetadata()
-            {
-                yield return new KeyValuePair<string, string>(ItemMetadataNames.resolvedFrom, reference.ResolvedSearchPath);
-
-                // Set the CopyLocal metadata.
-                yield return new KeyValuePair<string, string>(ItemMetadataNames.copyLocal, reference.IsCopyLocal ? "true" : "false");
-
-                // Set the Redist name metadata.
-                if (!string.IsNullOrEmpty(reference.RedistName))
+                foreach (KeyValuePair<string, string> kvp in nonForwardableMetadata)
                 {
-                    yield return new KeyValuePair<string, string>(ItemMetadataNames.redist, reference.RedistName);
-                }
-
-                if (Reference.IsFrameworkFile(reference.FullPath, _frameworkPaths) || (_installedAssemblies?.FrameworkAssemblyEntryInRedist(assemblyName) == true))
-                {
-                    if (!IsAssemblyRemovedFromDotNetFramework(assemblyName, reference.FullPath, _frameworkPaths, _installedAssemblies))
-                    {
-                        yield return new KeyValuePair<string, string>(ItemMetadataNames.frameworkFile, "true");
-                    }
-                }
-
-                if (!string.IsNullOrEmpty(reference.ImageRuntime))
-                {
-                    yield return new KeyValuePair<string, string>(ItemMetadataNames.imageRuntime, reference.ImageRuntime);
-                }
-
-                // The redist root is "null" when there was no IsRedistRoot flag in the Redist XML
-                // (or there was no redist XML at all for this item).
-                if (reference.IsRedistRoot != null)
-                {
-                    yield return new KeyValuePair<string, string>(ItemMetadataNames.isRedistRoot, (bool)reference.IsRedistRoot ? "true" : "false");
+                    referenceItem.SetMetadata(kvp.Key, kvp.Value);
                 }
             }
+
+            return referenceItem;
         }
 
         /// <summary>

src/Utilities.UnitTests/TaskItem_Tests.cs

@@ -324,25 +324,6 @@ public void SetNullMetadataValue()
             item.GetMetadata("m").ShouldBe(string.Empty);
         }
 
-        [Fact]
-        public void ImplementsIMetadataContainer()
-        {
-            Dictionary<string, string> metadata = new()
-            {
-                { "a", "a1" },
-                { "b", "b1" },
-            };
-
-            TaskItem item = new TaskItem("foo");
-            IMetadataContainer metadataContainer = (IMetadataContainer)item;
-
-            metadataContainer.ImportMetadata(metadata);
-
-            var actualMetadata = metadataContainer.EnumerateMetadata().OrderBy(metadata => metadata.Key).ToList();
-            var expectedMetadata = metadata.OrderBy(metadata => metadata.Value).ToList();
-            Assert.True(actualMetadata.SequenceEqual(expectedMetadata));
-        }
-
 #if FEATURE_APPDOMAIN
         /// <summary>
         /// Test that task items can be successfully constructed based on a task item from another appdomain.  

src/Utilities/TaskItem.cs

@@ -480,12 +480,6 @@ IEnumerable<KeyValuePair<string, string>> IMetadataContainer.EnumerateMetadata()
             return EnumerateMetadataLazy();
         }
 
-        void IMetadataContainer.ImportMetadata(IEnumerable<KeyValuePair<string, string>> metadata)
-        {
-            _metadata ??= new CopyOnWriteDictionary<string>(MSBuildNameIgnoreCaseComparer.Default);
-            _metadata.SetItems(metadata.Select(kvp => new KeyValuePair<string, string>(kvp.Key, kvp.Value ?? string.Empty)));
-        }
-
         private IEnumerable<KeyValuePair<string, string>> EnumerateMetadataEager()
         {
             if (_metadata == null)