Skip to content

Commit

Permalink
Merge pull request #73897 from CyrusNajmabadi/sgWork
Browse files Browse the repository at this point in the history
Avoid running generators when nothing has changed in a compilation tracker.
  • Loading branch information
CyrusNajmabadi authored Jun 10, 2024
2 parents 77edc84 + ed0bfc3 commit 64fcf76
Show file tree
Hide file tree
Showing 3 changed files with 142 additions and 5 deletions.
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;

// 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;
}

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

0 comments on commit 64fcf76

Please sign in to comment.