From 55fb6d58f477f199f3b932a96eccb163ae7e9df2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 6 Mar 2023 10:31:39 -0800 Subject: [PATCH 1/5] Walk green tree to avoid excessive allocations in reported trace --- .../CSharp/Portable/Syntax/SyntaxFacts.cs | 58 +++++++++++++------ 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs index d1b73848384fe..ecaae82651fef 100644 --- a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs +++ b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs @@ -8,6 +8,7 @@ using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; using static Microsoft.CodeAnalysis.CSharp.SyntaxKind; @@ -544,28 +545,51 @@ internal static bool HasAwaitOperations(SyntaxNode node) } internal static bool IsNestedFunction(SyntaxNode child) + => IsNestedFunction(child.Kind()); + + private static bool IsNestedFunction(SyntaxKind kind) + => kind is SyntaxKind.LocalFunctionStatement + or SyntaxKind.AnonymousMethodExpression + or SyntaxKind.SimpleLambdaExpression + or SyntaxKind.ParenthesizedLambdaExpression; + + [PerformanceSensitive("", Constraint = "Use Green nodes for walking to avoid heavy allocations.")] + internal static bool HasYieldOperations(SyntaxNode? node) { - switch (child.Kind()) + if (node is null) + return false; + + // Intentionally walk green tree to avoid allocations. This accounted for + var stack = ArrayBuilder.GetInstance(); + stack.Push(node.Green); + + while (stack.Count > 0) { - case SyntaxKind.LocalFunctionStatement: - case SyntaxKind.AnonymousMethodExpression: - case SyntaxKind.SimpleLambdaExpression: - case SyntaxKind.ParenthesizedLambdaExpression: + var current = stack.Pop(); + Debug.Assert(node.Green == current || current is not Syntax.InternalSyntax.MemberDeclarationSyntax and not Syntax.InternalSyntax.TypeDeclarationSyntax); + + if (current is null) + continue; + + // Do not descend into functions and expressions + if (IsNestedFunction((SyntaxKind)current.RawKind) || + current is Syntax.InternalSyntax.ExpressionSyntax) + { + continue; + } + + if (current is Syntax.InternalSyntax.YieldStatementSyntax) return true; - default: - return false; + + foreach (var child in current.ChildNodesAndTokens()) + { + if (!child.IsToken) + stack.Push(child); + } } - } - internal static bool HasYieldOperations(SyntaxNode? node) - { - // Do not descend into functions and expressions - return node is object && - node.DescendantNodesAndSelf(child => - { - Debug.Assert(ReferenceEquals(node, child) || child is not (MemberDeclarationSyntax or TypeDeclarationSyntax)); - return !IsNestedFunction(child) && !(node is ExpressionSyntax); - }).Any(n => n is YieldStatementSyntax); + stack.Free(); + return false; } internal static bool HasReturnWithExpression(SyntaxNode? node) From 550d0ee38da1ad55182dcb88efd44388413de617 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 6 Mar 2023 10:32:31 -0800 Subject: [PATCH 2/5] Make private --- src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs index ecaae82651fef..d15317902b876 100644 --- a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs +++ b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs @@ -544,7 +544,7 @@ internal static bool HasAwaitOperations(SyntaxNode node) }); } - internal static bool IsNestedFunction(SyntaxNode child) + private static bool IsNestedFunction(SyntaxNode child) => IsNestedFunction(child.Kind()); private static bool IsNestedFunction(SyntaxKind kind) From 8c4bd9e7a1cb2c33f70de27d386db500ad4ba2d7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 6 Mar 2023 10:35:20 -0800 Subject: [PATCH 3/5] Remove comment --- src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs index d15317902b876..01b63f0ea06ed 100644 --- a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs +++ b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs @@ -559,7 +559,6 @@ internal static bool HasYieldOperations(SyntaxNode? node) if (node is null) return false; - // Intentionally walk green tree to avoid allocations. This accounted for var stack = ArrayBuilder.GetInstance(); stack.Push(node.Green); From 4d0519f9f4fcd1345fbf27b06fd82484226a6cd9 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Tue, 7 Mar 2023 10:04:43 -0800 Subject: [PATCH 4/5] Free stack on return --- src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs index 01b63f0ea06ed..53e4578558778 100644 --- a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs +++ b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs @@ -578,7 +578,10 @@ internal static bool HasYieldOperations(SyntaxNode? node) } if (current is Syntax.InternalSyntax.YieldStatementSyntax) + { + stack.Free(); return true; + } foreach (var child in current.ChildNodesAndTokens()) { From 571dbc81a61cffd18dc80fa54b8d8fb555941973 Mon Sep 17 00:00:00 2001 From: Fred Silberberg Date: Tue, 7 Mar 2023 11:33:36 -0800 Subject: [PATCH 5/5] Include github link for attribute descriptions. Co-authored-by: Rikki Gibson --- src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs index 53e4578558778..e17874ad8bea3 100644 --- a/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs +++ b/src/Compilers/CSharp/Portable/Syntax/SyntaxFacts.cs @@ -553,7 +553,7 @@ or SyntaxKind.AnonymousMethodExpression or SyntaxKind.SimpleLambdaExpression or SyntaxKind.ParenthesizedLambdaExpression; - [PerformanceSensitive("", Constraint = "Use Green nodes for walking to avoid heavy allocations.")] + [PerformanceSensitive("https://github.com/dotnet/roslyn/pull/66970", Constraint = "Use Green nodes for walking to avoid heavy allocations.")] internal static bool HasYieldOperations(SyntaxNode? node) { if (node is null)