Update .Net source generators to use new SyntaxValueProvider.ForAttributeWithMetadataName api.

[CyrusNajmabadi]

Roslyn recently added a new API for greatly speeding up SGs that use the pattern of looking for nodes that have an attribute on it with a particular fully qualified name for the attribute. For example, the json serialization generator looks for this attribute:

runtime/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs

Line 46 in 978df67

private const string JsonSerializableAttributeFullName = "System.Text.Json.Serialization.JsonSerializableAttribute";

Currently, this is extremely expensive as on every edit, the generator must hit every attributed class in every file and semantically check if it has an attribute that binds to this fully qualified name. This may mean thousands of files bound, with expensive semantic models and binding checks performed, all for nodes+attributes completely unrelated to this SG.

The added API provides a new entry point to get the same functionality as previously possible, but with much better performance. Now, the generator says what attributes they care about (passing in teh fully qualified metadata name) and roslyn can smartly examine the source code and not even bother doing any binding or semantics when it would be pointless. Intuitively, you can think of it that when roslyn sees [FooBar] it goes "well, FooBar (and FooBarAttribute) could not ever bind to JsonSerializableAttribute, so it's pointless to even try". This massively reduces the amount of work done (>40x in roslyn itself) especially when the attribute is never, or almost never, used.

Once this api is available in a consumable roslyn package, we should update all the runtime's SGs to use it. That includes the generators for

1. Regex. Done in Polyfill the incremental generator ForAttributeWithMetadataName from roslyn. #70911. This also added the main polyfill code.
2. Json. Done in Polyfill the incremental generator ForAttributeWithMetadataName from roslyn (for JsonGenerator). #71653
3. Logging. Done in Polyfill the incremental generator ForAttributeWithMetadataName from roslyn (for LoggingGenerator). #71651
4. eventsource. Done in Polyfill the incremental generator ForAttributeWithMetadataName from roslyn (for EventSourceGeneration). #71662
5. libraryimport. in progress: Polyfill the incremental generator ForAttributeWithMetadataName from roslyn (for LibraryImportGenerator). #71652

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[CyrusNajmabadi]

Tagging @stephentoub @ViktorHofer for visibility. I'm happy to contribute the changes on the runtime side for this if that can help out.

[jkoritzinsky]

We need to make sure that we not only keep compatibility for downlevel generators (so introduce another build variant of the out-of-band packaged generators that uses the new roslyn API version) and ensure that we don't break uplevel repos like aspnetcore by adopting a Roslyn version that is newer than what many of them use (as we turn on in-box generators by default, which causes build failures when the Roslyn version the upstream repo uses is too old).

Generally, we try to keep the version of Roslyn that the "new" generators build against to at newest the version that ships with the SDK that dotnet/arcade is using to ensure we won't cause compile errors upstack.

runtime/eng/Versions.props

Lines 34 to 39 in 042fdf4

	<!-- Compatibility with VS 17.X/.NET SDK 7.0.1xx -->
	<!--
	This version is a moving target until we ship.
	It should never go ahead of the Roslyn version included in the SDK version in dotnet/arcade's global.json to avoid causing breaks in product construction.
	-->
	<MicrosoftCodeAnalysisVersion_4_X>4.2.0-2.final</MicrosoftCodeAnalysisVersion_4_X>

For generators that don't ship outside of dotnet/runtime, we can update them as soon as we have a NuGet package (so that includes the EventSource generator and the XUnitWrapperGenerator)

[CyrusNajmabadi]

Note: teh major offenders i'm seeing in traces are Json and Logging. So it woudl be great to be able to update these asap and get these into a VS release that has the new updated API.

[jkoritzinsky]

Since logging ships out-of-band, we could add another build dimension to it now against the newer Roslyn API version since we can put it into a different path in the NuGet package based on Roslyn version.

JSON ships in-box too, so we can't update the in-box version without breaking upstream repos, but we could update the package version for downlevel consumers immediately if we wanted to.

Basically we can update every generator project that isn't in the following list in some manner now (either by introducing new projects for the out-of-band generators or updating the in-repo ones directly), and then we can update the ones in the list after we know we won't break upstream repos.

runtime/src/libraries/NetCoreAppLibrary.props

Lines 187 to 191 in 6896e1b

	<NetCoreAppLibraryGenerator>
	LibraryImportGenerator;
	System.Text.Json.SourceGeneration.Roslyn4.0;
	System.Text.RegularExpressions.Generator;
	</NetCoreAppLibraryGenerator>

[danmoseley]

cc @joperezr who I believe is looking at source generator perf at the moment.

Tagging subscribers to this area: @dotnet/area-meta
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Roslyn recently added a new API for greatly speeding up SGs that use the pattern of looking for nodes that have an attribute on it with a particular fully qualified name for the attribute. For example, the json serialization generator looks for this attribute:

runtime/src/libraries/System.Text.Json/gen/JsonSourceGenerator.Parser.cs

Line 46 in 978df67

private const string JsonSerializableAttributeFullName = "System.Text.Json.Serialization.JsonSerializableAttribute";

Currently, this is extremely expensive as on every edit, the generator must hit every attributed class in every file and semantically check if it has an attribute that binds to this fully qualified name. This may mean thousands of files bound, with expensive semantic models and binding checks performed, all for nodes+attributes completely unrelated to this SG.

The added API provides a new entry point to get the same functionality as previously possible, but with much better performance. Now, the generator says what attributes they care about (passing in teh fully qualified metadata name) and roslyn can smartly examine the source code and not even bother doing any binding or semantics when it would be pointless. Intuitively, you can think of it that when roslyn sees [FooBar] it goes "well, FooBar (and FooBarAttribute) could not ever bind to JsonSerializableAttribute, so it's pointless to even try". This massively reduces the amount of work done (>40x in roslyn itself) especially when the attribute is never, or almost never, used.

Once this api is available in a consumable roslyn package, we should update all the runtime's SGs to use it. That includes the generators for

1. Json
2. Regex
3. Logging
4. libraryimport
5. eventsource
6. Client.SourceGenerator (asp.net: https://github.com/dotnet/aspnetcore/tree/3ea008c80d5cc63de7f90ddfd6823b7b006251ff/src/SignalR/clients/csharp/Client.SourceGenerator/src)

@jaredpar has prototyped this for a couple here:
909709a
c22809d

The changes are straightforward and easy to adopt.

Author:	CyrusNajmabadi
Assignees:	-
Labels:	area-Meta, tenet-performance, untriaged, source-generator
Milestone:	-

[CyrusNajmabadi]

@stephentoub @jkoritzinsky

Basically we can update every generator project that isn't in the following list in some manner now (either by introducing new projects for the out-of-band generators or updating the in-repo ones directly), and then we can update the ones in the list after we know we won't break upstream repos.

There's a potential hybrid option we can take.

1. use the new roslyn API in all the generators where it is safe to do so immediately.
2. port most** of the implementation of hte new API over to the runtime repo for the generators that cannot move over immediately. This would allow us to get the perf benefit without having to wait a very long.

So what does most** mean? Well, currently the API depends on some internal functionality in order to support a very unlikely, but useful full correctness case. Specifically, the API will understand if you do the following:

File1:

global alias FooBarAttribute = System.Text.Json.Serialization.JsonSerializableAttribute;

File2:

[FooBar]
class Whatever { }

In other words, it knows that that global alias exists, and as such the json-generator should run on Whatever. If you were willing to say that that was a currently unsupported scenario, we could port over the entirety of the new API except teh part that supports global aliases. Practically speaking, i think all users would still be fine. It would be up to you if this functionality break would be worth the massive speedup we're seeing (over 40x in roslyn itself).

To me, given just how bad the perf problem is here, my preference is that we take the perf gain for all the generators, and we accept a highly unlikely functionality break. Then, when you can finally move all generators over to the new api we'll have teh best of both worlds. High performance, and complete correctness.

Any thoughts from your perspective?

[ViktorHofer]

Here are my thoughts on this:

• We should only bump code analysis dependencies if they work with publicly released Visual Studio versions (including the preview channel).
• For packages, we already include roslyn3.x source generators so that our packages work on older Visual Studio releases. I don't think we want to add another analyzer version as we already have the fall back version. It's impossible to multi-target based on different code analysis dependencies today in a single project, therefore you need to create a dedicated project per roslyn version to be targeted which adds up to the overall build.

Based on that I would propose the following:

• We should upgrade non-shipping roslyn4.x source generators to leverage the new API when publicly released tooling supports them. My assumption is that this will be VS 17.4 but I could be mistaken EDIT: VS 17.3 Preview 3.
• We should upgrade shipping roslyn4.x source generators to leverage the new API when dependent repositories upgraded to the newer code analysis version. If I'm not mistaken, the roslyn version that this depends on, will ship with .NET 7 Preview 6. Based on that, such changes would need to target .NET 7 RC1 or RC2.
• It might make sense to change the roslyn4.0 folder in our packages in which the source generators reside to the actual roslyn version that we will depend on when shipping .NET 7. If I'm not mistaken, that would be roslyn4.3 with this change. Presumably the SDK falls back to use the roslyn3.11 targeting source generators if a Visual Studio version is used that doesn't support roslyn4.3.

cc @eerhardt @jaredpar

[eerhardt]

It might make sense to change the roslyn4.0 folder in our packages in which the source generators reside to the actual roslyn version that we will depend on when shipping .NET 7. If I'm not mistaken, that would be roslyn4.4 with this change. Presumably the SDK falls back to use the roslyn3.11 targeting source generators if a Visual Studio version is used that doesn't support roslyn4.4.

The issue with this proposal is that projects building with the 6.0.x00 SDK will now see degraded performance when they use the 7.0.0 System.Text.Json nupkg. With the 6.0.x System.Text.Json nupkg, they will use the roslyn4.0 version, which has incremental build support. When the project upgrades to the 7.0.0 nupkg, there is no roslyn4.0 built version, so they will fall back to roslyn3.11 and now all the incremental support we took advantage of in .NET 6 is removed.

It is pretty easy to get into this situation AFAICT. You just need to be on a VS 2022 version that doesn't include .NET 7 - so let's say VS 17.0. You are using the 6.0.0 System.Text.Json nupkg, and NuGet Manager says a new version is out in the "Updates" tab, so you update to STJ 7.0.0.

[ViktorHofer]

Agreed, that's a problem and needs to be solved, thanks for pointing that out. What makes this problematic is that we currently don't have a lifecycle policy for source generators when they ship inside packages. I just recently had this discussion with @eerhardt.

Visual Studio 2019, which only works with the roslyn3.11 source generators is supported until April 2029. Based on that, we would need to build and package our roslyn3.11 source generators for the next 6-7 years.

Visual Studio 2022 17.0 is supported until July 2023, 17.2 until January 2024 and the support date for 17.3 isn't yet announced. Because of these support dates, we would need to keep our roslyn4.0 targeted source generators around until at least .NET 9.

It's very likely that we will again depend on newer roslyn features going forward and with that build and package even more versions of our source generators. Packages will grow unbounded and the main branch hosts all different versions of the roslyn source generators.

I don't think there's a good solution to mitigate the increasing build complexity, package growth and ever expanding matrix of source generators. One option that we might want to discuss is to couple our package support policy with toolset versions. Just theoretically speaking, we could mark the System.Text.Json 7.0 package to not support Visual Studio 2019 or older SDKs that don't have a specific roslyn version available. I don't suggest that we should to that but wanted mention it.