aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Very slow build - can't build project

Open mokshinpv opened this issue 1 year ago • 21 comments

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.

mokshinpv avatar Feb 16 '24 18:02 mokshinpv

There are some significant allocations in SplitCases:

image

Not sure if they are related though.

Also, a bit of array resizes in ComputeDeclarations:

image

Youssef1313 avatar Feb 17 '24 10:02 Youssef1313

@mokshinpv, thanks for reporting this issue.

Currently, the repro project fails with:

error NU1101: Unable to find package Antlr4.

cston avatar Feb 19 '24 18:02 cston

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>

mokshinpv avatar Feb 19 '24 22:02 mokshinpv

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.

cston avatar Feb 20 '24 07:02 cston

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?

mokshinpv avatar Feb 21 '24 02:02 mokshinpv

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/).

jjonescz avatar Feb 21 '24 09:02 jjonescz

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

mokshinpv avatar Feb 23 '24 01:02 mokshinpv

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: https://github.com/dotnet/aspnetcore/issues/53899

jjonescz avatar Feb 28 '24 10:02 jjonescz

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

mokshinpv avatar Mar 01 '24 11:03 mokshinpv

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 avatar Mar 01 '24 13:03 jjonescz

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

jjonescz avatar Mar 01 '24 13:03 jjonescz

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

image

cston avatar Mar 01 '24 17:03 cston

Moving to the minimal are path. This is a potential dupe of https://github.com/dotnet/aspnetcore/issues/53899

mkArtakMSFT avatar Mar 06 '24 18:03 mkArtakMSFT

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

cston avatar Mar 06 '24 19:03 cston

Is that a problem?

The code here: https://github.com/dotnet/aspnetcore/blob/276e3fe165e299c090c5ff66394420d4ede15a8e/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteEmbeddedLanguage/Infrastructure/RouteStringSyntaxDetector.cs#L142-L206

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

JamesNK avatar Mar 07 '24 00:03 JamesNK

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

cston avatar Mar 07 '24 00:03 cston

I've recreated the slow performance in a test: https://github.com/dotnet/aspnetcore/pull/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 avatar Mar 11 '24 13:03 JamesNK

image

JamesNK avatar Mar 11 '24 23:03 JamesNK

cc @CyrusNajmabadi

JamesNK avatar Mar 11 '24 23:03 JamesNK

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";

JamesNK avatar Mar 11 '24 23:03 JamesNK

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

cston avatar Mar 12 '24 22:03 cston