From a8b81cda7d2f0cd6208cf7c83f8f020a2531d247 Mon Sep 17 00:00:00 2001 From: SimaTian Date: Tue, 29 Oct 2024 19:10:53 +0100 Subject: [PATCH 1/2] fixing logging of duplicates for KeepDuplicates=false option. optimizing portion of the code by using a hashset. implementing review comments --- .../BackEnd/IntrinsicTask_Tests.cs | 21 ++++++++++ .../IntrinsicTasks/ItemGroupIntrinsicTask.cs | 27 ++++++++----- .../Components/RequestBuilder/Lookup.cs | 38 ++++++++++++++++--- 3 files changed, 71 insertions(+), 15 deletions(-) diff --git a/src/Build.UnitTests/BackEnd/IntrinsicTask_Tests.cs b/src/Build.UnitTests/BackEnd/IntrinsicTask_Tests.cs index 225ea077970..068809ccc8c 100644 --- a/src/Build.UnitTests/BackEnd/IntrinsicTask_Tests.cs +++ b/src/Build.UnitTests/BackEnd/IntrinsicTask_Tests.cs @@ -314,6 +314,27 @@ public void ItemKeepDuplicatesFalse() Assert.Single(group); } + [Fact] + public void ItemKeepDuplicatesFalseTwoDuplicatesAtOnce() + { + string content = ObjectModelHelpers.CleanupFileContents(""" + + + + + + + + + """); + IntrinsicTask task = CreateIntrinsicTask(content); + Lookup lookup = LookupHelpers.CreateEmptyLookup(); + ExecuteTask(task, lookup); + + var group = lookup.GetItems("i1"); + Assert.Single(group); + } + [Fact] public void ItemKeepDuplicatesAsCondition() { diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs index 2a9cc0e26b0..423679a1f6e 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections; using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; @@ -215,21 +216,27 @@ private void ExecuteAdd(ProjectItemGroupTaskItemInstance child, ItemBucket bucke FileSystems.Default, LoggingContext); + Action logFunction = null; + if (LogTaskInputs && !LoggingContext.LoggingService.OnlyLogCriticalEvents && itemsToAdd?.Count > 0) { - ItemGroupLoggingHelper.LogTaskParameter( - LoggingContext, - TaskParameterMessageKind.AddItem, - parameterName: null, - propertyName: null, - child.ItemType, - itemsToAdd, - logItemMetadata: true, - child.Location); + logFunction = (itemList) => + { + ItemGroupLoggingHelper.LogTaskParameter( + LoggingContext, + TaskParameterMessageKind.AddItem, + parameterName: null, + propertyName: null, + child.ItemType, + itemList, + logItemMetadata: true, + child.Location); + }; } // Now add the items we created to the lookup. - bucket.Lookup.AddNewItemsOfItemType(child.ItemType, itemsToAdd, !keepDuplicates); // Add in one operation for potential copy-on-write + bucket.Lookup.AddNewItemsOfItemType(child.ItemType, itemsToAdd, !keepDuplicates, logFunction); + // Add in one operation for potential copy-on-write } /// diff --git a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs index e0bcc5eff38..e91c4682bf9 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections; using System.Collections.Generic; using System.Linq; using System.Threading; @@ -636,7 +637,7 @@ internal void SetProperty(ProjectPropertyInstance property) /// /// Implements a true add, an item that has been created in a batch. /// - internal void AddNewItemsOfItemType(string itemType, ICollection group, bool doNotAddDuplicates = false) + internal void AddNewItemsOfItemType(string itemType, ICollection group, bool doNotAddDuplicates = false, Action logFunction = null) { // Adding to outer scope could be easily implemented, but our code does not do it at present MustNotBeOuterScope(); @@ -658,14 +659,41 @@ internal void AddNewItemsOfItemType(string itemType, ICollection itemsToAdd = group; if (doNotAddDuplicates) { - // Remove duplicates from the inputs. - itemsToAdd = itemsToAdd.Distinct(ProjectItemInstance.EqualityComparer); - // Ensure we don't also add any that already exist. var existingItems = GetItems(itemType); + + var existingItemsHashSet = existingItems.ToHashSet(ProjectItemInstance.EqualityComparer); + if (existingItems.Count > 0) { - itemsToAdd = itemsToAdd.Where(item => !existingItems.Contains(item, ProjectItemInstance.EqualityComparer)); + var deduplicatedItemsToAdd = new List(); + foreach (var item in itemsToAdd) + { + if (existingItemsHashSet.Add(item)) + { + deduplicatedItemsToAdd.Add(item); + } + } + + itemsToAdd = deduplicatedItemsToAdd; + } + else + { + // Remove the duplicates in case we're not concerned with the existing items. + itemsToAdd = itemsToAdd.Distinct(ProjectItemInstance.EqualityComparer); + } + } + + if (logFunction != null) + { + if (doNotAddDuplicates) + { + logFunction.Invoke(itemsToAdd.ToList()); + } + else + { + var groupAsList = group as List; + logFunction.Invoke(groupAsList ?? group.ToList()); } } From 028fdb5db432e268bdda780177a027b2735b5e99 Mon Sep 17 00:00:00 2001 From: SimaTian Date: Fri, 1 Nov 2024 09:28:29 +0100 Subject: [PATCH 2/2] removing if branch as per discussion with Kyrill --- .../Components/RequestBuilder/Lookup.cs | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs index e91c4682bf9..069a9482f62 100644 --- a/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs +++ b/src/Build/BackEnd/Components/RequestBuilder/Lookup.cs @@ -661,34 +661,25 @@ internal void AddNewItemsOfItemType(string itemType, ICollection 0) + var deduplicatedItemsToAdd = new List(); + foreach (var item in itemsToAdd) { - var deduplicatedItemsToAdd = new List(); - foreach (var item in itemsToAdd) + if (existingItemsHashSet.Add(item)) { - if (existingItemsHashSet.Add(item)) - { - deduplicatedItemsToAdd.Add(item); - } + deduplicatedItemsToAdd.Add(item); } - - itemsToAdd = deduplicatedItemsToAdd; - } - else - { - // Remove the duplicates in case we're not concerned with the existing items. - itemsToAdd = itemsToAdd.Distinct(ProjectItemInstance.EqualityComparer); } + itemsToAdd = deduplicatedItemsToAdd; } if (logFunction != null) { if (doNotAddDuplicates) { - logFunction.Invoke(itemsToAdd.ToList()); + // itemsToAdd is guaranteed to be a List if we're doing the doNotAddDuplicates part. + logFunction.Invoke(itemsToAdd as List); } else {