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/6.0-rc1] Make JsonGenerator be an incremental generator (#57088) #58278

Merged
merged 1 commit into from
Aug 27, 2021

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented Aug 27, 2021

Backport of #57088 to release/6.0-rc1

Contributes to #56702

Customer Impact

Customers are reporting sluggish IDE behavior when targeting net6.0 in large solutions.

VS typing performance quickly degrades (hangs, up to several seconds pauses between characters typing, etc.) after working with several cs files for like 20 minutes or more in the context of a medium-sized DotNet 6 Preview 6 solution with maybe 70 projects, each with ~ 10 to 500 files.

This change is converting the System.Text.Json source generator to use the new IIncrementalGenerator Roslyn API. This allows for the source generator to do considerably less work in the background of the IDE, especially when no code in the solution uses the source generator.

Testing

All automated tests continue to pass.

I also tested manually using VS 2022 that the source generator still works after converting to the new Roslyn API.

The responsiveness of VS 2022 in a large solution on my Standard_D8as_v4 Azure VM went from ~2 seconds to open the Intellisense window to almost instantaneous.

Risk

Since the IIncrementalGenerator Roslyn API is new in Roslyn v4.0, it doesn't work in VS 2019. This means a customer using the 6.0-rc1 NuGet package in VS 2019 will get a warning:

Warning CS8032 An instance of analyzer System.Text.Json.SourceGeneration.JsonSourceGenerator cannot be created from C:\Users\eerhardt\.nuget\packages\system.text.json\7.0.0-dev\analyzers\dotnet\cs\System.Text.Json.SourceGeneration.dll : Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified.

The plan is to enable "multi-targeting" different Roslyn versions in 6.0-rc2. This will allow us to ship 2 versions of the source generator in the NuGet package - one targeting the 3.x Roslyn APIs and one targeting the 4.0 Roslyn APIs, with the performance of the 4.0 Roslyn version being much better.

cc @chsienki @ericstj @jaredpar @sharwell

* Make JsonGenerator be an incremental generator

* Improve incrementalism by doing less work when not applicable

* Change SourceGeneration.UnitTests to SourceGeneration.Unit.Tests so it is built and executed in CI

* Get unit tests running after IIncrementalGenerator migration

* Fix duplicate file name tests by working around dotnet/roslyn#54185.

* Fix unit tests now that they are running in CI against non-English languages.

* Fix System.Text.Json.SourceGeneration.Unit.Tests on WASM

* Disable STJ.SourceGeneration.Unit.Tests on Browser

Co-authored-by: Eric Erhardt <[email protected]>
@ghost
Copy link

ghost commented Aug 27, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #57088 to release/6.0-rc1

Contributes to #56702

Customer Impact

Customers are reporting sluggish IDE behavior when targeting net6.0 in large solutions.

VS typing performance quickly degrades (hangs, up to several seconds pauses between characters typing, etc.) after working with several cs files for like 20 minutes or more in the context of a medium-sized DotNet 6 Preview 6 solution with maybe 70 projects, each with ~ 10 to 500 files.

This change is converting the System.Text.Json source generator to use the new IIncrementalGenerator Roslyn API. This allows for the source generator to do considerably less work in the background of the IDE, especially when no code in the solution uses the source generator.

Testing

All automated tests continue to pass.

I also tested manually using VS 2022 that the source generator still works after converting to the new Roslyn API.

Risk

Since the IIncrementalGenerator Roslyn API is new in Roslyn v4.0, it doesn't work in VS 2019. This means a customer using the 6.0-rc1 NuGet package in VS 2019 will get a warning:

Warning CS8032 An instance of analyzer System.Text.Json.SourceGeneration.JsonSourceGenerator cannot be created from C:\Users\eerhardt\.nuget\packages\system.text.json\7.0.0-dev\analyzers\dotnet\cs\System.Text.Json.SourceGeneration.dll : Could not load file or assembly 'Microsoft.CodeAnalysis, Version=4.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. The system cannot find the file specified.

The plan is to enable "multi-targeting" different Roslyn versions in 6.0-rc2. This will allow us to ship 2 versions of the source generator in the NuGet package - one targeting the 3.x Roslyn APIs and one targeting the 4.0 Roslyn APIs, with the performance of the 4.0 Roslyn version being much better.

cc @chsienki @ericstj @jaredpar @sharwell

Author: eerhardt
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@eerhardt eerhardt added the Servicing-consider Issue for next servicing release review label Aug 27, 2021
@Anipik
Copy link
Contributor

Anipik commented Aug 27, 2021

approved offline

@Anipik Anipik added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Aug 27, 2021
@Anipik Anipik merged commit 72f18b5 into dotnet:release/6.0-rc1 Aug 27, 2021
@eerhardt eerhardt deleted the BackPort57088 branch August 27, 2021 22:48
@ghost ghost locked as resolved and limited conversation to collaborators Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants