Avoid concurrent use of ConditionalWeakTable on net472

[sharwell]

Avoids a read lock in ConditionalWeakTable for .NET Framework, which results in extreme contention on concurrent paths. For source generators producing large amounts of code in parallel, the default use of ElasticMarker made most SyntaxFactory methods completely unusable (e.g. CsWin32 wrote a complete new syntax factory just to avoid this).

The clear downside of this change is it increases the size of GreenNode by 8 bytes when executing on .NET Framework (including inside Visual Studio), which is heavily allocated.

Fixes AB#1420484

[sharwell]

This is going to be a tough call since the change offers significant benefits to one scenario (heavy use of SyntaxFactory) with a non-trivial disadvantage across all .NET Framework usage. The problems resolve themselves when .NET Core / .NET is used, but some environments including Visual Studio and certain builds still use .NET Framework.

/cc @jaredpar @cston @VSadov

/cc @davkean @AArnott

[jaredpar]

Seems like we should start a permanent exception thread with the VS team before taking this PR. Seemingly we need to decide between the idea of taking a higher allocation hit here or living with the contention issue on .NET Framework. Those are the only viable options that we have on the table right now.

Certainly open to other ideas here but thus far those seem like the only two viable ones.

[davkean]

This has a pretty wild stats in the wild, it's ranked #3 of all thread pool issues in 16.11:

Failure	Score	Session %	Cab Count	Threads 90th	Threads 50th	Avg Threads
Stk_ThdPl_Microsoft_CodeAnalysis!Microsoft.CodeAnalysis.GreenNode.GetAnnotations()	313	0.06	16	260	11	66

While it's not seen in a large number of cabs, when it hits, it hits hard.

[CyrusNajmabadi]

it increases the size of GreenNode by 8 bytes

Whooboy...

when executing on .NET Framework (including inside Visual Studio

Ok. That seems less impactful for me. With VS2022 we are now doing all automatic processing of files (outside of those opened/edited) in our external OOP process.

So you should only potentially run into something here specifically when doing something like a Fix-All or large refactoring that actually touches a significant amount of the in-process solution.

If we can get those things running OOP, then we greatly mitigate that single vector for issues here.

--

Overall then, i think this seems ok.

[AlekseyTs]

var

Please spell out the type.

[sharwell]

➡️ Changed. If this is a common item for code reviews in this path, to save time and make reviews less negative consider updating .editorconfig settings to reveal the situation during initial development.