Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid running generators when nothing has changed in a compilation tracker. #73897

Merged
merged 6 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions src/VisualStudio/Core/Test.Next/Services/ServiceHubServicesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,108 @@ internal async Task TestSourceGenerationExecution_SolutionAndProjectChange_2(Sou
Assert.Equal(initialSolution.GetSourceGeneratorExecutionVersion(projectId2).IncrementMajorVersion(), currentSolution.GetSourceGeneratorExecutionVersion(projectId2));
}

[Theory, CombinatorialData]
internal async Task TestSourceGenerationExecution_NoChange_ButExternalUpdateSignal(
SourceGeneratorExecutionPreference executionPreference,
bool forceRegeneration)
{
using var workspace = CreateWorkspace([typeof(TestWorkspaceConfigurationService)]);

var globalOptionService = workspace.ExportProvider.GetExportedValue<IGlobalOptionService>();
globalOptionService.SetGlobalOption(WorkspaceConfigurationOptionsStorage.SourceGeneratorExecution, executionPreference);

var callCount = 0;
AddSimpleDocument(workspace, new CallbackGenerator(() => ("hintName.cs", "// callCount: " + callCount++)));

var project = workspace.CurrentSolution.Projects.Single();
var documents = await project.GetSourceGeneratedDocumentsAsync();

var document = Assert.Single(documents);
Assert.Equal("// callCount: 0", (await document.GetTextAsync()).ToString());

workspace.EnqueueUpdateSourceGeneratorVersion(projectId: null, forceRegeneration);
await WaitForSourceGeneratorsAsync(workspace);

project = workspace.CurrentSolution.Projects.Single();
documents = await project.GetSourceGeneratedDocumentsAsync();

document = Assert.Single(documents);

if (forceRegeneration)
{
// In balanced/automatic mode, we were asked to force regenerate. So that should be respected.
Assert.Equal("// callCount: 1", (await document.GetTextAsync()).ToString());
}
else
{
// In balanced or automatic mode, since nothing happened and we were not forced, we should not regenerate.
Assert.Equal("// callCount: 0", (await document.GetTextAsync()).ToString());
}
}

[Theory, CombinatorialData]
internal async Task TestSourceGenerationExecution_DocumentChange_ButExternalUpdateSignal(
SourceGeneratorExecutionPreference executionPreference,
bool forceRegeneration,
bool enqueueChangeBeforeEdit,
bool enqueueChangeAfterEdit)
{
using var workspace = CreateWorkspace([typeof(TestWorkspaceConfigurationService)]);

var globalOptionService = workspace.ExportProvider.GetExportedValue<IGlobalOptionService>();
globalOptionService.SetGlobalOption(WorkspaceConfigurationOptionsStorage.SourceGeneratorExecution, executionPreference);

var callCount = 0;
var normalDocId = AddSimpleDocument(workspace, new CallbackGenerator(() => ("hintName.cs", "// callCount: " + callCount++)));

var project = workspace.CurrentSolution.Projects.Single();
var documents = await project.GetSourceGeneratedDocumentsAsync();

var document = Assert.Single(documents);
Assert.Equal("// callCount: 0", (await document.GetTextAsync()).ToString());

if (enqueueChangeBeforeEdit)
workspace.EnqueueUpdateSourceGeneratorVersion(projectId: null, forceRegeneration);
await WaitForSourceGeneratorsAsync(workspace);

// Now, make a simple edit to the main document.
Contract.ThrowIfFalse(workspace.TryApplyChanges(workspace.CurrentSolution.WithDocumentText(normalDocId, SourceText.From("// new text"))));

if (enqueueChangeAfterEdit)
workspace.EnqueueUpdateSourceGeneratorVersion(projectId: null, forceRegeneration);
await WaitForSourceGeneratorsAsync(workspace);

project = workspace.CurrentSolution.Projects.Single();
documents = await project.GetSourceGeneratedDocumentsAsync();

document = Assert.Single(documents);

if (executionPreference == SourceGeneratorExecutionPreference.Automatic)
{
// in automatic mode we always rerun after a doc edit.
Assert.Equal("// callCount: 1", (await document.GetTextAsync()).ToString());
return;
}

if (forceRegeneration && (enqueueChangeBeforeEdit || enqueueChangeAfterEdit))
{
// If a force-regenerate notification came through either before or after the edit, we should regenerate.
Assert.Equal("// callCount: 1", (await document.GetTextAsync()).ToString());
return;
}

if (enqueueChangeAfterEdit)
{
// In balanced mode, if we hear about a save/build after a the last change to a project, we do want to regenerate.
Assert.Equal("// callCount: 1", (await document.GetTextAsync()).ToString());
}
else
{
// In balanced mode, if there was no save/build after the last change, we want to reuse whatever we produced last time.
Assert.Equal("// callCount: 0", (await document.GetTextAsync()).ToString());
}
}

private static async Task<Solution> VerifyIncrementalUpdatesAsync(
TestWorkspace localWorkspace,
Workspace remoteWorkspace,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,18 @@ private sealed class FinalCompilationTrackerState : CompilationTrackerState
/// </summary>
public readonly Compilation FinalCompilationWithGeneratedDocuments;

/// <summary>
/// Whether or not this final compilation state *just* generated documents which exactly correspond to the
/// state of the compilation. False if the generated documents came from a point in the past, and are being
/// carried forward until the next time we run generators.
/// </summary>
public readonly bool GeneratedDocumentsUpToDate;

public override Compilation CompilationWithoutGeneratedDocuments { get; }

private FinalCompilationTrackerState(
CreationPolicy creationPolicy,
bool generatedDocumentsUpToDate,
Compilation finalCompilationWithGeneratedDocuments,
Compilation compilationWithoutGeneratedDocuments,
bool hasSuccessfullyLoaded,
Expand All @@ -173,6 +181,8 @@ private FinalCompilationTrackerState(
{
Contract.ThrowIfNull(finalCompilationWithGeneratedDocuments);

this.GeneratedDocumentsUpToDate = generatedDocumentsUpToDate;

// As a policy, all partial-state projects are said to have incomplete references, since the
// state has no guarantees.
this.CompilationWithoutGeneratedDocuments = compilationWithoutGeneratedDocuments;
Expand Down Expand Up @@ -201,6 +211,7 @@ private FinalCompilationTrackerState(
/// <param name="metadataReferenceToProjectId">Not held onto</param>
public static FinalCompilationTrackerState Create(
CreationPolicy creationPolicy,
bool generatedDocumentsUpToDate,
Compilation finalCompilationWithGeneratedDocuments,
Compilation compilationWithoutGeneratedDocuments,
bool hasSuccessfullyLoaded,
Expand All @@ -215,6 +226,7 @@ public static FinalCompilationTrackerState Create(

return new FinalCompilationTrackerState(
creationPolicy,
generatedDocumentsUpToDate,
finalCompilationWithGeneratedDocuments,
compilationWithoutGeneratedDocuments,
hasSuccessfullyLoaded,
Expand All @@ -229,6 +241,7 @@ public FinalCompilationTrackerState WithCreationPolicy(CreationPolicy creationPo
=> creationPolicy == this.CreationPolicy
? this
: new(creationPolicy,
GeneratedDocumentsUpToDate,
FinalCompilationWithGeneratedDocuments,
CompilationWithoutGeneratedDocuments,
HasSuccessfullyLoaded,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,17 @@ await compilationState.GetCompilationAsync(
staleCompilationWithGeneratedDocuments,
cancellationToken).ConfigureAwait(false);

// Our generated documents are up to date if we just created them. Note: when in balanced mode, we
// will then change our creation policy below to DoNotCreate. This means that any successive forks
// will move us to an in-progress-state that is not running generators. And the next time we get
// here and produce a final compilation, this will then be 'false' since we'll be reusing old
// generated docs.
//
// This flag can then be used later when we hear about external user events (like save/build) to
// decide if we need to do anything. If the generated documents are up to date, then we don't need
// to do anything in that case.
var generatedDocumentsUpToDate = creationPolicy.GeneratedDocumentCreationPolicy == GeneratedDocumentCreationPolicy.Create;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: looking at the code below this line is very relevant.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

importantly, we only generate the docs when the policy is GeneratedDocumentCreationPolicy.Create, but in balanced mode we then immediately transition after this line to .DoNotCreate mode.

So the final state will say "i just generated up to date docs, but any forks off of me should reuse the docs and not regenerate them". Those forks will then end up back in the FinalCompilationTrackerState, except this time they will say "i did not just generate the SG docs" as their creation policy on this line was DoNotCreate.


// If the user has the option set to only run generators to something other than 'automatic' then we
// want to set ourselves to not run generators again now that generators have run. That way, any
// further *automatic* changes to the solution will not run generators again. Instead, when one of
Expand All @@ -605,6 +616,7 @@ await compilationState.GetCompilationAsync(

var finalState = FinalCompilationTrackerState.Create(
creationPolicy,
generatedDocumentsUpToDate,
compilationWithGeneratedDocuments,
compilationWithoutGeneratedDocuments,
hasSuccessfullyLoaded,
Expand Down Expand Up @@ -685,11 +697,21 @@ public ICompilationTracker WithCreateCreationPolicy(bool forceRegeneration)
if (state is null)
return this;

// If we're already in the state where we are running generators and skeletons (and we're not forcing
// regeneration) we don't need to do anything and can just return ourselves. The next request to create
// the compilation will do so fully.
if (state.CreationPolicy == desiredCreationPolicy && !forceRegeneration)
return this;
// If we're not forcing regeneration, we can bail out from doing work in a few cases.
if (!forceRegeneration)
{
// First If we're *already* in the state where we are running generators and skeletons we don't need
// to do anything and can just return ourselves. The next request to create the compilation will do
// so fully.
if (state.CreationPolicy == desiredCreationPolicy)
return this;

// Second, if we know we are already in a final compilation state where the generated documents were
// produced, then clearly we don't need to do anything. Nothing changed between then and now, so we
// can reuse the final compilation as is.
if (state is FinalCompilationTrackerState { GeneratedDocumentsUpToDate: true })
return this;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here's the checking point. We have been told that there was an external signal to rerun generators (a save or build) and that it was not the user explicitly clicking on the button that says "dump all generator state and rerun". Since we can see we just did that, we don't need to proceed.

}

// If we're forcing regeneration then we have to drop whatever driver we have so that we'll start from
// scratch next time around.
Expand Down
Loading