Skip to content

Commit 06b6b9a

Browse files
[Backport] Always run 'required' generators: (#80058)
- Add a CreateOnlyRequired generation state - Add a filter that only runs razor when required generators are requested Manual backport of #79510
2 parents afeda84 + c14a3a0 commit 06b6b9a

10 files changed

+490
-57
lines changed

src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs

Lines changed: 281 additions & 18 deletions
Large diffs are not rendered by default.

src/Workspaces/Core/Portable/SourceGeneration/IRemoteSourceGenerationService.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ ValueTask<ImmutableArray<string>> GetContentsAsync(
4747
Checksum solutionChecksum, ProjectId projectId, ImmutableArray<DocumentId> documentIds, bool withFrozenSourceGeneratedDocuments, CancellationToken cancellationToken);
4848

4949
/// <summary>
50-
/// Whether or not the specified analyzer references have source generators or not.
50+
/// Whether or not the specified analyzer references have source generators, and which kind of generators they have if they do.
5151
/// </summary>
52-
ValueTask<bool> HasGeneratorsAsync(
52+
ValueTask<SourceGeneratorPresence> GetSourceGeneratorPresenceAsync(
5353
Checksum solutionChecksum, ProjectId projectId, ImmutableArray<Checksum> analyzerReferenceChecksums, string language, CancellationToken cancellationToken);
5454

5555
/// <summary>
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Immutable;
6+
using System.Linq;
7+
8+
namespace Microsoft.CodeAnalysis.SourceGeneration;
9+
10+
internal static class SourceGeneratorExtensions
11+
{
12+
/// <summary>
13+
/// Determines if this generator is considered required, and should still run when the
14+
/// solution <see cref="SolutionCompilationState.GeneratedDocumentCreationPolicy"/> is set to
15+
/// <see cref="SolutionCompilationState.GeneratedDocumentCreationPolicy.CreateOnlyRequired"/>.
16+
/// </summary>
17+
/// <param name="generator">The generator to test</param>
18+
/// <returns><c>True</c> if the generator is considered 'required'</returns>
19+
/// <remarks>
20+
/// Currently, only Razor is considered to be a required generator.
21+
/// </remarks>
22+
public static bool IsRequiredGenerator(this ISourceGenerator generator)
23+
{
24+
// For now, we hard code the required generator list to Razor.
25+
// In the future we might want to expand this to e.g. run any generators with open generated files
26+
return generator.GetGeneratorType().FullName == "Microsoft.NET.Sdk.Razor.SourceGenerators.RazorSourceGenerator";
27+
}
28+
29+
/// <summary>
30+
/// Determines the presence and type of source generators in the specified collection.
31+
/// </summary>
32+
/// <returns>A value indicating whether the collection contains no source generators, only optional source generators, or at
33+
/// least one required source generator.</returns>
34+
public static SourceGeneratorPresence GetSourceGeneratorPresence(this ImmutableArray<ISourceGenerator> generators)
35+
{
36+
if (generators.IsDefaultOrEmpty)
37+
return SourceGeneratorPresence.NoSourceGenerators;
38+
39+
var hasRequiredGenerators = generators.Any(g => g.IsRequiredGenerator());
40+
return hasRequiredGenerators
41+
? SourceGeneratorPresence.ContainsRequiredSourceGenerators
42+
: SourceGeneratorPresence.OnlyOptionalSourceGenerators;
43+
}
44+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
namespace Microsoft.CodeAnalysis.SourceGeneration;
6+
7+
/// <summary>
8+
/// Indicates the presence and types of source generators in a project.
9+
/// </summary>
10+
internal enum SourceGeneratorPresence
11+
{
12+
/// <summary>
13+
/// The project contains no source generators.
14+
/// </summary>
15+
NoSourceGenerators,
16+
17+
/// <summary>
18+
/// The project contains source generators, but none of them are considered "required".
19+
/// These generators may be skipped in certain scenarios for performance reasons.
20+
/// </summary>
21+
OnlyOptionalSourceGenerators,
22+
23+
/// <summary>
24+
/// The project contains at least one required source generator that must always run.
25+
/// The project may also contain additional optional generators.
26+
/// </summary>
27+
ContainsRequiredSourceGenerators
28+
}

src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.CreationPolicy.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@ private enum GeneratedDocumentCreationPolicy
1616
/// </summary>
1717
Create,
1818

19+
/// <summary>
20+
/// Source generators that are considered required should be run and produce results. Previously
21+
/// computed results should be reused for other generators.
22+
/// </summary>
23+
/// <remarks>
24+
/// Today the only required generator is Razor.
25+
/// </remarks>
26+
CreateOnlyRequired,
27+
1928
/// <summary>
2029
/// Source generators should not run. Whatever results were previously computed should be reused.
2130
/// </summary>

src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.ICompilationTracker.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ bool ContainsAssemblyOrModuleOrDynamic(
4747
ICompilationTracker WithCreateCreationPolicy(bool forceRegeneration);
4848

4949
/// <summary>
50-
/// Updates the creation policy for this tracker. Setting it to <see cref="CreationPolicy.DoNotCreate"/>.
50+
/// Updates the creation policy for this tracker. Setting it to <see cref="CreationPolicy.DoNotCreate"/>.
5151
/// </summary>
5252
ICompilationTracker WithDoNotCreateCreationPolicy();
5353

src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.RegularCompilationTracker.cs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -589,21 +589,21 @@ await compilationState.GetCompilationAsync(
589589
cancellationToken).ConfigureAwait(false);
590590

591591
// Our generated documents are up to date if we just created them. Note: when in balanced mode, we
592-
// will then change our creation policy below to DoNotCreate. This means that any successive forks
593-
// will move us to an in-progress-state that is not running generators. And the next time we get
594-
// here and produce a final compilation, this will then be 'false' since we'll be reusing old
595-
// generated docs.
592+
// will then change our creation policy below to CreateOnlyRequired. This means that any successive
593+
// forks will move us to an in-progress-state that only runs required generators. The next time we get
594+
// here and produce a final compilation, this will then be 'false' since we'll only be generating required
595+
// docs and combining them with the old docs from other generators.
596596
//
597597
// This flag can then be used later when we hear about external user events (like save/build) to
598598
// decide if we need to do anything. If the generated documents are up to date, then we don't need
599599
// to do anything in that case.
600600
var generatedDocumentsUpToDate = creationPolicy.GeneratedDocumentCreationPolicy == GeneratedDocumentCreationPolicy.Create;
601601

602602
// If the user has the option set to only run generators to something other than 'automatic' then we
603-
// want to set ourselves to not run generators again now that generators have run. That way, any
604-
// further *automatic* changes to the solution will not run generators again. Instead, when one of
605-
// those external events happen, we'll grab the workspace's solution, transition all states *out* of
606-
// this state and then let the next 'GetCompilationAsync' operation cause generators to run.
603+
// want to set ourselves to only run required generators now that all other generators have run. That way,
604+
// any further *automatic* changes to the solution will only run the required generators again.
605+
// When one of those external events happen, we'll grab the workspace's solution, transition all states
606+
// *out* of this state and then let the next 'GetCompilationAsync' operation cause all generators to run.
607607
//
608608
// Similarly, we don't want to automatically create skeletons at this point (unless they're missing
609609
// entirely).
@@ -612,7 +612,7 @@ await compilationState.GetCompilationAsync(
612612
if (workspacePreference != SourceGeneratorExecutionPreference.Automatic)
613613
{
614614
if (creationPolicy.GeneratedDocumentCreationPolicy == GeneratedDocumentCreationPolicy.Create)
615-
creationPolicy = creationPolicy with { GeneratedDocumentCreationPolicy = GeneratedDocumentCreationPolicy.DoNotCreate };
615+
creationPolicy = creationPolicy with { GeneratedDocumentCreationPolicy = GeneratedDocumentCreationPolicy.CreateOnlyRequired };
616616

617617
if (creationPolicy.SkeletonReferenceCreationPolicy == SkeletonReferenceCreationPolicy.Create)
618618
creationPolicy = creationPolicy with { SkeletonReferenceCreationPolicy = SkeletonReferenceCreationPolicy.CreateIfAbsent };

src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.RegularCompilationTracker_Generators.cs

Lines changed: 74 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@ private sealed partial class RegularCompilationTracker : ICompilationTracker
3636
Compilation? compilationWithStaleGeneratedTrees,
3737
CancellationToken cancellationToken)
3838
{
39-
if (creationPolicy.GeneratedDocumentCreationPolicy is GeneratedDocumentCreationPolicy.DoNotCreate)
39+
var canSkipRunningGenerators = await CanSkipRunningGeneratorsAsync(creationPolicy, compilationState, cancellationToken).ConfigureAwait(false);
40+
if (canSkipRunningGenerators)
4041
{
41-
// We're frozen. So we do not want to go through the expensive cost of running generators. Instead, we
42-
// just whatever prior generated docs we have.
42+
// We're either frozen, or we only want required generators and know that there aren't any to run, so we
43+
// do not want to go through the expensive cost of running generators. Instead, we just use whatever
44+
// prior generated docs we have.
4345
var generatedSyntaxTrees = await generatorInfo.Documents.States.Values.SelectAsArrayAsync(
4446
static (state, cancellationToken) => state.GetSyntaxTreeAsync(cancellationToken), cancellationToken).ConfigureAwait(false);
4547

@@ -69,9 +71,33 @@ private sealed partial class RegularCompilationTracker : ICompilationTracker
6971
generatorInfo.Documents,
7072
generatorInfo.Driver,
7173
compilationWithStaleGeneratedTrees,
74+
creationPolicy.GeneratedDocumentCreationPolicy,
7275
cancellationToken).ConfigureAwait(false);
7376
return (compilationWithGeneratedFiles, new(nextGeneratedDocuments, nextGeneratorDriver));
7477
}
78+
79+
async ValueTask<bool> CanSkipRunningGeneratorsAsync(CreationPolicy creationPolicy, SolutionCompilationState compilationState, CancellationToken cancellationToken)
80+
{
81+
// if we don't want to create generated documents, we can skip
82+
if (creationPolicy.GeneratedDocumentCreationPolicy is GeneratedDocumentCreationPolicy.DoNotCreate)
83+
return true;
84+
85+
// if we only want required documents, we can skip if we don't have any required generators
86+
if (creationPolicy.GeneratedDocumentCreationPolicy is GeneratedDocumentCreationPolicy.CreateOnlyRequired)
87+
{
88+
var hasRequiredGenerators = await HasRequiredGeneratorsAsync(compilationState, cancellationToken).ConfigureAwait(false);
89+
return !hasRequiredGenerators;
90+
}
91+
92+
// we need to run generators
93+
return false;
94+
}
95+
}
96+
97+
private async Task<bool> HasRequiredGeneratorsAsync(SolutionCompilationState compilationState, CancellationToken cancellationToken)
98+
{
99+
var presence = await compilationState.GetProjectGeneratorPresenceAsync(ProjectState.Id, cancellationToken).ConfigureAwait(false);
100+
return presence is SourceGeneratorPresence.ContainsRequiredSourceGenerators;
75101
}
76102

77103
private async Task<(Compilation compilationWithGeneratedFiles, TextDocumentStates<SourceGeneratedDocumentState> generatedDocuments)?> TryComputeNewGeneratorInfoInRemoteProcessAsync(
@@ -236,12 +262,16 @@ await newGeneratedDocuments.States.Values.SelectAsArrayAsync(
236262
TextDocumentStates<SourceGeneratedDocumentState> oldGeneratedDocuments,
237263
GeneratorDriver? generatorDriver,
238264
Compilation? compilationWithStaleGeneratedTrees,
265+
GeneratedDocumentCreationPolicy creationPolicy,
239266
CancellationToken cancellationToken)
240267
{
241268
// If we don't have any source generators. Trivially bail out.
242269
if (!await compilationState.HasSourceGeneratorsAsync(this.ProjectState.Id, cancellationToken).ConfigureAwait(false))
243270
return (compilationWithoutGeneratedFiles, TextDocumentStates<SourceGeneratedDocumentState>.Empty, generatorDriver);
244271

272+
// Hold onto the prior results so we can compare when filtering
273+
var priorRunResult = generatorDriver?.GetRunResult();
274+
245275
// If we don't already have an existing generator driver, create one from scratch
246276
generatorDriver ??= CreateGeneratorDriver(this.ProjectState);
247277

@@ -268,7 +298,7 @@ await newGeneratedDocuments.States.Values.SelectAsArrayAsync(
268298
var compilationToRunGeneratorsOn = compilationWithoutGeneratedFiles.RemoveSyntaxTrees(treesToRemove);
269299
// END HACK HACK HACK HACK.
270300

271-
generatorDriver = generatorDriver.RunGenerators(compilationToRunGeneratorsOn, cancellationToken);
301+
generatorDriver = generatorDriver.RunGenerators(compilationToRunGeneratorsOn, ShouldGeneratorRun, cancellationToken);
272302

273303
Contract.ThrowIfNull(generatorDriver);
274304

@@ -425,6 +455,46 @@ static void CheckGeneratorDriver(GeneratorDriver generatorDriver, ProjectState p
425455

426456
Contract.ThrowIfFalse(additionalTexts.Length == projectState.AdditionalDocumentStates.Count);
427457
}
458+
459+
bool ShouldGeneratorRun(GeneratorFilterContext context)
460+
{
461+
// We should never try and run a generator driver if we're not expecting to do any work
462+
Contract.ThrowIfTrue(creationPolicy is GeneratedDocumentCreationPolicy.DoNotCreate);
463+
464+
// If we're in Create mode, we're always going to run all generators
465+
if (creationPolicy is GeneratedDocumentCreationPolicy.Create)
466+
return true;
467+
468+
// If we get here we expect to be in CreateOnlyRequired. Throw to ensure we catch if someone adds a new state
469+
Contract.ThrowIfFalse(creationPolicy is GeneratedDocumentCreationPolicy.CreateOnlyRequired);
470+
471+
// We want to only run required generators, but it's also possible that there are generators that
472+
// have never been run (for instance, an AddGenerator operation might have occurred between runs).
473+
// Our model is that it's acceptable for documents to be slightly out of date, but it is
474+
// fundamentally incorrect to have *no* documents for a generator that could be producing them.
475+
476+
// If there was no prior run result, then we can't have any documents for this generator, so we
477+
// need to re-run it.
478+
if (priorRunResult is null)
479+
return true;
480+
481+
// Next we need to check if this particular generator was run as part of the prior driver execution.
482+
// Either we have no state for the generator, in which case it can't have run. If we do have state,
483+
// the contract from the generator driver is that a generator that hasn't run yet produces a default
484+
// ImmutableArray for GeneratedSources. Note that this is different from an empty array, which
485+
// indicates that the generator ran, but didn't produce any documents:
486+
487+
// - GeneratedSources == default ImmutableArray: the generator was not invoked during that run (must run).
488+
// - GeneratedSources == non-default empty array: the generator ran but produced no documents (may skip).
489+
// - GeneratedSources == non-default non-empty array: the generator ran and produced documents (may skip).
490+
491+
if (!priorRunResult.Results.Any(r => r.Generator == context.Generator && !r.GeneratedSources.IsDefault))
492+
return true;
493+
494+
// We have results for this generator, and we're in CreateOnlyRequired, so only run this generator if
495+
// we consider it to be required.
496+
return context.Generator.IsRequiredGenerator();
497+
}
428498
}
429499
}
430500
}

0 commit comments

Comments
 (0)