Skip to content

Commit f1bf205

Browse files
perf: optimise SyntaxNodeComparer (#1730)
It appears that `SyntaxNodeComparer` is run after a file is formated, optimisation here may be impactful - Prevent null `Func<T, T, Result>` allocations (still seeing some in the profiler, no idea how to sotp this) - Prevent boxing of `SyntaxTokenList`, `SyntaxNodeOrTokenList` - Use `OrderBy.ToArray` instead of `OrderBy.ToList` - Create `AllSeparatorsButLast` to prevent boxing `SeparatedSyntaxList<SyntaxNode>` and `IEnumerable` creation I have some other idea, but I'm focusing on the unhappy path right now. ### Benchmark (comparing `complexCode`) #### Before | Method | Mean | Error | StdDev | Gen0 | Gen1 | Gen2 | Allocated | |--------------------------- |---------:|--------:|--------:|----------:|----------:|----------:|----------:| | Default_SyntaxNodeComparer | 229.1 ms | 4.55 ms | 7.60 ms | 5000.0000 | 3000.0000 | 1000.0000 | 40.53 MB | #### After | Method | Mean | Error | StdDev | Gen0 | Gen1 | Allocated | |--------------------------- |---------:|--------:|--------:|----------:|----------:|----------:| | Default_SyntaxNodeComparer | 187.4 ms | 3.51 ms | 7.64 ms | 3000.0000 | 1000.0000 | 31.39 MB | Co-authored-by: Bela VanderVoort <[email protected]>
1 parent 0349524 commit f1bf205

File tree

2 files changed

+74
-20
lines changed

2 files changed

+74
-20
lines changed

Src/CSharpier.Core/CSharp/SyntaxNodeComparer.cs

Lines changed: 66 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ CancellationToken cancellationToken
5050
cSharpParseOptions,
5151
cancellationToken: cancellationToken
5252
);
53+
this.CompareFunc = Compare;
5354
}
5455

5556
public string CompareSource()
@@ -148,14 +149,16 @@ SyntaxNode formattedStart
148149
return Equal;
149150
}
150151

152+
#pragma warning disable CA1822
151153
private CompareResult CompareLists<T>(
152-
IReadOnlyList<T> originalList,
153-
IReadOnlyList<T> formattedList,
154-
Func<T, T, CompareResult> comparer,
155-
Func<T, TextSpan> getSpan,
154+
T originalList,
155+
T formattedList,
156+
Func<SyntaxToken, SyntaxToken, CompareResult> comparer,
157+
Func<SyntaxToken, TextSpan> getSpan,
156158
TextSpan originalParentSpan,
157159
TextSpan newParentSpan
158160
)
161+
where T : IReadOnlyList<SyntaxToken>
159162
{
160163
for (var x = 0; x < originalList.Count || x < formattedList.Count; x++)
161164
{
@@ -169,25 +172,71 @@ TextSpan newParentSpan
169172
return NotEqual(getSpan(originalList[x]), newParentSpan);
170173
}
171174

172-
if (
173-
originalList[x] is SyntaxNode originalNode
174-
&& formattedList[x] is SyntaxNode formattedNode
175-
)
175+
var result = comparer(originalList[x], formattedList[x]);
176+
if (result.IsInvalid)
177+
{
178+
return result;
179+
}
180+
}
181+
182+
return Equal;
183+
}
184+
#pragma warning restore CA1822
185+
186+
private CompareResult CompareLists<T>(
187+
T originalList,
188+
T formattedList,
189+
Func<SyntaxNode, SyntaxNode, CompareResult> comparer,
190+
Func<SyntaxNode, TextSpan> getSpan,
191+
TextSpan originalParentSpan,
192+
TextSpan newParentSpan
193+
)
194+
where T : IReadOnlyList<SyntaxNode>
195+
{
196+
for (var x = 0; x < originalList.Count || x < formattedList.Count; x++)
197+
{
198+
if (x == originalList.Count)
199+
{
200+
return NotEqual(originalParentSpan, getSpan(formattedList[x]));
201+
}
202+
203+
if (x == formattedList.Count)
176204
{
177-
this.originalStack.Push((originalNode, originalNode.Parent));
178-
this.formattedStack.Push((formattedNode, formattedNode.Parent));
205+
return NotEqual(getSpan(originalList[x]), newParentSpan);
179206
}
180-
else
207+
208+
var originalNode = originalList[x];
209+
var formattedNode = formattedList[x];
210+
this.originalStack.Push((originalNode, originalNode.Parent));
211+
this.formattedStack.Push((formattedNode, formattedNode.Parent));
212+
}
213+
214+
return Equal;
215+
}
216+
217+
private static SyntaxToken[] AllSeparatorsButLast(in SeparatedSyntaxList<SyntaxNode> list)
218+
{
219+
if (list.Count <= 1)
220+
{
221+
return [];
222+
}
223+
224+
var tokens = new SyntaxToken[list.Count - 1];
225+
var tokenIndex = 0;
226+
227+
foreach (var element in list.GetWithSeparators())
228+
{
229+
if (element.IsToken)
181230
{
182-
var result = comparer(originalList[x], formattedList[x]);
183-
if (result.IsInvalid)
231+
tokens[tokenIndex++] = element.AsToken();
232+
if (tokenIndex == tokens.Length)
184233
{
185-
return result;
234+
break;
186235
}
187236
}
188237
}
189238

190-
return Equal;
239+
return tokens;
191240
}
192241

193242
private static CompareResult NotEqual(SyntaxNode? originalNode, SyntaxNode? formattedNode)
@@ -210,6 +259,8 @@ private static CompareResult NotEqual(TextSpan? originalSpan, TextSpan? formatte
210259
};
211260
}
212261

262+
private Func<SyntaxToken, SyntaxToken, CompareResult> CompareFunc { get; }
263+
213264
private CompareResult Compare(SyntaxToken originalToken, SyntaxToken formattedToken)
214265
{
215266
return this.Compare(originalToken, formattedToken, null, null);

Src/CSharpier.Generators/SyntaxNodeComparerGenerator.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ private static void GenerateMethod(StringBuilder sourceBuilder, INamedTypeSymbol
137137
$$"""
138138
private CompareResult Compare{{type.Name}}({{type.Name}} originalNode, {{type.Name}} formattedNode)
139139
{
140-
CompareResult result;
140+
CompareResult result;
141141
"""
142142
);
143143

@@ -228,10 +228,13 @@ private static void GenerateMethod(StringBuilder sourceBuilder, INamedTypeSymbol
228228
}
229229
else
230230
{
231-
var compare = propertyType.Name == nameof(SyntaxTokenList) ? "Compare" : "null";
231+
var compare =
232+
propertyType.Name == nameof(SyntaxTokenList)
233+
? "CompareFunc"
234+
: "static (_, _) => default";
232235
if (propertyName == "Modifiers")
233236
{
234-
propertyName += ".OrderBy(o => o.Text).ToList()";
237+
propertyName += ".OrderBy(o => o.Text).ToArray()";
235238
}
236239

237240
sourceBuilder.AppendLine(
@@ -249,13 +252,13 @@ private static void GenerateMethod(StringBuilder sourceBuilder, INamedTypeSymbol
249252
)
250253
{
251254
sourceBuilder.AppendLine(
252-
$" result = this.CompareLists(originalNode.{propertyName}, formattedNode.{propertyName}, null, o => o.Span, originalNode.Span, formattedNode.Span);"
255+
$" result = this.CompareLists(originalNode.{propertyName}, formattedNode.{propertyName}, static (_, _) => default, o => o.Span, originalNode.Span, formattedNode.Span);"
253256
);
254257
sourceBuilder.AppendLine(" if (result.IsInvalid) return result;");
255258

256259
// Omit the last separator when comparing the original node with the formatted node, as it legitimately may be added or removed
257260
sourceBuilder.AppendLine(
258-
$" result = this.CompareLists(originalNode.{propertyName}.GetSeparators().Take(originalNode.{propertyName}.Count() - 1).ToList(), formattedNode.{propertyName}.GetSeparators().Take(formattedNode.{propertyName}.Count() - 1).ToList(), Compare, o => o.Span, originalNode.Span, formattedNode.Span);"
261+
$" result = this.CompareLists(AllSeparatorsButLast(originalNode.{propertyName}), AllSeparatorsButLast(formattedNode.{propertyName}), CompareFunc, o => o.Span, originalNode.Span, formattedNode.Span);"
259262
);
260263
sourceBuilder.AppendLine(" if (result.IsInvalid) return result;");
261264
}

0 commit comments

Comments
 (0)