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

Very slow build - can't build project #54293

Closed
mokshinpv opened this issue Feb 16, 2024 · 21 comments · Fixed by #54479
Closed

Very slow build - can't build project #54293

mokshinpv opened this issue Feb 16, 2024 · 21 comments · Fixed by #54479
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc

Comments

@mokshinpv
Copy link

Version Used:
8.0.200
Visual Studio 2022, 17.9.0

Steps to Reproduce:

After upgrading to .NET 8 I can't build my project.
Build doesn't finish (maybe infinite loop).
This project contains classes which are generated by Antlr4.
These classes are very large so maybe this causes problems.

I created small project with same problem:
academitslearn.zip

Here build finishes but it takes about 10 seconds.
In previous versions build was immediate.

Expected Behavior:

Build finishes and build is fast.

Actual Behavior:

Build not finishes for real project.
Build is very slow for small project.

@dotnet-issue-labeler dotnet-issue-labeler bot added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework untriaged labels Feb 16, 2024
@mokshinpv mokshinpv changed the title Very slow build Very slow build - can't build project Feb 16, 2024
@cston cston removed the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Feb 17, 2024
@Youssef1313
Copy link
Member

Youssef1313 commented Feb 17, 2024

There are some significant allocations in SplitCases:

image

Not sure if they are related though.

Also, a bit of array resizes in ComputeDeclarations:

image

@cston
Copy link
Member

cston commented Feb 19, 2024

@mokshinpv, thanks for reporting this issue.

Currently, the repro project fails with:

error NU1101: Unable to find package Antlr4.

@mokshinpv
Copy link
Author

mokshinpv commented Feb 19, 2024

Hi, @cston, I think it should work, I downloaded project now and Antlr4 package was downloaded correctly from Nuget.

Libraries from csproj file:

<ItemGroup>
  <PackageReference Include="Antlr4" Version="4.6.6">
    <PrivateAssets>all</PrivateAssets>
    <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
  </PackageReference>
  <PackageReference Include="Antlr4.Runtime" Version="4.6.6" />
</ItemGroup>

@cston
Copy link
Member

cston commented Feb 20, 2024

Thanks @mokshinpv, I was able to build successfully after all.

Are you seeing the compile performance issue when building with msbuild.exe or dotnet build? Were additional analyzers included after upgrading? (To see analyzer times, add <ReportAnalyzer>true</ReportAnalyzer> to the project file, and build with msbuild.exe -v:detailed.)

I compared build times for the project using the following:

  • Visual Studio 17.9.0 / SDK 8.0.200
  • Visual Studio 17.6.12 / SDK 7.0.406

For me, the build times were similar between the two versions, with 17.6.12 perhaps slightly faster.
With analyzers included, msbuild /t:rebuild took 7-8 secs for each. With analyzers removed, the equivalent csc.exe command took 4-5 secs for each.

@mokshinpv
Copy link
Author

Hi, @cston.

Are you seeing the compile performance issue when building with msbuild.exe or dotnet build?

Problem with both.

Were additional analyzers included after upgrading?

No, I only changed net7.0 to net8.0 in all csproj files.

I think the problem is in some analyzers.
I disabled "Code Analysis" -> "All analyzers" -> "Run on build" in my real project and now project builds.

Here is screenshot of used analyzers:
Analyzers

Maybe do you know how to determine analyzers which cause problem with build?
Or maybe I can provide some needed information or check something?

@jjonescz
Copy link
Member

If you execute dotnet build -bl -p:ReportAnalyzer=true, it will produce a msbuild.binlog file which should have useful info about the analyzers running (that file can be opened with https://msbuildlog.com/).

@mokshinpv
Copy link
Author

Launched this command for my real project: dotnet build -bl -p:ReportAnalyzer=true
Build took 29 minutes.
In Windows task manager this command uses 4630 Mb of memory for my real project.

If I understood correctly these are the slowest analyzers:
CUsersPavelDesktopmsbuild binlog - MSBuild Structured Log Viewer

@jjonescz
Copy link
Member

I have downloaded the sample you provided and compared clean builds between TargetFramework net7.0 / SDK 7.0.100 and TargetFramework net8.0 / SDK 8.0.201. The former builds in ~7 seconds, the latter in ~10 seconds. So there's some slowdown but not that significant.

In the screenshot you posted I see RoutePatternAnalyzer takes a ridiculous amount of time. That analyzer does not even run in the small demo you posted, so we cannot investigate the problem without a better repro. But I see there's already an issue tracking performance problems with the analyzer: #53899

@mokshinpv
Copy link
Author

Hi, here is better repro.
Build for .NET 8 is very slow.
If set .NET 7 it builds much faster.
mokshinpv-academitslearn-f14ea4856e47.zip

@jjonescz
Copy link
Member

jjonescz commented Mar 1, 2024

Thanks @mokshinpv, I can repro that. Clean 7.0.100/net7.0 build took 19 seconds; clean 8.0.201/net8.0 build took 1833 seconds (half an hour), 9.0.100-preview.3.24129.12 build was also taking long time (I didn't wait for it to finish).

Note: adding this to .editorconfig disables the problematic analyzer and the build is much faster:

dotnet_diagnostic.ASP0017.severity = none
dotnet_diagnostic.ASP0018.severity = none

Here's comparison between net7 and net8 builds:

image

cc @jaredpar @captainsafia

@jjonescz
Copy link
Member

jjonescz commented Mar 1, 2024

The problem are probably strings generated by Antl4 like this:

image

I've created a simplified repro with a few strings like that copied over and the analyzer takes 8 seconds: https://github.com/jjonescz/RoslynIssue72148

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Mar 1, 2024
@jaredpar jaredpar transferred this issue from dotnet/roslyn Mar 1, 2024
@cston
Copy link
Member

cston commented Mar 1, 2024

With the simplified repro from @jjonescz, it looks like 46% of the time is in RoutePatternAnalyzer.AnalyzeToken().

image

@mkArtakMSFT mkArtakMSFT added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework untriaged labels Mar 6, 2024
@mkArtakMSFT
Copy link
Member

Moving to the minimal are path. This is a potential dupe of #53899

@cston cston assigned JamesNK and unassigned cston Mar 6, 2024
@cston
Copy link
Member

cston commented Mar 6, 2024

@JamesNK, it looks like HasLanguageComment may be walking up the parent chain for each token in the string concatenation.

@JamesNK
Copy link
Member

JamesNK commented Mar 7, 2024

Is that a problem?

The code here:

private static bool HasLanguageComment(
SyntaxToken token,
[NotNullWhen(true)] out string? identifier,
[NotNullWhen(true)] out IEnumerable<string>? options)
{
if (HasLanguageComment(token.GetPreviousToken().TrailingTrivia, out identifier, out options))
{
return true;
}
for (var node = token.Parent; node != null; node = node.Parent)
{
if (HasLanguageComment(node.GetLeadingTrivia(), out identifier, out options))
{
return true;
}
// Stop walking up once we hit a statement. We don't need/want statements higher up the parent chain to
// have any impact on this token.
if (IsStatement(node))
{
break;
}
}
return false;
}
private static bool HasLanguageComment(
SyntaxTriviaList list,
[NotNullWhen(true)] out string? identifier,
[NotNullWhen(true)] out IEnumerable<string>? options)
{
foreach (var trivia in list)
{
if (HasLanguageComment(trivia, out identifier, out options))
{
return true;
}
}
identifier = null;
options = null;
return false;
}
private static bool HasLanguageComment(
SyntaxTrivia trivia,
[NotNullWhen(true)] out string? identifier,
[NotNullWhen(true)] out IEnumerable<string>? options)
{
if (IsRegularComment(trivia))
{
// Note: ToString on SyntaxTrivia is non-allocating. It will just return the
// underlying text that the trivia is already pointing to.
var text = trivia.ToString();
if (_commentDetector.TryMatch(text, out identifier, out options))
{
return true;
}
}
identifier = null;
options = null;
return false;
}

Is copied from Roslyn:
https://github.com/dotnet/roslyn/blob/891584232dc8112f33376e9ee9486051a1014b24/src/Features/Core/Portable/EmbeddedLanguages/EmbeddedLanguageDetector.cs#L67-L125

@cston
Copy link
Member

cston commented Mar 7, 2024

If HasLanguageComment() is called for each token, for an expression that is a concatenation of N strings, this could be O(N^2).

@JamesNK
Copy link
Member

JamesNK commented Mar 11, 2024

I've recreated the slow performance in a test: #54479

I looked into this for a couple of hours - and I stepped through code in the aspnetcore repo and Roslyn repo - and to be honest I don't understand why Roslyn's AbstractRegexDiagnosticAnalyzer.AnalyzeToken is fast and RoutePatternAnalyzer.AnalyzeToken is slow.

I copied the code for the route analyzer from the regex analyzer. The only modifications I made were to replace missing public APIs. They both call HasLanguageComment on each piece of the string the same way.

@JamesNK
Copy link
Member

JamesNK commented Mar 11, 2024

image

@JamesNK
Copy link
Member

JamesNK commented Mar 11, 2024

cc @CyrusNajmabadi

@JamesNK
Copy link
Member

JamesNK commented Mar 11, 2024

I'm going to see if I can optimize walking up the nested string concat tree by remembering the last analyzed string.

For example, if "c" in the example below doesn't have a classification then "d" must not have one either:

var s = "a" + "b" + "c" + "d";

@cston
Copy link
Member

cston commented Mar 12, 2024

It looks like CSharpRegexDiagnosticAnalyzer has similar perf. In an expression new Regex("a" + "b" + ...) with a concatenation of N strings, the analyzer perf appears to be O(N^2).

cc @CyrusNajmabadi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants