Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/8.0] Fix route analyzer performance with highly concatenated strings #54763

Merged
merged 16 commits into from
Apr 2, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,19 @@ private static bool HasLanguageComment(
return true;
}

// Check for the common case of a string literal in a large binary expression. For example `"..." + "..." +
// "..."` We never want to consider these as regex/json tokens as processing them would require knowing the
// contents of every string literal, and having our lexers/parsers somehow stitch them all together. This is
// beyond what those systems support (and would only work for constant strings anyways). This prevents both
// incorrect results *and* avoids heavy perf hits walking up large binary expressions (often while a caller is
// themselves walking down such a large expression).
if (token.Parent.IsLiteralExpression() &&
token.Parent.Parent.IsBinaryExpression() &&
token.Parent.Parent.RawKind == (int)SyntaxKind.AddExpression)
{
return false;
}

for (var node = token.Parent; node != null; node = node.Parent)
{
if (HasLanguageComment(node.GetLeadingTrivia(), out identifier, out options))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ public static SyntaxNode GetRequiredParent(this SyntaxNode node)
return parent;
}

public static bool IsLiteralExpression([NotNullWhen(true)] this SyntaxNode? node)
=> node is LiteralExpressionSyntax;

public static bool IsBinaryExpression([NotNullWhen(true)] this SyntaxNode? node)
=> node is BinaryExpressionSyntax;

[return: NotNullIfNotNull("node")]
public static SyntaxNode? WalkUpParentheses(this SyntaxNode? node)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Globalization;
using System.Text;
using Microsoft.AspNetCore.Analyzer.Testing;
using Microsoft.AspNetCore.Analyzers.RenderTreeBuilder;
using Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage.Infrastructure;
using Microsoft.CodeAnalysis;
using Xunit.Abstractions;

namespace Microsoft.AspNetCore.Analyzers.RouteEmbeddedLanguage;

public partial class RoutePatternAnalyzerTests
{
private readonly ITestOutputHelper _testOutputHelper;

private TestDiagnosticAnalyzerRunner Runner { get; } = new(new RoutePatternAnalyzer());

public RoutePatternAnalyzerTests(ITestOutputHelper testOutputHelper)
{
_testOutputHelper = testOutputHelper;
}

[Fact]
public async Task CommentOnString_ReportResults()
{
Expand Down Expand Up @@ -512,4 +523,125 @@ public object TestAction(int id)
// Assert
Assert.Empty(diagnostics);
}

[Fact]
public async Task ConcatString_PerformanceTest()
{
// Arrange
var builder = new StringBuilder();
builder.AppendLine("""
class Program
{
static void Main() { }
static readonly string _s =
""");
for (var i = 0; i < 2000; i++)
{
builder.AppendLine(" \"a{}bc\" +");
}
builder.AppendLine("""
"";
}
""");
var source = TestSource.Read(builder.ToString());

// Act 1
// Warm up.
var diagnostics1 = await Runner.GetDiagnosticsAsync(source.Source);

// Assert 1
Assert.Empty(diagnostics1);

// Act 2
// Measure analysis.
var stopwatch = Stopwatch.StartNew();

var diagnostics2 = await Runner.GetDiagnosticsAsync(source.Source);
_testOutputHelper.WriteLine($"Elapsed time: {stopwatch.Elapsed}");

// Assert 2
Assert.Empty(diagnostics2);
}

[Fact]
public async Task ConcatString_DetectLanguage_NoWarningsBecauseConcatString()
{
// Arrange
var builder = new StringBuilder();
builder.AppendLine("""
class Program
{
static void Main() { }
// lang=Route
static readonly string _s =
""");
for (var i = 0; i < 2000; i++)
{
builder.AppendLine(" \"a{}bc\" +");
}
builder.AppendLine("""
"";
}
""");
var source = TestSource.Read(builder.ToString());

// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);

// Assert
Assert.Empty(diagnostics);
}

[Fact]
public async Task NestedLangComment_NoWarningsBecauseConcatString()
{
// Arrange
var builder = new StringBuilder();
builder.AppendLine("""
class Program
{
static void Main() { }
static readonly string _s =
"{/*MM0*/te*st0}" +
// lang=Route
"{/*MM1*/te*st1}" +
"{/*MM2*/te*st2}" +
"{test3}";
}
""");
var source = TestSource.Read(builder.ToString());

// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);

// Assert
Assert.Empty(diagnostics);
}

[Fact]
public async Task TopLangComment_NoWarningsBecauseConcatString()
{
// Arrange
var builder = new StringBuilder();
builder.AppendLine("""
class Program
{
static void Main() { }
static readonly string _s =
// lang=Route
"{/*MM0*/te*st0}" +
"{/*MM1*/te*st1}" +
"{/*MM2*/te*st2}" +
// lang=regex
"{test3}";
}
""");
var source = TestSource.Read(builder.ToString());

// Act
var diagnostics = await Runner.GetDiagnosticsAsync(source.Source);

// Assert
Assert.Empty(diagnostics);
}
}
Loading