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.

Sign up for GitHub

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

Create stronger type to expression compilation tracker internal policy #72673

Merged
merged 8 commits into from
Mar 23, 2024
Merged

Create stronger type to expression compilation tracker internal policy #72673

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ad07803
Create stronger type to expression compilation tracker internal policy
CyrusNajmabadi Mar 22, 2024
aed1da1
Fix comment
CyrusNajmabadi Mar 22, 2024
7bf8603
Docs
CyrusNajmabadi Mar 22, 2024
450625c
Docs
CyrusNajmabadi Mar 22, 2024
1b24cc9
Skip failing test
333fred Mar 22, 2024
f45d57d
Merge remote-tracking branch 'upstream/main' into creationPolicy
CyrusNajmabadi Mar 22, 2024
fc6c79f
Merge remote-tracking branch '333fred/skip' into creationPolicy
CyrusNajmabadi Mar 22, 2024
f99e64d
Merge branch 'main' into creationPolicy
CyrusNajmabadi Mar 22, 2024
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
43 changes: 24 additions & 19 deletions ...Workspace/Solution/SolutionCompilationState.CompilationTracker.CompilationTrackerState.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,11 @@ private abstract class CompilationTrackerState
/// whether we even have references -- it's pretty likely that running a generator might produce worse results
/// than what we originally had.</item>
/// </list>
/// This also controls if we will generate skeleton references for cross-language P2P references when
/// creating the compilation for a particular project. When entirely frozen, we do not want to do this due
/// to the enormous cost of emitting ref assemblies for cross language cases.
/// </summary>
public readonly bool IsFrozen;
public readonly CreationPolicy CreationPolicy;

/// <summary>
/// The best compilation that is available that source generators have not ran on. May be an
Expand All @@ -47,10 +50,10 @@ private abstract class CompilationTrackerState
public CompilationTrackerGeneratorInfo GeneratorInfo { get; }

protected CompilationTrackerState(
bool isFrozen,
CreationPolicy creationPolicy,
CompilationTrackerGeneratorInfo generatorInfo)
{
IsFrozen = isFrozen;
CreationPolicy = creationPolicy;
GeneratorInfo = generatorInfo;
}
}
Expand Down Expand Up @@ -80,12 +83,12 @@ private sealed class InProgressState : CompilationTrackerState
public override Compilation CompilationWithoutGeneratedDocuments => LazyCompilationWithoutGeneratedDocuments.Value;

public InProgressState(
bool isFrozen,
CreationPolicy creationPolicy,
Lazy<Compilation> compilationWithoutGeneratedDocuments,
CompilationTrackerGeneratorInfo generatorInfo,
Lazy<Compilation?> staleCompilationWithGeneratedDocuments,
ImmutableList<TranslationAction> pendingTranslationActions)
: base(isFrozen, generatorInfo)
: base(creationPolicy, generatorInfo)
{
// Note: Intermediate projects can be empty.
Contract.ThrowIfTrue(pendingTranslationActions is null);
Expand All @@ -105,13 +108,13 @@ public InProgressState(
}

public InProgressState(
bool isFrozen,
CreationPolicy creationPolicy,
Compilation compilationWithoutGeneratedDocuments,
CompilationTrackerGeneratorInfo generatorInfo,
Compilation? staleCompilationWithGeneratedDocuments,
ImmutableList<TranslationAction> pendingTranslationActions)
: this(
isFrozen,
creationPolicy,
new Lazy<Compilation>(() => compilationWithoutGeneratedDocuments),
generatorInfo,
// Extracted as a method call to prevent captures.
Expand Down Expand Up @@ -164,20 +167,20 @@ private sealed class FinalCompilationTrackerState : CompilationTrackerState
public override Compilation CompilationWithoutGeneratedDocuments { get; }

private FinalCompilationTrackerState(
bool isFrozen,
CreationPolicy creationPolicy,
Compilation finalCompilationWithGeneratedDocuments,
Compilation compilationWithoutGeneratedDocuments,
bool hasSuccessfullyLoaded,
CompilationTrackerGeneratorInfo generatorInfo,
UnrootedSymbolSet unrootedSymbolSet)
: base(isFrozen, generatorInfo)
: base(creationPolicy, generatorInfo)
{
Contract.ThrowIfNull(finalCompilationWithGeneratedDocuments);

// As a policy, all partial-state projects are said to have incomplete references, since the
// state has no guarantees.
this.CompilationWithoutGeneratedDocuments = compilationWithoutGeneratedDocuments;
HasSuccessfullyLoaded = hasSuccessfullyLoaded && !isFrozen;
Copy link
Contributor

Choose a reason for hiding this comment

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

&& !isFrozen;

Just wanted to make sure this was intentional

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it was. deciding to freeze things doesn't have any impact on our determination if things successfully loaded or not.

HasSuccessfullyLoaded = hasSuccessfullyLoaded;
FinalCompilationWithGeneratedDocuments = finalCompilationWithGeneratedDocuments;
UnrootedSymbolSet = unrootedSymbolSet;

Expand All @@ -202,7 +205,7 @@ private FinalCompilationTrackerState(
/// <param name="projectId">Not held onto</param>
/// <param name="metadataReferenceToProjectId">Not held onto</param>
public static FinalCompilationTrackerState Create(
bool isFrozen,
CreationPolicy creationPolicy,
Compilation finalCompilationWithGeneratedDocuments,
Compilation compilationWithoutGeneratedDocuments,
bool hasSuccessfullyLoaded,
Expand All @@ -217,21 +220,23 @@ public static FinalCompilationTrackerState Create(
RecordAssemblySymbols(projectId, finalCompilationWithGeneratedDocuments, metadataReferenceToProjectId);

return new FinalCompilationTrackerState(
isFrozen,
creationPolicy,
finalCompilationWithGeneratedDocuments,
compilationWithoutGeneratedDocuments,
hasSuccessfullyLoaded,
generatorInfo,
unrootedSymbolSet);
}

public FinalCompilationTrackerState WithIsFrozen()
=> new(isFrozen: true,
FinalCompilationWithGeneratedDocuments,
CompilationWithoutGeneratedDocuments,
HasSuccessfullyLoaded,
GeneratorInfo,
UnrootedSymbolSet);
public FinalCompilationTrackerState WithCreationPolicy(CreationPolicy creationPolicy)
=> creationPolicy == this.CreationPolicy
? this
: new(creationPolicy,
FinalCompilationWithGeneratedDocuments,
CompilationWithoutGeneratedDocuments,
HasSuccessfullyLoaded,
GeneratorInfo,
UnrootedSymbolSet);

private static void RecordAssemblySymbols(ProjectId projectId, Compilation compilation, Dictionary<MetadataReference, ProjectId>? metadataReferenceToProjectId)
{
Expand Down
68 changes: 40 additions & 28 deletions ...orkspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.CompilationTracker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public ICompilationTracker Fork(
};

var newState = new InProgressState(
state.IsFrozen,
state.CreationPolicy,
compilationWithoutGeneratedDocuments,
state.GeneratorInfo,
staleCompilationWithGeneratedDocuments,
Expand Down Expand Up @@ -334,12 +334,13 @@ InProgressState BuildInProgressStateFromNoCompilationState()

var compilationWithoutGeneratedDocuments = CreateEmptyCompilation();

// We only got here when we had no compilation state at all. So we couldn't have gotten
// here from a frozen state (as a frozen state always ensures we have a
// WithCompilationTrackerState). As such, we can safely still preserve that we're not
// frozen here.
// We only got here when we had no compilation state at all. So we couldn't have gotten here
// from a frozen state (as a frozen state always ensures we have at least an InProgressState).
// As such, we want to start initially in the state where we will both run generators and create
// skeleton references for p2p references. That will ensure the most correct state for our
// compilation the first time we create it.
var allSyntaxTreesParsedState = new InProgressState(
isFrozen: false,
CreationPolicy.Create,
new Lazy<Compilation>(CreateEmptyCompilation),
CompilationTrackerGeneratorInfo.Empty,
staleCompilationWithGeneratedDocuments: s_lazyNullCompilation,
Expand Down Expand Up @@ -380,7 +381,7 @@ async Task<InProgressState> CollapseInProgressStateAsync(InProgressState initial
// generator docs back to the uncomputed state from that point onwards. We'll just keep
// whateverZ generated docs we have.
currentState = new InProgressState(
currentState.IsFrozen,
currentState.CreationPolicy,
compilationWithoutGeneratedDocuments,
generatorInfo,
staleCompilationWithGeneratedDocuments,
Expand Down Expand Up @@ -472,7 +473,7 @@ async Task<FinalCompilationTrackerState> FinalizeCompilationWorkerAsync(InProgre
Contract.ThrowIfTrue(inProgressState.PendingTranslationActions.Count > 0);

// The final state we produce will be frozen or not depending on if a frozen state was passed into it.
var isFrozen = inProgressState.IsFrozen;
var creationPolicy = inProgressState.CreationPolicy;
var generatorInfo = inProgressState.GeneratorInfo;
var compilationWithoutGeneratedDocuments = inProgressState.CompilationWithoutGeneratedDocuments;
var staleCompilationWithGeneratedDocuments = inProgressState.LazyStaleCompilationWithGeneratedDocuments.Value;
Expand Down Expand Up @@ -524,27 +525,33 @@ await compilationState.GetCompilationAsync(
{
// Not a submission. Add as a metadata reference.

if (isFrozen)
if (creationPolicy.SkeletonReferenceCreationPolicy is SkeletonReferenceCreationPolicy.Create)
{
// In the frozen case, attempt to get a partial reference, or fallback to the last
// successful reference for this project if we can find one.
var metadataReference = compilationState.GetPartialMetadataReference(projectReference, this.ProjectState);
var metadataReference = await compilationState.GetMetadataReferenceAsync(projectReference, this.ProjectState, cancellationToken).ConfigureAwait(false);
AddMetadataReference(projectReference, metadataReference);
}
else
{
Contract.ThrowIfFalse(creationPolicy.SkeletonReferenceCreationPolicy is SkeletonReferenceCreationPolicy.CreateIfAbsent or SkeletonReferenceCreationPolicy.DoNotCreate);

// If not asked to explicit create an up to date skeleton, attempt to get a partial
// reference, or fallback to the last successful reference for this project if we can
// find one.
var metadataReference = compilationState.GetPartialMetadataReference(projectReference, this.ProjectState);
if (metadataReference is null)
{
// if we failed to get the metadata and we were frozen, check to see if we
// previously had existing metadata and reuse it instead.
var inProgressCompilationNotRef = staleCompilationWithGeneratedDocuments ?? compilationWithoutGeneratedDocuments;
metadataReference = inProgressCompilationNotRef.ExternalReferences.FirstOrDefault(
r => GetProjectId(inProgressCompilationNotRef.GetAssemblyOrModuleSymbol(r) as IAssemblySymbol) == projectReference.ProjectId);
}

AddMetadataReference(projectReference, metadataReference);
}
else
{
// For the non-frozen case, attempt to get the full metadata reference.
var metadataReference = await compilationState.GetMetadataReferenceAsync(projectReference, this.ProjectState, cancellationToken).ConfigureAwait(false);
// If we still failed, but our policy is to create when absent, then do the work to
// create a real skeleton here.
if (metadataReference is null && creationPolicy.SkeletonReferenceCreationPolicy is SkeletonReferenceCreationPolicy.CreateIfAbsent)
{
metadataReference = await compilationState.GetMetadataReferenceAsync(projectReference, this.ProjectState, cancellationToken).ConfigureAwait(false);
}

AddMetadataReference(projectReference, metadataReference);
}
}
Expand All @@ -563,7 +570,7 @@ await compilationState.GetCompilationAsync(

// We will finalize the compilation by adding full contents here.
var (compilationWithGeneratedDocuments, generatedDocuments, generatorDriver) = await AddExistingOrComputeNewGeneratorInfoAsync(
isFrozen,
creationPolicy,
compilationState,
compilationWithoutGeneratedDocuments,
generatorInfo,
Expand All @@ -574,7 +581,7 @@ await compilationState.GetCompilationAsync(
var nextGeneratorInfo = new CompilationTrackerGeneratorInfo(generatedDocuments, generatorDriver);

var finalState = FinalCompilationTrackerState.Create(
isFrozen,
creationPolicy,
compilationWithGeneratedDocuments,
compilationWithoutGeneratedDocuments,
hasSuccessfullyLoaded,
Expand Down Expand Up @@ -667,13 +674,18 @@ public ICompilationTracker FreezePartialState(CancellationToken cancellationToke
{
var state = this.ReadState();

// We're freezing the solution for features where latency performance is parameter. Do not run SGs or
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
// create skeleton references at this point. Just use whatever we've already generated for each in the
// past.
var desiredCreationPolicy = CreationPolicy.DoNotCreate;

if (state is FinalCompilationTrackerState finalState)
{
// If we're finalized and already frozen, we can just use ourselves. Otherwise, flip the frozen bit
// so that any future forks keep things frozen.
return finalState.IsFrozen
// Attempt to transition our state to one where we do not run generators or create skeleton references.
var newFinalState = finalState.WithCreationPolicy(desiredCreationPolicy);
return newFinalState == finalState
? this
: new CompilationTracker(this.ProjectState, finalState.WithIsFrozen(), skeletonReferenceCacheToClone: _skeletonReferenceCache);
: new CompilationTracker(this.ProjectState, newFinalState, skeletonReferenceCacheToClone: _skeletonReferenceCache);
}

// Non-final state currently. Produce an in-progress-state containing the forked change. Note: we
Expand Down Expand Up @@ -722,7 +734,7 @@ public ICompilationTracker FreezePartialState(CancellationToken cancellationToke
return new CompilationTracker(
frozenProjectState,
new InProgressState(
isFrozen: true,
desiredCreationPolicy,
lazyCompilationWithoutGeneratedDocuments,
CompilationTrackerGeneratorInfo.Empty,
lazyCompilationWithGeneratedDocuments,
Expand All @@ -748,7 +760,7 @@ public ICompilationTracker FreezePartialState(CancellationToken cancellationToke
return new CompilationTracker(
frozenProjectState,
new InProgressState(
isFrozen: true,
desiredCreationPolicy,
compilationWithoutGeneratedDocuments,
generatorInfo,
compilationWithGeneratedDocuments,
Expand Down
4 changes: 2 additions & 2 deletions ...ore/Portable/Workspace/Solution/SolutionCompilationState.CompilationTracker_Generators.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@ internal partial class SolutionCompilationState
private partial class CompilationTracker : ICompilationTracker
{
private async Task<(Compilation compilationWithGeneratedFiles, TextDocumentStates<SourceGeneratedDocumentState> generatedDocuments, GeneratorDriver? generatorDriver)> AddExistingOrComputeNewGeneratorInfoAsync(
bool isFrozen,
CreationPolicy creationPolicy,
SolutionCompilationState compilationState,
Compilation compilationWithoutGeneratedFiles,
CompilationTrackerGeneratorInfo generatorInfo,
Compilation? compilationWithStaleGeneratedTrees,
CancellationToken cancellationToken)
{
if (isFrozen)
if (creationPolicy.GeneratedDocumentCreationPolicy is GeneratedDocumentCreationPolicy.DoNotCreate)
{
// We're frozen. So we do not want to go through the expensive cost of running generators. Instead, we
// just whatever prior generated docs we have.
Expand Down
Loading
Loading