From 523a2c98301cab38e5dad3474ef14acf0b4da94f Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 28 Sep 2023 15:12:21 +0200 Subject: [PATCH 1/4] Avoid captures --- .../ShimLayer/LightupHelpers.cs | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs index 0c24b6efb41..296d651151b 100644 --- a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs +++ b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs @@ -5,6 +5,7 @@ using System.Linq.Expressions; using System.Reflection; using Microsoft.CodeAnalysis.CSharp; +using Roslyn.Utilities; namespace StyleCop.Analyzers.Lightup; @@ -39,6 +40,7 @@ private static readonly ConcurrentDictionary SupportsCSharp73; + [PerformanceSensitive("https://github.com/SonarSource/sonar-dotnet/issues/8106", AllowCaptures = false, AllowGenericEnumeration = false, AllowImplicitBoxing = false)] internal static bool CanWrapObject(object obj, Type underlyingType) { if (obj == null) @@ -53,11 +55,14 @@ internal static bool CanWrapObject(object obj, Type underlyingType) return false; } - ConcurrentDictionary wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, _ => new ConcurrentDictionary()); + // Avoid creating the delegate if the value already exists + if (!SupportedObjectWrappers.TryGetValue(underlyingType, out var wrappedObject)) + { + wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + } // Avoid creating the delegate if the value already exists - bool canCast; - if (!wrappedObject.TryGetValue(obj.GetType(), out canCast)) + if (!wrappedObject.TryGetValue(obj.GetType(), out var canCast)) { canCast = wrappedObject.GetOrAdd( obj.GetType(), @@ -67,6 +72,7 @@ internal static bool CanWrapObject(object obj, Type underlyingType) return canCast; } + [PerformanceSensitive("https://github.com/SonarSource/sonar-dotnet/issues/8106", AllowCaptures = false, AllowGenericEnumeration = false, AllowImplicitBoxing = false)] internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) { if (node == null) @@ -81,11 +87,14 @@ internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) return false; } - ConcurrentDictionary wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, _ => new ConcurrentDictionary()); + // Avoid creating the delegate if the value already exists + if (!SupportedSyntaxWrappers.TryGetValue(underlyingType, out var wrappedSyntax)) + { + wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + } // Avoid creating the delegate if the value already exists - bool canCast; - if (!wrappedSyntax.TryGetValue(node.Kind(), out canCast)) + if (!wrappedSyntax.TryGetValue(node.Kind(), out var canCast)) { canCast = wrappedSyntax.GetOrAdd( node.Kind(), @@ -95,6 +104,7 @@ internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) return canCast; } + [PerformanceSensitive("https://github.com/SonarSource/sonar-dotnet/issues/8106", AllowCaptures = false, AllowGenericEnumeration = false, AllowImplicitBoxing = false)] internal static bool CanWrapOperation(IOperation operation, Type underlyingType) { if (operation == null) @@ -109,11 +119,14 @@ internal static bool CanWrapOperation(IOperation operation, Type underlyingType) return false; } - ConcurrentDictionary wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, _ => new ConcurrentDictionary()); + // Avoid creating the delegate if the value already exists + if (!SupportedOperationWrappers.TryGetValue(underlyingType, out var wrappedSyntax)) + { + wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + } // Avoid creating the delegate if the value already exists - bool canCast; - if (!wrappedSyntax.TryGetValue(operation.Kind, out canCast)) + if (!wrappedSyntax.TryGetValue(operation.Kind, out var canCast)) { canCast = wrappedSyntax.GetOrAdd( operation.Kind, From 412e90b2ec645e098f3d455677819011933a4f86 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 28 Sep 2023 17:06:40 +0200 Subject: [PATCH 2/4] Avoid captures --- .../ShimLayer/LightupHelpers.cs | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs index 296d651151b..1e9793d7214 100644 --- a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs +++ b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs @@ -62,14 +62,15 @@ internal static bool CanWrapObject(object obj, Type underlyingType) } // Avoid creating the delegate if the value already exists - if (!wrappedObject.TryGetValue(obj.GetType(), out var canCast)) - { - canCast = wrappedObject.GetOrAdd( - obj.GetType(), - kind => underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo())); - } - - return canCast; + return wrappedObject.TryGetValue(obj.GetType(), out var canCast) + ? canCast + : GetOrAdd(obj, underlyingType, wrappedObject); + + // Dont' inline this method. The capture class will span the whole method otherwise. +#pragma warning disable HAA0301, HAA0302 // Display class allocation to capture closure + static bool GetOrAdd(object obj, Type underlyingType, ConcurrentDictionary wrappedObject) => + wrappedObject.GetOrAdd(obj.GetType(), kind => underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo())); +#pragma warning restore HAA0301, HAA0302 } [PerformanceSensitive("https://github.com/SonarSource/sonar-dotnet/issues/8106", AllowCaptures = false, AllowGenericEnumeration = false, AllowImplicitBoxing = false)] @@ -93,15 +94,13 @@ internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); } - // Avoid creating the delegate if the value already exists - if (!wrappedSyntax.TryGetValue(node.Kind(), out var canCast)) - { - canCast = wrappedSyntax.GetOrAdd( - node.Kind(), - kind => underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo())); - } + return wrappedSyntax.TryGetValue(node.Kind(), out var canCast) ? canCast : GetOrAdd(node, underlyingType, wrappedSyntax); - return canCast; + // Dont' inline this method. The capture class will span the whole method otherwise. +#pragma warning disable HAA0301, HAA0302 // Display class allocation to capture closure + static bool GetOrAdd(SyntaxNode node, Type underlyingType, ConcurrentDictionary wrappedSyntax) => + wrappedSyntax.GetOrAdd(node.Kind(), kind => underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo())); +#pragma warning restore HAA0301, HAA0302 } [PerformanceSensitive("https://github.com/SonarSource/sonar-dotnet/issues/8106", AllowCaptures = false, AllowGenericEnumeration = false, AllowImplicitBoxing = false)] @@ -126,14 +125,15 @@ internal static bool CanWrapOperation(IOperation operation, Type underlyingType) } // Avoid creating the delegate if the value already exists - if (!wrappedSyntax.TryGetValue(operation.Kind, out var canCast)) - { - canCast = wrappedSyntax.GetOrAdd( - operation.Kind, - kind => underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo())); - } - - return canCast; + return wrappedSyntax.TryGetValue(operation.Kind, out var canCast) + ? canCast + : GetOrAdd(operation, underlyingType, wrappedSyntax); + + // Dont' inline this method. The capture class will span the whole method otherwise. +#pragma warning disable HAA0301, HAA0302 // Display class allocation to capture closure + static bool GetOrAdd(IOperation operation, Type underlyingType, ConcurrentDictionary wrappedSyntax) => + wrappedSyntax.GetOrAdd(operation.Kind, kind => underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo())); +#pragma warning restore HAA0301, HAA0302 } internal static Func CreateOperationPropertyAccessor(Type type, string propertyName) From 423e1fe5b508822dc38d5f64af96df4e50859284 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Thu, 28 Sep 2023 17:21:49 +0200 Subject: [PATCH 3/4] Rename --- .../SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs index 1e9793d7214..dde8bf516b0 100644 --- a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs +++ b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs @@ -119,20 +119,20 @@ internal static bool CanWrapOperation(IOperation operation, Type underlyingType) } // Avoid creating the delegate if the value already exists - if (!SupportedOperationWrappers.TryGetValue(underlyingType, out var wrappedSyntax)) + if (!SupportedOperationWrappers.TryGetValue(underlyingType, out var wrappedOperation)) { - wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + wrappedOperation = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); } // Avoid creating the delegate if the value already exists - return wrappedSyntax.TryGetValue(operation.Kind, out var canCast) + return wrappedOperation.TryGetValue(operation.Kind, out var canCast) ? canCast - : GetOrAdd(operation, underlyingType, wrappedSyntax); + : GetOrAdd(operation, underlyingType, wrappedOperation); // Dont' inline this method. The capture class will span the whole method otherwise. #pragma warning disable HAA0301, HAA0302 // Display class allocation to capture closure - static bool GetOrAdd(IOperation operation, Type underlyingType, ConcurrentDictionary wrappedSyntax) => - wrappedSyntax.GetOrAdd(operation.Kind, kind => underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo())); + static bool GetOrAdd(IOperation operation, Type underlyingType, ConcurrentDictionary wrappedOperation) => + wrappedOperation.GetOrAdd(operation.Kind, kind => underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo())); #pragma warning restore HAA0301, HAA0302 } From c59a99baaad1364d483a0f544d05c326415f3b08 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Wed, 18 Oct 2023 09:51:09 +0200 Subject: [PATCH 4/4] Sync with #DotNetAnalyzers/StyleCopAnalyzers/3711 from StyleCop --- .../ShimLayer/LightupHelpers.cs | 57 +++++++------------ 1 file changed, 21 insertions(+), 36 deletions(-) diff --git a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs index dde8bf516b0..f4870a1768c 100644 --- a/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs +++ b/analyzers/src/SonarAnalyzer.CFG/ShimLayer/LightupHelpers.cs @@ -55,22 +55,16 @@ internal static bool CanWrapObject(object obj, Type underlyingType) return false; } - // Avoid creating the delegate if the value already exists - if (!SupportedObjectWrappers.TryGetValue(underlyingType, out var wrappedObject)) + ConcurrentDictionary wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + + // Avoid creating a delegate and capture class + if (!wrappedObject.TryGetValue(obj.GetType(), out var canCast)) { - wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + canCast = underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo()); + wrappedObject.TryAdd(obj.GetType(), canCast); } - // Avoid creating the delegate if the value already exists - return wrappedObject.TryGetValue(obj.GetType(), out var canCast) - ? canCast - : GetOrAdd(obj, underlyingType, wrappedObject); - - // Dont' inline this method. The capture class will span the whole method otherwise. -#pragma warning disable HAA0301, HAA0302 // Display class allocation to capture closure - static bool GetOrAdd(object obj, Type underlyingType, ConcurrentDictionary wrappedObject) => - wrappedObject.GetOrAdd(obj.GetType(), kind => underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo())); -#pragma warning restore HAA0301, HAA0302 + return canCast; } [PerformanceSensitive("https://github.com/SonarSource/sonar-dotnet/issues/8106", AllowCaptures = false, AllowGenericEnumeration = false, AllowImplicitBoxing = false)] @@ -88,19 +82,16 @@ internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) return false; } - // Avoid creating the delegate if the value already exists - if (!SupportedSyntaxWrappers.TryGetValue(underlyingType, out var wrappedSyntax)) + ConcurrentDictionary wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + + // Avoid creating a delegate and capture class + if (!wrappedSyntax.TryGetValue(node.Kind(), out var canCast)) { - wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + canCast = underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo()); + wrappedSyntax.TryAdd(node.Kind(), canCast); } - return wrappedSyntax.TryGetValue(node.Kind(), out var canCast) ? canCast : GetOrAdd(node, underlyingType, wrappedSyntax); - - // Dont' inline this method. The capture class will span the whole method otherwise. -#pragma warning disable HAA0301, HAA0302 // Display class allocation to capture closure - static bool GetOrAdd(SyntaxNode node, Type underlyingType, ConcurrentDictionary wrappedSyntax) => - wrappedSyntax.GetOrAdd(node.Kind(), kind => underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo())); -#pragma warning restore HAA0301, HAA0302 + return canCast; } [PerformanceSensitive("https://github.com/SonarSource/sonar-dotnet/issues/8106", AllowCaptures = false, AllowGenericEnumeration = false, AllowImplicitBoxing = false)] @@ -118,22 +109,16 @@ internal static bool CanWrapOperation(IOperation operation, Type underlyingType) return false; } - // Avoid creating the delegate if the value already exists - if (!SupportedOperationWrappers.TryGetValue(underlyingType, out var wrappedOperation)) + ConcurrentDictionary wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + + // Avoid creating a delegate and capture class + if (!wrappedSyntax.TryGetValue(operation.Kind, out var canCast)) { - wrappedOperation = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); + canCast = underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo()); + wrappedSyntax.TryAdd(operation.Kind, canCast); } - // Avoid creating the delegate if the value already exists - return wrappedOperation.TryGetValue(operation.Kind, out var canCast) - ? canCast - : GetOrAdd(operation, underlyingType, wrappedOperation); - - // Dont' inline this method. The capture class will span the whole method otherwise. -#pragma warning disable HAA0301, HAA0302 // Display class allocation to capture closure - static bool GetOrAdd(IOperation operation, Type underlyingType, ConcurrentDictionary wrappedOperation) => - wrappedOperation.GetOrAdd(operation.Kind, kind => underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo())); -#pragma warning restore HAA0301, HAA0302 + return canCast; } internal static Func CreateOperationPropertyAccessor(Type type, string propertyName)