From 16c310eb1a0dc04f6ce05b9aa660972aee576ada Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Mon, 16 Oct 2023 17:43:46 +0200 Subject: [PATCH 1/4] Avoid allocations in CanWrap... methods --- .../Lightup/LightupHelpers.cs | 60 +++++++++++-------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs index b247b263e..e66ab838e 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs @@ -65,18 +65,22 @@ 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 - bool canCast; - if (!wrappedObject.TryGetValue(obj.GetType(), out canCast)) + if (!SupportedObjectWrappers.TryGetValue(underlyingType, out var wrappedObject)) { - canCast = wrappedObject.GetOrAdd( - obj.GetType(), - kind => underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo())); + wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); } - return canCast; + // Avoid creating the delegate and capture class if the value already exists + return wrappedObject.TryGetValue(obj.GetType(), out var canCast) + ? canCast + : GetOrAdd(obj, underlyingType, wrappedObject); + + // Don't inline this method. Otherwise a capture class is generated on each call to CanWrapObject. + static bool GetOrAdd(object obj, Type underlyingType, ConcurrentDictionary wrappedObject) + => wrappedObject.GetOrAdd( + obj.GetType(), + kind => underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo())); } internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) @@ -93,18 +97,22 @@ 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 - bool canCast; - if (!wrappedSyntax.TryGetValue(node.Kind(), out canCast)) + if (!SupportedSyntaxWrappers.TryGetValue(underlyingType, out var wrappedSyntax)) { - canCast = wrappedSyntax.GetOrAdd( - node.Kind(), - kind => underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo())); + wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); } - return canCast; + // Avoid creating the delegate and capture class if the value already exists + return wrappedSyntax.TryGetValue(node.Kind(), out var canCast) + ? canCast + : GetOrAdd(node, underlyingType, wrappedSyntax); + + // Don't inline this method. Otherwise a capture class is generated on each call to CanWrapNode. + static bool GetOrAdd(SyntaxNode node, Type underlyingType, ConcurrentDictionary wrappedSyntax) => + wrappedSyntax.GetOrAdd( + node.Kind(), + kind => underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo())); } internal static bool CanWrapOperation(IOperation operation, Type underlyingType) @@ -121,18 +129,22 @@ 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 - bool canCast; - if (!wrappedSyntax.TryGetValue(operation.Kind, out canCast)) + if (!SupportedOperationWrappers.TryGetValue(underlyingType, out var wrappedSyntax)) { - canCast = wrappedSyntax.GetOrAdd( - operation.Kind, - kind => underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo())); + wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); } - return canCast; + // Avoid creating the delegate if the value already exists + return wrappedSyntax.TryGetValue(operation.Kind, out var canCast) + ? canCast + : GetOrAdd(operation, underlyingType, wrappedSyntax); + + // Don't inline this method. Otherwise a capture class is generated on each call to CanWrapOperation. + static bool GetOrAdd(IOperation operation, Type underlyingType, ConcurrentDictionary wrappedSyntax) => + wrappedSyntax.GetOrAdd( + operation.Kind, + kind => underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo())); } internal static Func CreateOperationPropertyAccessor(Type type, string propertyName) From b5fd4b57c96e1e83793faf0b318617dc3008961c Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 17 Oct 2023 10:05:31 +0200 Subject: [PATCH 2/4] Remove local function --- .../Lightup/LightupHelpers.cs | 52 ++++++++----------- 1 file changed, 23 insertions(+), 29 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs index e66ab838e..a2b0a2be9 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs @@ -71,16 +71,14 @@ internal static bool CanWrapObject(object obj, Type underlyingType) wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); } - // Avoid creating the delegate and capture class if the value already exists - return wrappedObject.TryGetValue(obj.GetType(), out var canCast) - ? canCast - : GetOrAdd(obj, underlyingType, wrappedObject); - - // Don't inline this method. Otherwise a capture class is generated on each call to CanWrapObject. - static bool GetOrAdd(object obj, Type underlyingType, ConcurrentDictionary wrappedObject) - => wrappedObject.GetOrAdd( - obj.GetType(), - kind => underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo())); + // Avoid creating the delegate and capture class + if (!wrappedObject.TryGetValue(obj.GetType(), out var canCast)) + { + canCast = underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo()); + wrappedObject.TryAdd(obj.GetType(), canCast); + } + + return canCast; } internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) @@ -103,16 +101,14 @@ internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); } - // Avoid creating the delegate and capture class if the value already exists - return wrappedSyntax.TryGetValue(node.Kind(), out var canCast) - ? canCast - : GetOrAdd(node, underlyingType, wrappedSyntax); + // Avoid creating the delegate and capture class + if (!wrappedSyntax.TryGetValue(node.Kind(), out var canCast)) + { + canCast = underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo()); + wrappedSyntax.TryAdd(node.Kind(), canCast); + } - // Don't inline this method. Otherwise a capture class is generated on each call to CanWrapNode. - static bool GetOrAdd(SyntaxNode node, Type underlyingType, ConcurrentDictionary wrappedSyntax) => - wrappedSyntax.GetOrAdd( - node.Kind(), - kind => underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo())); + return canCast; } internal static bool CanWrapOperation(IOperation operation, Type underlyingType) @@ -135,16 +131,14 @@ internal static bool CanWrapOperation(IOperation operation, Type underlyingType) wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); } - // Avoid creating the delegate if the value already exists - return wrappedSyntax.TryGetValue(operation.Kind, out var canCast) - ? canCast - : GetOrAdd(operation, underlyingType, wrappedSyntax); - - // Don't inline this method. Otherwise a capture class is generated on each call to CanWrapOperation. - static bool GetOrAdd(IOperation operation, Type underlyingType, ConcurrentDictionary wrappedSyntax) => - wrappedSyntax.GetOrAdd( - operation.Kind, - kind => underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo())); + // Avoid creating the delegate and capture class + if (!wrappedSyntax.TryGetValue(operation.Kind, out var canCast)) + { + canCast = underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo()); + wrappedSyntax.TryAdd(operation.Kind, canCast); + } + + return canCast; } internal static Func CreateOperationPropertyAccessor(Type type, string propertyName) From 3386603e8b9e530d9ec60655b136f18267496fc9 Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 17 Oct 2023 20:09:07 +0200 Subject: [PATCH 3/4] Revert changes for static delegate --- .../Lightup/LightupHelpers.cs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs index a2b0a2be9..5ec0bb38a 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs @@ -65,11 +65,7 @@ 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)) - { - wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); - } + ConcurrentDictionary wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); // Avoid creating the delegate and capture class if (!wrappedObject.TryGetValue(obj.GetType(), out var canCast)) @@ -95,11 +91,7 @@ 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)) - { - wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); - } + ConcurrentDictionary wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); // Avoid creating the delegate and capture class if (!wrappedSyntax.TryGetValue(node.Kind(), out var canCast)) @@ -125,11 +117,7 @@ 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 wrappedSyntax)) - { - wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); - } + ConcurrentDictionary wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); // Avoid creating the delegate and capture class if (!wrappedSyntax.TryGetValue(operation.Kind, out var canCast)) From 12b6f938c7325bf4f0e7940a234ada3f4371bb3a Mon Sep 17 00:00:00 2001 From: Martin Strecker Date: Tue, 17 Oct 2023 20:12:00 +0200 Subject: [PATCH 4/4] Comments --- .../StyleCop.Analyzers/Lightup/LightupHelpers.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs index 5ec0bb38a..43e460faf 100644 --- a/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs +++ b/StyleCop.Analyzers/StyleCop.Analyzers/Lightup/LightupHelpers.cs @@ -67,7 +67,7 @@ internal static bool CanWrapObject(object obj, Type underlyingType) ConcurrentDictionary wrappedObject = SupportedObjectWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); - // Avoid creating the delegate and capture class + // Avoid creating a delegate and capture class if (!wrappedObject.TryGetValue(obj.GetType(), out var canCast)) { canCast = underlyingType.GetTypeInfo().IsAssignableFrom(obj.GetType().GetTypeInfo()); @@ -93,7 +93,7 @@ internal static bool CanWrapNode(SyntaxNode node, Type underlyingType) ConcurrentDictionary wrappedSyntax = SupportedSyntaxWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); - // Avoid creating the delegate and capture class + // Avoid creating a delegate and capture class if (!wrappedSyntax.TryGetValue(node.Kind(), out var canCast)) { canCast = underlyingType.GetTypeInfo().IsAssignableFrom(node.GetType().GetTypeInfo()); @@ -119,7 +119,7 @@ internal static bool CanWrapOperation(IOperation operation, Type underlyingType) ConcurrentDictionary wrappedSyntax = SupportedOperationWrappers.GetOrAdd(underlyingType, static _ => new ConcurrentDictionary()); - // Avoid creating the delegate and capture class + // Avoid creating a delegate and capture class if (!wrappedSyntax.TryGetValue(operation.Kind, out var canCast)) { canCast = underlyingType.GetTypeInfo().IsAssignableFrom(operation.GetType().GetTypeInfo());