From a12c6c05e193259c32dd8bfb8fda14eb151a00e3 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 30 Jul 2024 15:55:27 -0700 Subject: [PATCH 1/2] Change SyntaxEquivalence.AreEquivalent to use a more appropriate pooling mechanism for the stack it uses to walk trees. The scrolling speedometer test indicates this method's pooling mechanism isn't working great for large files. Specifically, ArrayBuilder instances aren't returned to their pool once their length exceeds 128 items. For even relatively large files, I'm seeing this exceeded in this context. Instead, I've switched this code to just use a pooled Stack. This was accounting for about 1.1% of allocations in the typing scenario for the scrolling speedometer test. --- .../CSharp/Portable/Syntax/SyntaxEquivalence.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs b/src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs index cba16e079df44..17ce6dd1fbcbc 100644 --- a/src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs +++ b/src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs @@ -13,6 +13,9 @@ namespace Microsoft.CodeAnalysis.CSharp.Syntax internal static class SyntaxEquivalence { + private static readonly ObjectPool> s_equivalenceCheckStack = + new ObjectPool>(() => new Stack<(GreenNode?, GreenNode?)>()); + internal static bool AreEquivalent(SyntaxTree? before, SyntaxTree? after, Func? ignoreChildNode, bool topLevel) { if (before == after) @@ -102,13 +105,14 @@ private static bool AreTokensEquivalent(GreenNode? before, GreenNode? after, Fun private static bool AreEquivalentRecursive(GreenNode? before, GreenNode? after, Func? ignoreChildNode, bool topLevel) { // Use an explicit stack so we can walk down deep trees without blowing the real stack. - var stack = ArrayBuilder<(GreenNode? before, GreenNode? after)>.GetInstance(); + var stack = s_equivalenceCheckStack.Allocate(); stack.Push((before, after)); try { - while (stack.TryPop(out var current)) + while (stack.Count > 0) { + var current = stack.Pop(); if (!areEquivalentSingleLevel(current.before, current.after)) return false; } @@ -117,7 +121,7 @@ private static bool AreEquivalentRecursive(GreenNode? before, GreenNode? after, } finally { - stack.Free(); + s_equivalenceCheckStack.Free(stack); } bool areEquivalentSingleLevel(GreenNode? before, GreenNode? after) From 718f265cf1e36242bd577552eafc860826313f5a Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 30 Jul 2024 17:44:27 -0700 Subject: [PATCH 2/2] Clear out the stack before placing back in the pool, as there is a chance that it won't be empty --- src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs b/src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs index 17ce6dd1fbcbc..8d4f888666a69 100644 --- a/src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs +++ b/src/Compilers/CSharp/Portable/Syntax/SyntaxEquivalence.cs @@ -121,6 +121,7 @@ private static bool AreEquivalentRecursive(GreenNode? before, GreenNode? after, } finally { + stack.Clear(); s_equivalenceCheckStack.Free(stack); }