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

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Mar 26, 2024

Backport of #54479 to release/8.0

/cc @JamesNK

Fix route analyzer performance with highly concatenated strings

Route analyzer introduced in .NET 8 can cause very long build times in some projects. A highly concatenated string in a C# file causes a lot of time to be spent in Roslyn's GetLeadingTrivia method. Many concatenated strings cause O(N^2) calls to this relatively expensive method.

For example, "x1" + "x2" + "x3" + "x4" + ... "x1000" will greatly increase build times by calling GetLeadingTrivia 1 million times. There isn't a limit to the number of string concats so performance gets worse the more string concats.

The fix is to skip analyzing concatenated strings. This improves performance and is better behavior. The analyzer isn't designed to analyze content spread across multiple string fragments. Note that Roslyn has the same issue (although their performance impact is smaller) and has implemented the same code fix - dotnet/roslyn#72620.

Fixes #54293
Fixes #53899

Customer Impact

Multiple customers have reported 5+ second increase in build time. They have code generation tools that produce hundreds of string concats in output C#.

Customers can work around this issue by disabling the slow analyzer. However, there is no indication that poor performance is linked to an analyzer. Customers are unlikely to find the fix themselves.

Regression?

  • Yes
  • No

Projects with highly concatenated strings that are built fast in .NET 7 are slow after updating to .NET 8.

Risk

  • High
  • Medium
  • Low

A small, isolated change to an analyzer.

Verification

  • Manual (required)
  • Automated

Manual testing using the framework samples project (note: part of aspnetcore solution so build performance has some overhead)

Before:

========== Build completed at 10:23 am and took 15.319 seconds ==========

After:

========== Build completed at 10:24 am and took 04.123 seconds ==========

Packaging changes reviewed?

  • Yes
  • No
  • N/A

When servicing release/2.1

  • Make necessary changes in eng/PatchConfig.props

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Mar 26, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 8.0.x milestone Mar 26, 2024
@JamesNK
Copy link
Member

JamesNK commented Mar 26, 2024

FYI @CyrusNajmabadi @cston @jjonescz

@JamesNK JamesNK added the Servicing-consider Shiproom approval is required for the issue label Mar 26, 2024
@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 labels Mar 28, 2024
@JamesNK JamesNK added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Apr 2, 2024
@JamesNK JamesNK modified the milestones: 8.0.x, 8.0.5 Apr 2, 2024
@wtgodbe wtgodbe merged commit aac8366 into release/8.0 Apr 2, 2024
25 checks passed
@wtgodbe wtgodbe deleted the backport/pr-54479-to-release/8.0 branch April 2, 2024 21:04
@dotnet-policy-service dotnet-policy-service bot modified the milestone: 8.0.5 Apr 2, 2024
oskogstad added a commit to digdir/dialogporten that referenced this pull request May 26, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [Microsoft.AspNetCore.Authentication.JwtBearer](https://asp.net/)
([source](https://togithub.com/dotnet/aspnetcore)) | `8.0.4` -> `8.0.5`
|
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.AspNetCore.Authentication.JwtBearer/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.AspNetCore.Authentication.JwtBearer/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.AspNetCore.Authentication.JwtBearer/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.AspNetCore.Authentication.JwtBearer/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [Microsoft.AspNetCore.Mvc.NewtonsoftJson](https://asp.net/)
([source](https://togithub.com/dotnet/aspnetcore)) | `8.0.4` -> `8.0.5`
|
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.AspNetCore.Mvc.NewtonsoftJson/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.AspNetCore.Mvc.NewtonsoftJson/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.AspNetCore.Mvc.NewtonsoftJson/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.AspNetCore.Mvc.NewtonsoftJson/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [Microsoft.AspNetCore.OpenApi](https://asp.net/)
([source](https://togithub.com/dotnet/aspnetcore)) | `8.0.4` -> `8.0.5`
|
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.AspNetCore.OpenApi/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.AspNetCore.OpenApi/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.AspNetCore.OpenApi/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.AspNetCore.OpenApi/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[Microsoft.EntityFrameworkCore.Relational](https://docs.microsoft.com/ef/core/)
([source](https://togithub.com/dotnet/efcore)) | `8.0.4` -> `8.0.5` |
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.EntityFrameworkCore.Relational/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.EntityFrameworkCore.Relational/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.EntityFrameworkCore.Relational/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.EntityFrameworkCore.Relational/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
|
[Microsoft.EntityFrameworkCore.Tools](https://docs.microsoft.com/ef/core/)
([source](https://togithub.com/dotnet/efcore)) | `8.0.4` -> `8.0.5` |
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.EntityFrameworkCore.Tools/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.EntityFrameworkCore.Tools/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.EntityFrameworkCore.Tools/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.EntityFrameworkCore.Tools/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [Microsoft.Extensions.Caching.StackExchangeRedis](https://asp.net/)
([source](https://togithub.com/dotnet/aspnetcore)) | `8.0.4` -> `8.0.5`
|
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.Extensions.Caching.StackExchangeRedis/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.Extensions.Caching.StackExchangeRedis/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.Extensions.Caching.StackExchangeRedis/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.Extensions.Caching.StackExchangeRedis/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
| [Microsoft.Extensions.Http.Polly](https://asp.net/)
([source](https://togithub.com/dotnet/aspnetcore)) | `8.0.4` -> `8.0.5`
|
[![age](https://developer.mend.io/api/mc/badges/age/nuget/Microsoft.Extensions.Http.Polly/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/nuget/Microsoft.Extensions.Http.Polly/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/nuget/Microsoft.Extensions.Http.Polly/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/nuget/Microsoft.Extensions.Http.Polly/8.0.4/8.0.5?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>dotnet/aspnetcore
(Microsoft.AspNetCore.Authentication.JwtBearer)</summary>

###
[`v8.0.5`](https://togithub.com/dotnet/aspnetcore/releases/tag/v8.0.5):
.NET 8.0.5

[Release](https://togithub.com/dotnet/core/releases/tag/v8.0.5)

##### What's Changed

- \[release/8.0] Update dependencies from dotnet/source-build-externals
by [@&#8203;dotnet-maestro](https://togithub.com/dotnet-maestro) in
[dotnet/aspnetcore#54744
- Update branding to 8.0.5 by
[@&#8203;vseanreesermsft](https://togithub.com/vseanreesermsft) in
[dotnet/aspnetcore#54907
- \[release/8.0] Convert to 1ES templates by
[@&#8203;RussKie](https://togithub.com/RussKie) in
[dotnet/aspnetcore#54660
- Increase logs and delays in CanLaunchPhotinoWebViewAndClickButton by
[@&#8203;Eilon](https://togithub.com/Eilon) in
[dotnet/aspnetcore#54608
- \[release/8.0] (deps): Bump src/submodules/googletest from `31993df`
to `77afe8e` by [@&#8203;dependabot](https://togithub.com/dependabot) in
[dotnet/aspnetcore#54872
- \[release/8.0] Reduce helix-matrix timeout to 5 hours by
[@&#8203;github-actions](https://togithub.com/github-actions) in
[dotnet/aspnetcore#54778
- \[release/8.0] Preserve RemoteAuthenticationContext during trimming if
used in JS interop by [@&#8203;halter73](https://togithub.com/halter73)
in
[dotnet/aspnetcore#54655
- \[release/8.0] Improve usage of `Type.GetType` when activating types
in data protection by
[@&#8203;github-actions](https://togithub.com/github-actions) in
[dotnet/aspnetcore#54762
- \[release/8.0] Fix route analyzer performance with highly concatenated
strings by [@&#8203;github-actions](https://togithub.com/github-actions)
in
[dotnet/aspnetcore#54763
- \[release/8.0] Suppress .ps1 SDL errors by
[@&#8203;wtgodbe](https://togithub.com/wtgodbe) in
[dotnet/aspnetcore#54915
- \[release/8.0] Backport test fixes by
[@&#8203;MackinnonBuck](https://togithub.com/MackinnonBuck) in
[dotnet/aspnetcore#54912
- \[release/8.0] Skip SpotBugs for now by
[@&#8203;wtgodbe](https://togithub.com/wtgodbe) in
[dotnet/aspnetcore#54952
- Merging internal commits for release/8.0 by
[@&#8203;vseanreesermsft](https://togithub.com/vseanreesermsft) in
[dotnet/aspnetcore#55034
- \[release/8.0] Update dependencies from dotnet/arcade by
[@&#8203;dotnet-maestro](https://togithub.com/dotnet-maestro) in
[dotnet/aspnetcore#55061
- \[release/8.0] Update Wix version by
[@&#8203;github-actions](https://togithub.com/github-actions) in
[dotnet/aspnetcore#55101

**Full Changelog**:
dotnet/aspnetcore@v8.0.4...v8.0.5

</details>

<details>
<summary>dotnet/efcore
(Microsoft.EntityFrameworkCore.Relational)</summary>

### [`v8.0.5`](https://togithub.com/dotnet/efcore/releases/tag/v8.0.5):
EF Core 8.0.5

This is a [patch release of EF Core
8.0](https://www.nuget.org/packages/Microsoft.EntityFrameworkCore/8.0.5)
containing only updates to dependencies. There are no additional fixes
in this release beyond those already shipped in EF Core 8.0.4.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "before 7am on Sunday,before 7am on
Wednesday" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about these
updates again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/digdir/dialogporten).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ1cGRhdGVkSW5WZXIiOiIzNy4zNjguMTAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbXX0=-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Ole Jørgen Skogstad <skogstad@softis.net>
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 Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants