From 008a558f03cd2f9e3c787922557f20c65376c3d2 Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Sun, 16 Mar 2025 19:28:00 +0000 Subject: [PATCH 1/2] perf: add `ValueListBuilder` to `InvocationExpression` --- .../InvocationExpression.cs | 172 ++++++++++-------- .../Utilities/ListExtensions.cs | 40 ++++ 2 files changed, 140 insertions(+), 72 deletions(-) diff --git a/Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs b/Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs index b5d522655..c6aa38638 100644 --- a/Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs +++ b/Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs @@ -6,7 +6,7 @@ namespace CSharpier.Core.CSharp.SyntaxPrinter.SyntaxNodePrinters; -internal record PrintedNode(CSharpSyntaxNode Node, Doc Doc); +internal record struct PrintedNode(CSharpSyntaxNode Node, Doc Doc); // This is based on prettier/src/language-js/print/member-chain.js // various discussions/prs about how to potentially improve the formatting @@ -25,27 +25,49 @@ public static Doc Print(InvocationExpressionSyntax node, PrintingContext context public static Doc PrintMemberChain(ExpressionSyntax node, PrintingContext context) { var parent = node.Parent; - var printedNodes = new List(); + var printedNodes = new ValueListBuilder( + [default, default, default, default, default, default, default, default] + ); + + FlattenAndPrintNodes(node, ref printedNodes, context); - FlattenAndPrintNodes(node, printedNodes, context); + var anyInvocationExpression = false; - var groups = printedNodes.Any(o => o.Node is InvocationExpressionSyntax) - ? GroupPrintedNodesPrettierStyle(printedNodes) - : GroupPrintedNodesOnLines(printedNodes); + foreach (var printedNode in printedNodes.AsSpan()) + { + if (printedNode.Node is InvocationExpressionSyntax) + { + anyInvocationExpression = true; + break; + } + } + + var groups = new ValueListBuilder( + [null, null, null, null, null, null, null, null] + ); + + if (anyInvocationExpression) + { + GroupPrintedNodesPrettierStyle(ref groups, ref printedNodes); + } + else + { + GroupPrintedNodesOnLines(ref groups, ref printedNodes); + } - var oneLine = SelectManyDocsToArray(groups); + var oneLine = SelectManyDocsToArray(ref groups); - var shouldMergeFirstTwoGroups = ShouldMergeFirstTwoGroups(groups, parent); + var shouldMergeFirstTwoGroups = ShouldMergeFirstTwoGroups(ref groups, parent); var cutoff = shouldMergeFirstTwoGroups ? 3 : 2; var forceOneLine = ( - groups.Count <= cutoff + groups.Length <= cutoff && ( - groups - .Skip(shouldMergeFirstTwoGroups ? 1 : 0) - .Any(o => + groups.ManualSkipAny( + shouldMergeFirstTwoGroups ? 1 : 0, + o => o.Last().Node is not ( InvocationExpressionSyntax @@ -55,11 +77,10 @@ or PostfixUnaryExpressionSyntax Operand: InvocationExpressionSyntax } ) - ) + ) // if the last group contains just a !, make sure it doesn't end up on a new line || ( - groups.Last().Count == 1 - && groups.Last()[0].Node is PostfixUnaryExpressionSyntax + groups[^1].Length == 1 && groups[^1][0].Node is PostfixUnaryExpressionSyntax ) ) ) @@ -95,15 +116,15 @@ is LiteralExpressionSyntax Doc.Concat(groups[0].Select(o => o.Doc).ToArray()), shouldMergeFirstTwoGroups ? Doc.IndentIf( - groups.Count > 2 && groups[1].Last().Doc is not Group { Contents: IndentDoc }, + groups.Length > 2 && groups[1].Last().Doc is not Group { Contents: IndentDoc }, Doc.Concat(groups[1].Select(o => o.Doc).ToArray()) ) : Doc.Null, - PrintIndentedGroup(groups.Skip(shouldMergeFirstTwoGroups ? 2 : 1).ToList()) + PrintIndentedGroup(groups.AsSpan()[(shouldMergeFirstTwoGroups ? 2 : 1)..]) ); return - oneLine.Skip(1).Any(DocUtilities.ContainsBreak) + oneLine.ManualSkipAny(1, DocUtilities.ContainsBreak) || groups[0] .Any(o => o.Node @@ -126,7 +147,7 @@ parent is ExpressionStatementSyntax expressionStatementSyntax private static void FlattenAndPrintNodes( ExpressionSyntax expression, - List printedNodes, + ref ValueListBuilder printedNodes, PrintingContext context ) { @@ -152,8 +173,8 @@ And we want to work with them from Left to Right */ if (expression is InvocationExpressionSyntax invocationExpressionSyntax) { - FlattenAndPrintNodes(invocationExpressionSyntax.Expression, printedNodes, context); - printedNodes.Add( + FlattenAndPrintNodes(invocationExpressionSyntax.Expression, ref printedNodes, context); + printedNodes.Append( new PrintedNode( invocationExpressionSyntax, ArgumentList.Print(invocationExpressionSyntax.ArgumentList, context) @@ -162,8 +183,8 @@ And we want to work with them from Left to Right } else if (expression is ElementAccessExpressionSyntax elementAccessExpression) { - FlattenAndPrintNodes(elementAccessExpression.Expression, printedNodes, context); - printedNodes.Add( + FlattenAndPrintNodes(elementAccessExpression.Expression, ref printedNodes, context); + printedNodes.Append( new PrintedNode( elementAccessExpression, Node.Print(elementAccessExpression.ArgumentList, context) @@ -172,8 +193,12 @@ And we want to work with them from Left to Right } else if (expression is MemberAccessExpressionSyntax memberAccessExpressionSyntax) { - FlattenAndPrintNodes(memberAccessExpressionSyntax.Expression, printedNodes, context); - printedNodes.Add( + FlattenAndPrintNodes( + memberAccessExpressionSyntax.Expression, + ref printedNodes, + context + ); + printedNodes.Append( new PrintedNode( memberAccessExpressionSyntax, Doc.Concat( @@ -187,10 +212,10 @@ And we want to work with them from Left to Right { FlattenAndPrintNodes( conditionalAccessExpressionSyntax.Expression, - printedNodes, + ref printedNodes, context ); - printedNodes.Add( + printedNodes.Append( new PrintedNode( conditionalAccessExpressionSyntax, Token.Print(conditionalAccessExpressionSyntax.OperatorToken, context) @@ -198,7 +223,7 @@ And we want to work with them from Left to Right ); FlattenAndPrintNodes( conditionalAccessExpressionSyntax.WhenNotNull, - printedNodes, + ref printedNodes, context ); } @@ -209,8 +234,8 @@ expression is PostfixUnaryExpressionSyntax } postfixUnaryExpression ) { - FlattenAndPrintNodes(postfixUnaryExpression.Operand, printedNodes, context); - printedNodes.Add( + FlattenAndPrintNodes(postfixUnaryExpression.Operand, ref printedNodes, context); + printedNodes.Append( new PrintedNode( postfixUnaryExpression, Token.Print(postfixUnaryExpression.OperatorToken, context) @@ -219,11 +244,14 @@ expression is PostfixUnaryExpressionSyntax } else { - printedNodes.Add(new PrintedNode(expression, Node.Print(expression, context))); + printedNodes.Append(new PrintedNode(expression, Node.Print(expression, context))); } } - private static List> GroupPrintedNodesOnLines(List printedNodes) + private static void GroupPrintedNodesOnLines( + ref ValueListBuilder groups, + ref ValueListBuilder printedNodes + ) { // We want to group the printed nodes in the following manner // @@ -234,18 +262,15 @@ private static List> GroupPrintedNodesOnLines(List([default, default, default, default]); + currentGroup.Append(printedNodes[0]); - var groups = new List>(); - - var currentGroup = new List { printedNodes[0] }; - groups.Add(currentGroup); - - for (var index = 1; index < printedNodes.Count; index++) + for (var index = 1; index < printedNodes.Length; index++) { if (printedNodes[index].Node is ConditionalAccessExpressionSyntax) { - currentGroup = []; - groups.Add(currentGroup); + groups.Append(currentGroup.AsSpan().ToArray()); + currentGroup.Length = 0; } else if ( printedNodes[index].Node @@ -255,18 +280,22 @@ or IdentifierNameSyntax && printedNodes[index + -1].Node is not ConditionalAccessExpressionSyntax ) { - currentGroup = []; - groups.Add(currentGroup); + groups.Append(currentGroup.AsSpan().ToArray()); + currentGroup.Length = 0; } - currentGroup.Add(printedNodes[index]); + currentGroup.Append(printedNodes[index]); } - return groups; + if (currentGroup.Length > 0) + { + groups.Append(currentGroup.AsSpan().ToArray()); + } } - private static List> GroupPrintedNodesPrettierStyle( - List printedNodes + private static void GroupPrintedNodesPrettierStyle( + ref ValueListBuilder groups, + ref ValueListBuilder printedNodes ) { // We want to group the printed nodes in the following manner @@ -289,14 +318,15 @@ List printedNodes // TODO #451 this whole thing could possibly just turn into a big loop // based on the current node, and the next/previous node, decide when to create new groups. // certain nodes need to stay in the current group, other nodes indicate that a new group needs to be created. - var groups = new List>(); - var currentGroup = new List { printedNodes[0] }; + var currentGroup = new ValueListBuilder([default, default, default, default]); + currentGroup.Append(printedNodes[0]); + var index = 1; - for (; index < printedNodes.Count; index++) + for (; index < printedNodes.Length; index++) { if (printedNodes[index].Node is InvocationExpressionSyntax) { - currentGroup.Add(printedNodes[index]); + currentGroup.Append(printedNodes[index]); } else { @@ -306,21 +336,21 @@ List printedNodes if ( printedNodes[0].Node is not (InvocationExpressionSyntax or PostfixUnaryExpressionSyntax) - && index < printedNodes.Count + && index < printedNodes.Length && printedNodes[index].Node is ElementAccessExpressionSyntax or PostfixUnaryExpressionSyntax ) { - currentGroup.Add(printedNodes[index]); + currentGroup.Append(printedNodes[index]); index++; } - groups.Add(currentGroup); - currentGroup = []; + groups.Append(currentGroup.AsSpan().ToArray()); + currentGroup.Length = 0; var hasSeenNodeThatRequiresBreak = false; - for (; index < printedNodes.Count; index++) + for (; index < printedNodes.Length; index++) { if ( hasSeenNodeThatRequiresBreak @@ -329,8 +359,8 @@ is MemberAccessExpressionSyntax or ConditionalAccessExpressionSyntax ) { - groups.Add(currentGroup); - currentGroup = []; + groups.Append(currentGroup.AsSpan().ToArray()); + currentGroup.Length = 0; hasSeenNodeThatRequiresBreak = false; } @@ -342,30 +372,28 @@ or ElementAccessExpressionSyntax { hasSeenNodeThatRequiresBreak = true; } - currentGroup.Add(printedNodes[index]); + currentGroup.Append(printedNodes[index]); } - if (currentGroup.Count != 0) + if (currentGroup.Length != 0) { - groups.Add(currentGroup); + groups.Append(currentGroup.AsSpan().ToArray()); } - - return groups; } [SuppressMessage("ReSharper", "ForeachCanBePartlyConvertedToQueryUsingAnotherGetEnumerator")] - private static Doc[] SelectManyDocsToArray(List> groups) + private static Doc[] SelectManyDocsToArray(ref ValueListBuilder groups) { var arrayLength = 0; - foreach (var group in groups) + foreach (var group in groups.AsSpan()) { - arrayLength += group.Count; + arrayLength += group.Length; } var outputArray = new Doc[arrayLength]; var pos = 0; - foreach (var group in groups) + foreach (var group in groups.AsSpan()) { foreach (var node in group) { @@ -377,9 +405,9 @@ private static Doc[] SelectManyDocsToArray(List> groups) return outputArray; } - private static Doc PrintIndentedGroup(List> groups) + private static Doc PrintIndentedGroup(ReadOnlySpan groups) { - if (groups.Count == 0) + if (groups.Length == 0) { return Doc.Null; } @@ -389,7 +417,7 @@ private static Doc PrintIndentedGroup(List> groups) Doc.HardLine, Doc.Join( Doc.HardLine, - groups.Select(o => Doc.Group(o.Select(p => p.Doc).ToArray())) + groups.ToArray().Select(o => Doc.Group(o.Select(p => p.Doc).ToArray())) ) ) ); @@ -408,11 +436,11 @@ private static Doc PrintIndentedGroup(List> groups) .CallMethod(); */ private static bool ShouldMergeFirstTwoGroups( - List> groups, + ref ValueListBuilder groups, SyntaxNode? parent ) { - if (groups.Count < 2 || groups[0].Count != 1) + if (groups.Length < 2 || groups[0].Length != 1) { return false; } @@ -435,7 +463,7 @@ or BaseExpressionSyntax // TODO maybe some things to fix in here // https://github.com/belav/csharpier-repos/pull/100/files if ( - groups[1].Count == 1 + groups[1].Length == 1 || parent is SimpleLambdaExpressionSyntax or ArgumentSyntax diff --git a/Src/CSharpier.Core/Utilities/ListExtensions.cs b/Src/CSharpier.Core/Utilities/ListExtensions.cs index 51ca7b1c3..148552b90 100644 --- a/Src/CSharpier.Core/Utilities/ListExtensions.cs +++ b/Src/CSharpier.Core/Utilities/ListExtensions.cs @@ -30,6 +30,46 @@ public static void AddIfNotNull(this List value, Doc doc) } } + public static bool ManualSkipAny( + ref this ValueListBuilder collection, + int count, + Func predicate + ) + { + if (predicate == null || count < 0) + throw new ArgumentException("Invalid arguments"); + + int skipped = 0; + foreach (var item in collection.AsSpan()) + { + if (skipped++ < count) + continue; // Skip the first 'count' items + + if (predicate(item)) + return true; // If any item satisfies the predicate, return true + } + + return false; // No match found after skipping 'count' items + } + + public static bool ManualSkipAny(this T[] collection, int count, Func predicate) + { + if (collection == null || predicate == null || count < 0) + throw new ArgumentException("Invalid arguments"); + + int skipped = 0; + foreach (var item in collection) + { + if (skipped++ < count) + continue; // Skip the first 'count' items + + if (predicate(item)) + return true; // If any item satisfies the predicate, return true + } + + return false; // No match found after skipping 'count' items + } + // Overload for Any to prevent unnecessary allocations of EnumeratorImpl public static bool Any(this in SyntaxTriviaList triviaList, Func predicate) { From e7d7b4fc41ef63ab99b7ee6e8f99cb287554044f Mon Sep 17 00:00:00 2001 From: Timothy Makkison Date: Sun, 16 Mar 2025 22:11:59 +0000 Subject: [PATCH 2/2] perf: replace `ManualSkipAny` with `Span` extensions --- .../InvocationExpression.cs | 68 ++++++++++++++----- .../Utilities/ListExtensions.cs | 39 +++-------- 2 files changed, 58 insertions(+), 49 deletions(-) diff --git a/Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs b/Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs index c6aa38638..4158acff1 100644 --- a/Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs +++ b/Src/CSharpier.Core/CSharp/SyntaxPrinter/SyntaxNodePrinters/InvocationExpression.cs @@ -1,5 +1,6 @@ using System.Diagnostics.CodeAnalysis; using CSharpier.Core.DocTypes; +using CSharpier.Core.Utilities; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -65,9 +66,10 @@ public static Doc PrintMemberChain(ExpressionSyntax node, PrintingContext contex ( groups.Length <= cutoff && ( - groups.ManualSkipAny( - shouldMergeFirstTwoGroups ? 1 : 0, - o => + groups + .AsSpan() + .Skip(shouldMergeFirstTwoGroups ? 1 : 0) + .Any(o => o.Last().Node is not ( InvocationExpressionSyntax @@ -77,7 +79,7 @@ or PostfixUnaryExpressionSyntax Operand: InvocationExpressionSyntax } ) - ) + ) // if the last group contains just a !, make sure it doesn't end up on a new line || ( groups[^1].Length == 1 && groups[^1][0].Node is PostfixUnaryExpressionSyntax @@ -112,19 +114,10 @@ is LiteralExpressionSyntax return Doc.Group(oneLine); } - var expanded = Doc.Concat( - Doc.Concat(groups[0].Select(o => o.Doc).ToArray()), - shouldMergeFirstTwoGroups - ? Doc.IndentIf( - groups.Length > 2 && groups[1].Last().Doc is not Group { Contents: IndentDoc }, - Doc.Concat(groups[1].Select(o => o.Doc).ToArray()) - ) - : Doc.Null, - PrintIndentedGroup(groups.AsSpan()[(shouldMergeFirstTwoGroups ? 2 : 1)..]) - ); - + var expanded = IntoExpanded(groups, shouldMergeFirstTwoGroups); + return - oneLine.ManualSkipAny(1, DocUtilities.ContainsBreak) + oneLine.AsSpan().Skip(1).Any(DocUtilities.ContainsBreak) || groups[0] .Any(o => o.Node @@ -140,11 +133,50 @@ is ParenthesizedExpressionSyntax parent is ExpressionStatementSyntax expressionStatementSyntax && expressionStatementSyntax.SemicolonToken.LeadingTrivia.Any(o => o.IsComment()) ) - || groups.Count == 1 + || groups.Length == 1 ? expanded : Doc.ConditionalGroup(Doc.Concat(oneLine), expanded); } + private static Doc IntoExpanded( + ValueListBuilder groups, + bool shouldMergeFirstTwoGroups + ) + { + var docs = new ValueListBuilder([null, null, null, null, null, null, null, null]); + + foreach (var item in groups[0]) + { + docs.Append(item.Doc); + } + + if (shouldMergeFirstTwoGroups) + { + if (groups.Length > 2 && groups[1].Last().Doc is not Group { Contents: IndentDoc }) + { + docs.Append(Doc.Indent(Doc.Concat(groups[1].Select(o => o.Doc).ToArray()))); + } + else + { + foreach (var item in groups[1]) + { + docs.Append(item.Doc); + } + } + } + + var indented = PrintIndentedGroup(groups.AsSpan()[(shouldMergeFirstTwoGroups ? 2 : 1)..]); + + if (indented != Doc.Null) + { + docs.Append(indented); + } + + var returnDoc = Doc.Concat(ref docs); + docs.Dispose(); + return returnDoc; + } + private static void FlattenAndPrintNodes( ExpressionSyntax expression, ref ValueListBuilder printedNodes, @@ -469,7 +501,7 @@ is SimpleLambdaExpressionSyntax or ArgumentSyntax or BinaryExpressionSyntax or ExpressionStatementSyntax - || groups[1].Skip(1).First().Node + || groups[1][1].Node is InvocationExpressionSyntax or ElementAccessExpressionSyntax or PostfixUnaryExpressionSyntax diff --git a/Src/CSharpier.Core/Utilities/ListExtensions.cs b/Src/CSharpier.Core/Utilities/ListExtensions.cs index 148552b90..483213820 100644 --- a/Src/CSharpier.Core/Utilities/ListExtensions.cs +++ b/Src/CSharpier.Core/Utilities/ListExtensions.cs @@ -30,45 +30,22 @@ public static void AddIfNotNull(this List value, Doc doc) } } - public static bool ManualSkipAny( - ref this ValueListBuilder collection, - int count, - Func predicate - ) + public static bool Any(this ReadOnlySpan span, Func predicate) { - if (predicate == null || count < 0) - throw new ArgumentException("Invalid arguments"); - - int skipped = 0; - foreach (var item in collection.AsSpan()) + foreach (var item in span) { - if (skipped++ < count) - continue; // Skip the first 'count' items - if (predicate(item)) - return true; // If any item satisfies the predicate, return true + return true; } - return false; // No match found after skipping 'count' items + return false; } - public static bool ManualSkipAny(this T[] collection, int count, Func predicate) - { - if (collection == null || predicate == null || count < 0) - throw new ArgumentException("Invalid arguments"); - - int skipped = 0; - foreach (var item in collection) - { - if (skipped++ < count) - continue; // Skip the first 'count' items - - if (predicate(item)) - return true; // If any item satisfies the predicate, return true - } + public static ReadOnlySpan Skip(this ReadOnlySpan span, int count) => + count > span.Length ? [] : span[count..]; - return false; // No match found after skipping 'count' items - } + public static ReadOnlySpan Skip(this Span span, int count) => + count > span.Length ? [] : span[count..]; // Overload for Any to prevent unnecessary allocations of EnumeratorImpl public static bool Any(this in SyntaxTriviaList triviaList, Func predicate)