From eaa2dd1f483209ec90e6857744f9d1610025e1ee Mon Sep 17 00:00:00 2001 From: Forgind Date: Fri, 17 Mar 2023 12:33:09 -0700 Subject: [PATCH 1/4] Remove unnecessary remove It's fairly straightforward to demonstrate that if we reach this block, then it isn't in the _strongCache anyway, so this is entirely unnecessary work. --- src/Build/Evaluation/ProjectRootElementCache.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs index 80552e8c950..03f5b5161cc 100644 --- a/src/Build/Evaluation/ProjectRootElementCache.cs +++ b/src/Build/Evaluation/ProjectRootElementCache.cs @@ -460,7 +460,6 @@ internal override void DiscardImplicitReferences() } else { - _strongCache.Remove(rootElement); RaiseProjectRootElementRemovedFromStrongCache(rootElement); } } From 742ad7a0abddac91658bd7c4fbb0a63d756ee9a5 Mon Sep 17 00:00:00 2001 From: Forgind Date: Tue, 21 Mar 2023 15:22:40 -0700 Subject: [PATCH 2/4] PR comment --- src/Build/Collections/WeakValueDictionary.cs | 16 ++++++++++++++- .../Evaluation/ProjectRootElementCache.cs | 20 +++++++++---------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/Build/Collections/WeakValueDictionary.cs b/src/Build/Collections/WeakValueDictionary.cs index c4c04d802eb..94fa279b835 100644 --- a/src/Build/Collections/WeakValueDictionary.cs +++ b/src/Build/Collections/WeakValueDictionary.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.Diagnostics; using Microsoft.Build.Shared; @@ -15,7 +16,7 @@ namespace Microsoft.Build.Collections /// /// Type of key /// Type of value, without the WeakReference wrapper. - internal class WeakValueDictionary + internal class WeakValueDictionary : IEnumerable> where V : class { /// @@ -233,5 +234,18 @@ public void Clear() { _dictionary.Clear(); } + + public IEnumerator> GetEnumerator() + { + foreach (KeyValuePair> kvp in _dictionary) + { + if (kvp.Value is not null && kvp.Value.TryGetTarget(out V target)) + { + yield return new KeyValuePair(kvp.Key, target); + } + } + } + + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); } } diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs index 03f5b5161cc..6aa59241ad1 100644 --- a/src/Build/Evaluation/ProjectRootElementCache.cs +++ b/src/Build/Evaluation/ProjectRootElementCache.cs @@ -443,24 +443,22 @@ internal override void DiscardImplicitReferences() LinkedList oldStrongCache = _strongCache; _strongCache = new LinkedList(); - foreach (string projectPath in oldWeakCache.Keys) + foreach (KeyValuePair kvp in oldWeakCache) { - ProjectRootElement rootElement; - - if (oldWeakCache.TryGetValue(projectPath, out rootElement)) + if (kvp.Value.IsExplicitlyLoaded) { - if (rootElement.IsExplicitlyLoaded) - { - _weakCache[projectPath] = rootElement; - } + _weakCache[kvp.Key] = kvp.Value; + } - if (rootElement.IsExplicitlyLoaded && oldStrongCache.Contains(rootElement)) + if (oldStrongCache.Contains(kvp.Value)) + { + if (kvp.Value.IsExplicitlyLoaded) { - _strongCache.AddFirst(rootElement); + _strongCache.AddFirst(kvp.Value); } else { - RaiseProjectRootElementRemovedFromStrongCache(rootElement); + RaiseProjectRootElementRemovedFromStrongCache(kvp.Value); } } } From 4b8d3b8355ca8a70b030371734cde8e43979a335 Mon Sep 17 00:00:00 2001 From: Forgind Date: Thu, 23 Mar 2023 12:16:40 -0700 Subject: [PATCH 3/4] Include nulls --- src/Build/Collections/WeakValueDictionary.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Build/Collections/WeakValueDictionary.cs b/src/Build/Collections/WeakValueDictionary.cs index 94fa279b835..34d1267a234 100644 --- a/src/Build/Collections/WeakValueDictionary.cs +++ b/src/Build/Collections/WeakValueDictionary.cs @@ -239,7 +239,11 @@ public IEnumerator> GetEnumerator() { foreach (KeyValuePair> kvp in _dictionary) { - if (kvp.Value is not null && kvp.Value.TryGetTarget(out V target)) + if (kvp.Value is null) + { + yield return new KeyValuePair(kvp.Key, null); + } + else if (kvp.Value.TryGetTarget(out V target)) { yield return new KeyValuePair(kvp.Key, target); } From 9bd1b660d3d60a853a9e3fb388f6e994ea47b176 Mon Sep 17 00:00:00 2001 From: Forgind Date: Thu, 23 Mar 2023 12:17:30 -0700 Subject: [PATCH 4/4] Avoid NREs --- src/Build/Evaluation/ProjectRootElementCache.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Build/Evaluation/ProjectRootElementCache.cs b/src/Build/Evaluation/ProjectRootElementCache.cs index 6aa59241ad1..0ae438c4412 100644 --- a/src/Build/Evaluation/ProjectRootElementCache.cs +++ b/src/Build/Evaluation/ProjectRootElementCache.cs @@ -445,6 +445,11 @@ internal override void DiscardImplicitReferences() foreach (KeyValuePair kvp in oldWeakCache) { + if (kvp.Value is null) + { + continue; + } + if (kvp.Value.IsExplicitlyLoaded) { _weakCache[kvp.Key] = kvp.Value;