From 7518228811b013173dbeda8dc1b722a102c2f95b Mon Sep 17 00:00:00 2001 From: Jason Malinowski Date: Mon, 10 Oct 2022 12:15:11 -0700 Subject: [PATCH] Don't generate a nearly empty file 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. --- ...eneratorInitializationContextExtensions.cs | 22 ++++++++++-------- .../Compiles.cs | 23 ++++++++----------- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs index 0b008795884810..b4a20afc3e7123 100644 --- a/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs +++ b/src/libraries/System.Runtime.InteropServices/gen/Microsoft.Interop.SourceGeneration/IncrementalGeneratorInitializationContextExtensions.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using System.Text; using Microsoft.CodeAnalysis; @@ -27,12 +28,19 @@ public static void RegisterDiagnostics(this IncrementalGeneratorInitializationCo public static void RegisterConcatenatedSyntaxOutputs(this IncrementalGeneratorInitializationContext context, IncrementalValuesProvider nodes, string fileName) where TNode : SyntaxNode { - IncrementalValueProvider generatedMethods = nodes + IncrementalValueProvider> generatedMethods = nodes .Select( static (node, ct) => node.NormalizeWhitespace().ToFullString()) - .Collect() - .Select(static (generatedSources, ct) => + .Collect(); + + context.RegisterSourceOutput(generatedMethods, + (context, generatedSources) => { + // Don't generate a file if we don't have to, to avoid the extra IDE overhead once we have generated + // files in play. + if (generatedSources.IsEmpty) + return; + StringBuilder source = new(); // Mark in source that the file is auto-generated. source.AppendLine("// "); @@ -40,13 +48,9 @@ public static void RegisterConcatenatedSyntaxOutputs(this IncrementalGene { source.AppendLine(generated); } - return source.ToString(); - }); - context.RegisterSourceOutput(generatedMethods, - (context, source) => - { - context.AddSource(fileName, source); + // Once https://github.com/dotnet/roslyn/issues/61326 is resolved, we can avoid the ToString() here. + context.AddSource(fileName, source.ToString()); }); } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs index 8acfd9f4a47def..0b5c612c972576 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/LibraryImportGenerator.UnitTests/Compiles.cs @@ -641,33 +641,30 @@ public async Task ValidateSnippetsWithMultipleSources(string id, string[] source TestUtils.AssertPostSourceGeneratorCompilation(newComp); } - public static IEnumerable CodeSnippetsToCompileToValidateAllowUnsafeBlocks() + public static IEnumerable CodeSnippetsToVerifyNoTreesProduced() { - yield return new object[] { ID(), CodeSnippets.TrivialClassDeclarations, TestTargetFramework.Net, true }; - - { - string source = @" + 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 }; - } + yield return new object[] { ID(), source, TestTargetFramework.Standard }; + yield return new object[] { ID(), source, TestTargetFramework.Framework }; + yield return new object[] { ID(), source, TestTargetFramework.Net }; } [Theory] - [MemberData(nameof(CodeSnippetsToCompileToValidateAllowUnsafeBlocks))] - public async Task ValidateRequireAllowUnsafeBlocksDiagnosticNoTrigger(string id, string source, TestTargetFramework framework, bool allowUnsafe) + [MemberData(nameof(CodeSnippetsToVerifyNoTreesProduced))] + public async Task ValidateNoGeneratedOuptutForNoImport(string id, string source, TestTargetFramework framework) { TestUtils.Use(id); - Compilation comp = await TestUtils.CreateCompilation(source, framework, allowUnsafe: allowUnsafe); + Compilation comp = await TestUtils.CreateCompilation(source, framework, allowUnsafe: false); TestUtils.AssertPreSourceGeneratorCompilation(comp); var newComp = TestUtils.RunGenerators(comp, out var generatorDiags, new Microsoft.Interop.LibraryImportGenerator()); Assert.Empty(generatorDiags); - TestUtils.AssertPostSourceGeneratorCompilation(newComp); + // Assert we didn't generate any syntax trees, even empty ones + Assert.Same(comp, newComp); } } }