Don't generate a nearly empty file

[jasonmalinowski]

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 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.

Fixes #76298

[ghost]

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

Issue Details

---

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 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.

Author:	jasonmalinowski
Assignees:	jasonmalinowski
Labels:	area-System.Runtime.InteropServices
Milestone:	-

[AaronRobinsonMSFT]

Thanks!

[AaronRobinsonMSFT]

@pavelsavara I think we might also need a fix in the JavaScript interop source generator, right?

[jasonmalinowski]

@AaronRobinsonMSFT: as far as I can tell, this is a common helper used for everything, but unless there's another copy somewhere else I don't know about?

[jasonmalinowski]

Marking as draft since I presume this broke some tests, but I don't know where they are yet.

[jkoritzinsky]

LGTM. I believe the JS generator uses the same extension methods to build out the generated sources, so this should cover the JS generators as well.

[jasonmalinowski]

@jkoritzinsky @AaronRobinsonMSFT I assumed this would have broken a test somewhere but I guess there's no negative test asserting no files are produced if nothing is needed. Where should one be added?

[AaronRobinsonMSFT]

I guess there's no negative test asserting no files are produced if nothing is needed

Likely not. I think the most appropriate place would be after the following tests or integrated into them. These are confirming we compile even though unsafe isn't enabled on the project. You could add a bool that indicates if a file should be generated. @elinor-fung do you have any thoughts on location?

runtime/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs

Lines 644 to 671 in 6556178

	public static IEnumerable<object[]> CodeSnippetsToCompileToValidateAllowUnsafeBlocks()
	{
	yield return new object[] { ID(), CodeSnippets.TrivialClassDeclarations, TestTargetFramework.Net, true };
	{
	string source = @"
	using System.Runtime.InteropServices;
	public class Basic { }
	";
	yield return new object[] { ID(), source, TestTargetFramework.Standard, false };
	yield return new object[] { ID(), source, TestTargetFramework.Framework, false };
	yield return new object[] { ID(), source, TestTargetFramework.Net, false };
	}
	}
	[Theory]
	[MemberData(nameof(CodeSnippetsToCompileToValidateAllowUnsafeBlocks))]
	public async Task ValidateRequireAllowUnsafeBlocksDiagnosticNoTrigger(string id, string source, TestTargetFramework framework, bool allowUnsafe)
	{
	TestUtils.Use(id);
	Compilation comp = await TestUtils.CreateCompilation(source, framework, allowUnsafe: allowUnsafe);
	TestUtils.AssertPreSourceGeneratorCompilation(comp);
	var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.LibraryImportGenerator());
	Assert.Empty(generatorDiags);
	TestUtils.AssertPostSourceGeneratorCompilation(newComp);
	}

[elinor-fung]

I think integrating with the one Aaron pointed at makes sense. I would:

• Remove the case with CodeSnippets.TrivialClassDeclarations and allowUnsafe=true
  • This is already covered by ValidateSnippets
• Rename to something like ValidateNoLibraryImport and then always create the compilation with allowUnsafe=false and add a check that no source is generated.

[jasonmalinowski]

@jkoritzinsky @AaronRobinsonMSFT @elinor-fung Test added, so this would need a small re-review. Otherwise I think it's ready to go?

[AaronRobinsonMSFT]

/backport to release/7.0

[github-actions]

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3277608142