[release/7.0] Don't generate a nearly empty file #77187
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
When generators are running in the Visual Studio IDE, there's some overhead to having to manage a project that actually has generated output. It requires us to maintain two Roslyn Compilation objects, each which have their own symbol information. These interop generators are producing a file that's effectively empty (just an <auto-generated> comment on the top), and since they're installed in all .NET 7.0 applications, they are the reason we'll be having to manage more memory than before. Since the fix is simple enough to only generate the output if necessary, we should do so. This also will help with telemetry, since we have telemetry that lets us tracking in the wild which generators are producing how many files; if we're always producing exactly one file we won't know how many sessions out there are actually using this generator in a meaningful way.
|
Tagging subscribers to this area: @dotnet/interop-contrib Issue DetailsBackport of #76835 to release/7.0 /cc @AaronRobinsonMSFT @jasonmalinowski Customer ImpactTestingRiskIMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.
|
carlossanlop
requested review from
elinor-fung,
AaronRobinsonMSFT and
jkoritzinsky
|
@jasonmalinowski @AaronRobinsonMSFT when this is ready, please add the |
jasonmalinowski
added
the
Servicing-consider
|
I wonder if we can collect these common source generator requirements in the cookbook and in common/shared tests? We've definitely had a set of generic requirements around generators behavior / performance. CC @joperezr @sharwell @CyrusNajmabadi @chsienki |
|
@AaronRobinsonMSFT / @jkoritzinsky / @elinor-fung can we please get a code review sign off here? |
|
@jasonmalinowski please send the email to Tactics if possible, although since you already added the label, we will discuss it in the meeting tomorrow. Sending the email never hurts 😄 . |
|
@carlossanlop: I sent one yesterday evening, did it not get through somehow? |
...en/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs
Show resolved
Hide resolved
src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs
Show resolved
Hide resolved
Oh I see why I couldn't find the email: the subject is different to the title of this PR: "Tweak to change output of import generators if the generators aren't being used". |
carlossanlop
added
Servicing-approved
|
Approved by Tactics via email. All that's left is addressing the open feedback. |
|
CI failure unrelated (unable to load the Json DLL in a Console test). |
ghost
locked as resolved and limited conversation to collaborators
Backport of #76835 to release/7.0
/cc @AaronRobinsonMSFT @jasonmalinowski
Customer Impact
.NET 7 is shipping with some new Roslyn source generators, specifically the JS Import Generator and the LibraryImport generators. Unlike other generators we’ve shipped in the past, if these generators have nothing to generate, they still “generate” an empty file which is passed to Roslyn. This has two notable impacts:
Testing
Unit tests were added.
Risk
Low -- the only behavior change to the generator is instead of outputting an empty file with a "autogenerated" comment, we output no file at all. No change is otherwise made to the generator behavior.