diff --git a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs index b03b2570f19..fce08167159 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs @@ -387,7 +387,7 @@ private void MergeScopeIntoLastScope() { foreach (KeyValuePair> kvp in PrimaryRemoveTable) { - _baseItems.RemoveItems(kvp.Value); + _baseItems.RemoveItemsOfType(kvp.Key, kvp.Value); } } @@ -974,14 +974,11 @@ private void ApplyModificationsToTable(IItemDictionary tabl ICollection existing = table[itemType]; if (existing != null) { - foreach (var kvPair in modify) + foreach (ProjectItemInstance item in existing) { - if (table.Contains(kvPair.Key)) + if (modify.TryGetValue(item, out MetadataModifications modificationsToApply)) { - var itemToModify = kvPair.Key; - var modificationsToApply = kvPair.Value; - - ApplyMetadataModificationsToItem(modificationsToApply, itemToModify); + ApplyMetadataModificationsToItem(modificationsToApply, item); } } } diff --git a/src/Build/Collections/IItemDictionary.cs b/src/Build/Collections/IItemDictionary.cs index 5977f324617..2019e4bb966 100644 --- a/src/Build/Collections/IItemDictionary.cs +++ b/src/Build/Collections/IItemDictionary.cs @@ -54,11 +54,6 @@ internal interface IItemDictionary : IEnumerable, IItemProvider /// void EnumerateItemsPerType(Action> itemTypeCallback); - /// - /// Whether the provided item is in this table or not. - /// - bool Contains(T projectItem); - /// /// Add a new item to the collection, at the /// end of the list of other items with its key. @@ -92,7 +87,8 @@ internal interface IItemDictionary : IEnumerable, IItemProvider /// /// Remove the set of items specified from this dictionary /// + /// The item type for all removes. /// An enumerator over the items to remove. - void RemoveItems(IEnumerable other); + void RemoveItemsOfType(string itemType, IEnumerable other); } } diff --git a/src/Build/Collections/ItemDictionary.cs b/src/Build/Collections/ItemDictionary.cs index 3a35b70ccce..952d84bf3a1 100644 --- a/src/Build/Collections/ItemDictionary.cs +++ b/src/Build/Collections/ItemDictionary.cs @@ -38,14 +38,7 @@ internal sealed class ItemDictionary : ICollection, IItemDictionary /// Dictionary of item lists used as a backing store. /// This collection provides quick access to the ordered set of items of a particular type. /// - private readonly Dictionary> _itemLists; - - /// - /// Dictionary of items in the collection, to speed up Contains, - /// Remove, and Replace. For those operations, we look up here, - /// then modify the other dictionary to match. - /// - private readonly Dictionary> _nodes; + private readonly Dictionary> _itemLists; /// /// Constructor for an empty collection. @@ -53,8 +46,7 @@ internal sealed class ItemDictionary : ICollection, IItemDictionary public ItemDictionary() { // Tracing.Record("new item dictionary"); - _itemLists = new Dictionary>(MSBuildNameIgnoreCaseComparer.Default); - _nodes = new Dictionary>(); + _itemLists = new Dictionary>(MSBuildNameIgnoreCaseComparer.Default); } /// @@ -64,8 +56,7 @@ public ItemDictionary() public ItemDictionary(int initialItemTypesCapacity, int initialItemsCapacity = 0) { // Tracing.Record("new item dictionary"); - _itemLists = new Dictionary>(initialItemTypesCapacity, MSBuildNameIgnoreCaseComparer.Default); - _nodes = new Dictionary>(initialItemsCapacity); + _itemLists = new Dictionary>(initialItemTypesCapacity, MSBuildNameIgnoreCaseComparer.Default); } /// @@ -74,15 +65,29 @@ public ItemDictionary(int initialItemTypesCapacity, int initialItemsCapacity = 0 public ItemDictionary(IEnumerable items) { // Tracing.Record("new item dictionary"); - _itemLists = new Dictionary>(MSBuildNameIgnoreCaseComparer.Default); - _nodes = new Dictionary>(); + _itemLists = new Dictionary>(MSBuildNameIgnoreCaseComparer.Default); ImportItems(items); } /// /// Number of items in total, for debugging purposes. /// - public int Count => _nodes.Count; + public int Count + { + get + { + int count = 0; + lock (_itemLists) + { + foreach (List list in _itemLists.Values) + { + count += list.Count; + } + } + + return count; + } + } /// /// Get the item types that have at least one item in this collection @@ -117,7 +122,7 @@ public ICollection this[string itemtype] { get { - LinkedList list; + List list; lock (_itemLists) { if (!_itemLists.TryGetValue(itemtype, out list)) @@ -137,13 +142,12 @@ public void Clear() { lock (_itemLists) { - foreach (ICollection list in _itemLists.Values) + foreach (List list in _itemLists.Values) { list.Clear(); } _itemLists.Clear(); - _nodes.Clear(); } } @@ -227,11 +231,19 @@ public ICollection GetItems(string itemType) /// /// Whether the provided item is in this table or not. /// - public bool Contains(T projectItem) + bool ICollection.Contains(T projectItem) { lock (_itemLists) { - return _nodes.ContainsKey(projectItem); + foreach (List list in _itemLists.Values) + { + if (list.Contains(projectItem)) + { + return true; + } + } + + return false; } } @@ -259,23 +271,31 @@ public bool Remove(T projectItem) { lock (_itemLists) { - if (!_nodes.TryGetValue(projectItem, out LinkedListNode node)) + if (!_itemLists.TryGetValue(projectItem.Key, out List list)) { return false; } - LinkedList list = node.List; - list.Remove(node); - _nodes.Remove(projectItem); - - // Save memory - if (list.Count == 0) + // Searching for a single object - just compare the reference pointer. + for (int i = 0; i < list.Count; i++) { - _itemLists.Remove(projectItem.Key); - } + T candidateItem = list[i]; + if (ReferenceEquals(candidateItem, projectItem)) + { + list.RemoveAt(i); - return true; + // Save memory if the item type is now empty. + if (list.Count == 0) + { + _itemLists.Remove(projectItem.Key); + } + + return true; + } + } } + + return false; } /// @@ -303,20 +323,21 @@ public void ImportItemsOfType(string itemType, IEnumerable items) { lock (_itemLists) { - if (!_itemLists.TryGetValue(itemType, out LinkedList list)) + if (!_itemLists.TryGetValue(itemType, out List list)) { - list = new LinkedList(); + list = new List(); _itemLists[itemType] = list; } - foreach (T item in items) + int count = list.Count; + list.AddRange(items); + + for (int i = count; i < list.Count; i++) { #if DEBUG // Debug only: hot code path - ErrorUtilities.VerifyThrow(String.Equals(itemType, item.Key, StringComparison.OrdinalIgnoreCase), "Item type mismatch"); + ErrorUtilities.VerifyThrow(String.Equals(itemType, list[i].Key, StringComparison.OrdinalIgnoreCase), "Item type mismatch"); #endif - LinkedListNode node = list.AddLast(item); - _nodes.Add(item, node); } } } @@ -324,25 +345,51 @@ public void ImportItemsOfType(string itemType, IEnumerable items) /// /// Remove the set of items specified from this dictionary /// + /// The item type for all removes. /// An enumerator over the items to remove. - public void RemoveItems(IEnumerable other) + public void RemoveItemsOfType(string itemType, IEnumerable other) { - foreach (T item in other) + lock (_itemLists) { - Remove(item); + if (!_itemLists.TryGetValue(itemType, out List list)) + { + return; + } + + // Since we'll need to search and remove an unknown number of items, we'll build up a new list of items to + // keep, using the incoming enumerable as a set, and swap out the result at the end. + // This minimizes the upper bound of ops and allocations here. + List listWithRemoves = new(list.Count); + HashSet itemsToRemove = new(other); + foreach (T item in list) + { + if (!itemsToRemove.Contains(item)) + { + listWithRemoves.Add(item); + } + } + + if (listWithRemoves.Count > 0) + { + _itemLists[itemType] = listWithRemoves; + } + else + { + // If the clone is empty, remove the item type from the dictionary + _itemLists.Remove(itemType); + } } } private void AddProjectItem(T projectItem) { - if (!_itemLists.TryGetValue(projectItem.Key, out LinkedList list)) + if (!_itemLists.TryGetValue(projectItem.Key, out List list)) { - list = new LinkedList(); + list = new List(); _itemLists[projectItem.Key] = list; } - LinkedListNode node = list.AddLast(projectItem); - _nodes.Add(projectItem, node); + list.Add(projectItem); } public void CopyTo(T[] array, int arrayIndex) diff --git a/src/Build/Instance/ImmutableProjectCollections/ImmutableItemDictionary.cs b/src/Build/Instance/ImmutableProjectCollections/ImmutableItemDictionary.cs index a05ddb25abb..6876d8618fc 100644 --- a/src/Build/Instance/ImmutableProjectCollections/ImmutableItemDictionary.cs +++ b/src/Build/Instance/ImmutableProjectCollections/ImmutableItemDictionary.cs @@ -21,7 +21,6 @@ internal sealed class ImmutableItemDictionary : IItemDictionary private readonly IDictionary> _itemsByType; private readonly ICollection _allCachedItems; private readonly Func _getInstance; - private readonly Func _getItemType; public ImmutableItemDictionary( ICollection allItems, @@ -37,7 +36,6 @@ public ImmutableItemDictionary( _allCachedItems = allItems; _itemsByType = itemsByType ?? throw new ArgumentNullException(nameof(itemsByType)); _getInstance = getInstance; - _getItemType = getItemType; } /// @@ -66,24 +64,6 @@ public ICollection this[string itemType] /// public void Clear() => throw new NotSupportedException(); - /// - public bool Contains(T projectItem) - { - if (projectItem == null) - { - return false; - } - - string? itemType = _getItemType(projectItem); - if (itemType == null) - { - return false; - } - - ICollection items = GetItems(itemType); - return items.Contains(projectItem); - } - /// public void EnumerateItemsPerType(Action> itemTypeCallback) { @@ -159,7 +139,7 @@ public ICollection GetItems(string itemType) public bool Remove(T projectItem) => throw new NotSupportedException(); /// - public void RemoveItems(IEnumerable other) => throw new NotSupportedException(); + public void RemoveItemsOfType(string itemType, IEnumerable other) => throw new NotSupportedException(); private sealed class ListConverter : ICollection {