Skip to content

Commit adfe920

Browse files
committed
Simplify stale project handling
1 parent c5325ab commit adfe920

File tree

6 files changed

+62
-41
lines changed

6 files changed

+62
-41
lines changed

src/Features/Core/Portable/EditAndContinue/CommittedSolution.cs

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,17 @@ internal enum DocumentState
6666

6767
/// <summary>
6868
/// Tracks stale projects. Changes in these projects are ignored and their representation in the <see cref="_solution"/> does not match the binaries on disk.
69+
/// The value is the MVID of the module at the time it was determined to be stale (source code content did not match the PDB).
70+
/// A build that updates the binary to new content (that presumably matches the source code) will update the MVID. When that happens we unstale the project.
6971
///
7072
/// Build of a multi-targeted project that sets <c>SingleTargetBuildForStartupProjects</c> msbuild property (e.g. MAUI) only
7173
/// builds TFM that's active. Other TFMs of the projects remain unbuilt or stale (from previous build).
7274
///
7375
/// A project is removed from this set if it's rebuilt.
7476
///
75-
/// Lock <see cref="_guard"/> to access.
77+
/// Lock <see cref="_guard"/> to update.
7678
/// </summary>
77-
private readonly HashSet<ProjectId> _staleProjects = [];
79+
private ImmutableDictionary<ProjectId, Guid> _staleProjects = ImmutableDictionary<ProjectId, Guid>.Empty;
7880

7981
/// <summary>
8082
/// Implements workaround for https://github.com/dotnet/project-system/issues/5457.
@@ -142,13 +144,8 @@ public bool HasNoChanges(Solution solution)
142144
public Project GetRequiredProject(ProjectId id)
143145
=> _solution.GetRequiredProject(id);
144146

145-
public bool IsStaleProject(ProjectId id)
146-
{
147-
lock (_guard)
148-
{
149-
return _staleProjects.Contains(id);
150-
}
151-
}
147+
public ImmutableDictionary<ProjectId, Guid> StaleProjects
148+
=> _staleProjects;
152149

153150
public ImmutableArray<DocumentId> GetDocumentIdsWithFilePath(string path)
154151
=> _solution.GetDocumentIdsWithFilePath(path);
@@ -458,16 +455,22 @@ await TryGetMatchingSourceTextAsync(log, sourceText, sourceFilePath, currentDocu
458455
}
459456
}
460457

461-
public void CommitChanges(Solution solution, ImmutableArray<ProjectId> projectsToStale, IReadOnlyCollection<ProjectId> projectsToUnstale)
458+
public void CommitChanges(Solution solution, ImmutableDictionary<ProjectId, Guid>? staleProjects, ImmutableArray<ProjectId>? projectsToUnstale = null)
462459
{
463-
Debug.Assert(projectsToStale.Intersect(projectsToUnstale).IsEmpty());
460+
Contract.ThrowIfTrue(staleProjects is null && projectsToUnstale is null);
464461

465462
lock (_guard)
466463
{
467464
_solution = solution;
468-
_staleProjects.AddRange(projectsToStale);
469-
_staleProjects.RemoveRange(projectsToUnstale);
470-
_documentState.RemoveAll(static (documentId, _, projectsToUnstale) => projectsToUnstale.Contains(documentId.ProjectId), projectsToUnstale);
465+
466+
staleProjects ??= _staleProjects.RemoveRange(projectsToUnstale!);
467+
468+
var oldStaleProjects = _staleProjects;
469+
_staleProjects = staleProjects;
470+
471+
_documentState.RemoveAll(
472+
static (documentId, _, args) => args.oldStaleProjects.ContainsKey(documentId.ProjectId) && !args.staleProjects.ContainsKey(documentId.ProjectId),
473+
(oldStaleProjects, staleProjects));
471474
}
472475
}
473476

src/Features/Core/Portable/EditAndContinue/DebuggingSession.cs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,7 @@ public async ValueTask<EmitSolutionUpdateResults> EmitSolutionUpdateAsync(
553553
{
554554
StorePendingUpdate(new PendingSolutionUpdate(
555555
solution,
556-
solutionUpdate.ProjectsToStale,
556+
solutionUpdate.StaleProjects,
557557
solutionUpdate.ProjectsToRebuild,
558558
solutionUpdate.ProjectBaselines,
559559
solutionUpdate.ModuleUpdates.Updates,
@@ -565,7 +565,7 @@ public async ValueTask<EmitSolutionUpdateResults> EmitSolutionUpdateAsync(
565565

566566
StorePendingUpdate(new PendingSolutionUpdate(
567567
solution,
568-
solutionUpdate.ProjectsToStale,
568+
solutionUpdate.StaleProjects,
569569
// if partial updates are not allowed we don't treat rebuild as part of solution update:
570570
projectsToRebuild: [],
571571
solutionUpdate.ProjectBaselines,
@@ -585,7 +585,7 @@ public async ValueTask<EmitSolutionUpdateResults> EmitSolutionUpdateAsync(
585585

586586
// No significant changes have been made.
587587
// Commit the solution to apply any insignificant changes that do not generate updates.
588-
LastCommittedSolution.CommitChanges(solution, projectsToStale: solutionUpdate.ProjectsToStale, projectsToUnstale: []);
588+
LastCommittedSolution.CommitChanges(solution, solutionUpdate.StaleProjects);
589589
break;
590590
}
591591

@@ -611,6 +611,8 @@ public void CommitSolutionUpdate()
611611

612612
ImmutableDictionary<ManagedMethodId, ImmutableArray<NonRemappableRegion>>? newNonRemappableRegions = null;
613613
using var _ = PooledHashSet<ProjectId>.GetInstance(out var projectsToRebuildTransitive);
614+
IEnumerable<ProjectId> baselinesToDiscard = [];
615+
Solution? solution = null;
614616

615617
var pendingUpdate = RetrievePendingUpdate();
616618
if (pendingUpdate is PendingSolutionUpdate pendingSolutionUpdate)
@@ -626,7 +628,7 @@ from region in moduleRegions.Regions
626628
if (newNonRemappableRegions.IsEmpty)
627629
newNonRemappableRegions = null;
628630

629-
var solution = pendingSolutionUpdate.Solution;
631+
solution = pendingSolutionUpdate.Solution;
630632

631633
// Once the project is rebuilt all its dependencies are going to be up-to-date.
632634
var dependencyGraph = solution.GetProjectDependencyGraph();
@@ -637,7 +639,7 @@ from region in moduleRegions.Regions
637639
}
638640

639641
// Unstale all projects that will be up-to-date after rebuild.
640-
LastCommittedSolution.CommitChanges(solution, projectsToStale: pendingSolutionUpdate.ProjectsToStale, projectsToUnstale: projectsToRebuildTransitive);
642+
LastCommittedSolution.CommitChanges(solution, staleProjects: pendingSolutionUpdate.StaleProjects.RemoveRange(projectsToRebuildTransitive));
641643

642644
foreach (var projectId in projectsToRebuildTransitive)
643645
{
@@ -654,12 +656,14 @@ from region in moduleRegions.Regions
654656
{
655657
foreach (var updatedBaseline in pendingUpdate.ProjectBaselines)
656658
{
657-
_projectBaselines[updatedBaseline.ProjectId] = [.. _projectBaselines[updatedBaseline.ProjectId].Select(existingBaseline => existingBaseline.ModuleId == updatedBaseline.ModuleId ? updatedBaseline : existingBaseline)];
659+
_projectBaselines[updatedBaseline.ProjectId] = [.. _projectBaselines[updatedBaseline.ProjectId]
660+
.Select(existingBaseline => existingBaseline.ModuleId == updatedBaseline.ModuleId ? updatedBaseline : existingBaseline)];
658661
}
659662

660663
// Discard any open baseline readers for projects that need to be rebuilt,
661664
// so that the build can overwrite the underlying files.
662-
DiscardProjectBaselinesNoLock(projectsToRebuildTransitive);
665+
Contract.ThrowIfNull(solution);
666+
DiscardProjectBaselinesNoLock(solution, projectsToRebuildTransitive.Concat(baselinesToDiscard));
663667
}
664668

665669
_baselineContentAccessLock.ExitWriteLock();
@@ -676,7 +680,7 @@ public void DiscardSolutionUpdate()
676680
_ = RetrievePendingUpdate();
677681
}
678682

679-
private void DiscardProjectBaselinesNoLock(IEnumerable<ProjectId> projects)
683+
private void DiscardProjectBaselinesNoLock(Solution solution, IEnumerable<ProjectId> projects)
680684
{
681685
foreach (var projectId in projects)
682686
{
@@ -693,6 +697,8 @@ private void DiscardProjectBaselinesNoLock(IEnumerable<ProjectId> projects)
693697

694698
_initialBaselineModuleReaders.Remove(projectBaseline.ModuleId);
695699
}
700+
701+
SessionLog.Write($"Baselines discarded: {solution.GetRequiredProject(projectId).GetLogDisplay()}.");
696702
}
697703
}
698704
}
@@ -705,15 +711,15 @@ public void UpdateBaselines(Solution solution, ImmutableArray<ProjectId> rebuilt
705711
// Make sure the solution snapshot has all source-generated documents up-to-date.
706712
solution = solution.WithUpToDateSourceGeneratorDocuments(solution.ProjectIds);
707713

708-
LastCommittedSolution.CommitChanges(solution, projectsToStale: [], projectsToUnstale: rebuiltProjects);
714+
LastCommittedSolution.CommitChanges(solution, staleProjects: null, projectsToUnstale: rebuiltProjects);
709715

710716
// Wait for all operations on baseline to finish before we dispose the readers.
711717

712718
_baselineContentAccessLock.EnterWriteLock();
713719

714720
lock (_projectEmitBaselinesGuard)
715721
{
716-
DiscardProjectBaselinesNoLock(rebuiltProjects);
722+
DiscardProjectBaselinesNoLock(solution, rebuiltProjects);
717723
}
718724

719725
_baselineContentAccessLock.ExitWriteLock();

src/Features/Core/Portable/EditAndContinue/EditSession.cs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -900,8 +900,9 @@ public async ValueTask<SolutionUpdate> EmitSolutionUpdateAsync(
900900
using var _1 = ArrayBuilder<ManagedHotReloadUpdate>.GetInstance(out var deltas);
901901
using var _2 = ArrayBuilder<(Guid ModuleId, ImmutableArray<(ManagedModuleMethodId Method, NonRemappableRegion Region)>)>.GetInstance(out var nonRemappableRegions);
902902
using var _3 = ArrayBuilder<ProjectBaseline>.GetInstance(out var newProjectBaselines);
903-
using var _4 = ArrayBuilder<ProjectId>.GetInstance(out var projectsToStale);
904-
using var _5 = PooledDictionary<ProjectId, ArrayBuilder<Diagnostic>>.GetInstance(out var diagnosticBuilders);
903+
using var _4 = ArrayBuilder<(ProjectId id, Guid mvid)>.GetInstance(out var projectsToStale);
904+
using var _5 = ArrayBuilder<ProjectId>.GetInstance(out var projectsToUnstale);
905+
using var _6 = PooledDictionary<ProjectId, ArrayBuilder<Diagnostic>>.GetInstance(out var diagnosticBuilders);
905906
using var projectDifferences = new ProjectDifferences();
906907

907908
// After all projects have been analyzed "true" value indicates changed document that is only included in stale projects.
@@ -931,6 +932,7 @@ void UpdateChangedDocumentsStaleness(bool isStale)
931932
ProjectAnalysisSummary? projectSummaryToReport = null;
932933

933934
var oldSolution = DebuggingSession.LastCommittedSolution;
935+
var staleProjects = oldSolution.StaleProjects;
934936

935937
var hasPersistentErrors = false;
936938
foreach (var newProject in solution.Projects)
@@ -978,17 +980,23 @@ void UpdateChangedDocumentsStaleness(bool isStale)
978980
Log.Write($"Found {projectDifferences.ChangedOrAddedDocuments.Count} potentially changed, {projectDifferences.DeletedDocuments.Count} deleted document(s) in project {newProject.GetLogDisplay()}");
979981
}
980982

981-
var isStaleProject = oldSolution.IsStaleProject(newProject.Id);
983+
var (mvid, mvidReadError) = await DebuggingSession.GetProjectModuleIdAsync(newProject, cancellationToken).ConfigureAwait(false);
982984

983985
// We don't consider document changes in stale projects until they are rebuilt (removed from stale set).
984-
if (isStaleProject)
986+
if (staleProjects.TryGetValue(newProject.Id, out var staleModuleId))
985987
{
986-
Log.Write($"EnC state of {newProject.GetLogDisplay()} queried: project is stale");
987-
UpdateChangedDocumentsStaleness(isStale: true);
988-
continue;
988+
// The module hasn't been rebuilt or we are unable to read the MVID -- keep treating the project as stale.
989+
if (mvid == staleModuleId || mvidReadError != null)
990+
{
991+
Log.Write($"EnC state of {newProject.GetLogDisplay()} queried: project is stale");
992+
UpdateChangedDocumentsStaleness(isStale: true);
993+
994+
continue;
995+
}
996+
997+
staleProjects = staleProjects.Remove(newProject.Id);
989998
}
990999

991-
var (mvid, mvidReadError) = await DebuggingSession.GetProjectModuleIdAsync(newProject, cancellationToken).ConfigureAwait(false);
9921000
if (mvidReadError != null)
9931001
{
9941002
// The error hasn't been reported by GetDocumentDiagnosticsAsync since it might have been intermittent.
@@ -1030,7 +1038,7 @@ void UpdateChangedDocumentsStaleness(bool isStale)
10301038
// Treat the project the same as if it hasn't been built. We won't produce delta for it until it gets rebuilt.
10311039
Log.Write($"Changes not applied to {newProject.GetLogDisplay()}: binaries not up-to-date");
10321040

1033-
projectsToStale.Add(newProject.Id);
1041+
staleProjects = staleProjects.Add(newProject.Id, mvid);
10341042
UpdateChangedDocumentsStaleness(isStale: true);
10351043

10361044
continue;
@@ -1318,7 +1326,7 @@ async ValueTask LogDocumentChangesAsync(int? generation, CancellationToken cance
13181326

13191327
if (hasPersistentErrors)
13201328
{
1321-
return SolutionUpdate.Empty(diagnostics, syntaxError, ModuleUpdateStatus.Blocked);
1329+
return SolutionUpdate.Empty(diagnostics, syntaxError, staleProjects, ModuleUpdateStatus.Blocked);
13221330
}
13231331

13241332
// syntax error is a persistent error
@@ -1338,7 +1346,7 @@ async ValueTask LogDocumentChangesAsync(int? generation, CancellationToken cance
13381346

13391347
return new SolutionUpdate(
13401348
moduleUpdates,
1341-
projectsToStale.ToImmutable(),
1349+
staleProjects,
13421350
nonRemappableRegions.ToImmutable(),
13431351
newProjectBaselines.ToImmutable(),
13441352
diagnostics,

src/Features/Core/Portable/EditAndContinue/PendingSolutionUpdate.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@ internal abstract class PendingUpdate(
1818

1919
internal sealed class PendingSolutionUpdate(
2020
Solution solution,
21-
ImmutableArray<ProjectId> projectsToStale,
21+
ImmutableDictionary<ProjectId, Guid> staleProjects,
2222
ImmutableArray<ProjectId> projectsToRebuild,
2323
ImmutableArray<ProjectBaseline> projectBaselines,
2424
ImmutableArray<ManagedHotReloadUpdate> deltas,
2525
ImmutableArray<(Guid ModuleId, ImmutableArray<(ManagedModuleMethodId Method, NonRemappableRegion Region)>)> nonRemappableRegions) : PendingUpdate(projectBaselines, deltas)
2626
{
2727
public readonly Solution Solution = solution;
28-
public readonly ImmutableArray<ProjectId> ProjectsToStale = projectsToStale;
28+
public readonly ImmutableDictionary<ProjectId, Guid> StaleProjects = staleProjects;
2929
public readonly ImmutableArray<ProjectId> ProjectsToRebuild = projectsToRebuild;
3030
public readonly ImmutableArray<(Guid ModuleId, ImmutableArray<(ManagedModuleMethodId Method, NonRemappableRegion Region)> Regions)> NonRemappableRegions = nonRemappableRegions;
3131
}

src/Features/Core/Portable/EditAndContinue/SolutionUpdate.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis.EditAndContinue;
1111

1212
internal readonly struct SolutionUpdate(
1313
ModuleUpdates moduleUpdates,
14-
ImmutableArray<ProjectId> projectsToStale,
14+
ImmutableDictionary<ProjectId, Guid> staleProjects,
1515
ImmutableArray<(Guid ModuleId, ImmutableArray<(ManagedModuleMethodId Method, NonRemappableRegion Region)>)> nonRemappableRegions,
1616
ImmutableArray<ProjectBaseline> projectBaselines,
1717
ImmutableArray<ProjectDiagnostics> diagnostics,
@@ -20,7 +20,7 @@ internal readonly struct SolutionUpdate(
2020
ImmutableArray<ProjectId> projectsToRebuild)
2121
{
2222
public readonly ModuleUpdates ModuleUpdates = moduleUpdates;
23-
public readonly ImmutableArray<ProjectId> ProjectsToStale = projectsToStale;
23+
public readonly ImmutableDictionary<ProjectId, Guid> StaleProjects = staleProjects;
2424
public readonly ImmutableArray<(Guid ModuleId, ImmutableArray<(ManagedModuleMethodId Method, NonRemappableRegion Region)>)> NonRemappableRegions = nonRemappableRegions;
2525
public readonly ImmutableArray<ProjectBaseline> ProjectBaselines = projectBaselines;
2626

@@ -33,10 +33,11 @@ internal readonly struct SolutionUpdate(
3333
public static SolutionUpdate Empty(
3434
ImmutableArray<ProjectDiagnostics> diagnostics,
3535
Diagnostic? syntaxError,
36+
ImmutableDictionary<ProjectId, Guid> staleProjects,
3637
ModuleUpdateStatus status)
3738
=> new(
3839
new(status, Updates: []),
39-
projectsToStale: [],
40+
staleProjects: staleProjects,
4041
nonRemappableRegions: [],
4142
projectBaselines: [],
4243
diagnostics,

src/Features/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4117,6 +4117,10 @@ public async Task MultiTargetedPartiallyBuiltProjects()
41174117

41184118
CommitSolutionUpdate(debuggingSession);
41194119

4120+
// Stale project baselines should get discarded after committing the update.
4121+
// Unlocks binaries and allows the user to rebuild the project.
4122+
Assert.False(debuggingSession.GetTestAccessor().HasProjectEmitBaseline(projectBId));
4123+
41204124
// update source file in the editor (source text is now matching the PDB of project B):
41214125
var text0 = CreateText(source0);
41224126
solution = solution.WithDocumentText(documentA.Id, text0).WithDocumentText(documentB.Id, text0);
@@ -4144,7 +4148,6 @@ public async Task MultiTargetedPartiallyBuiltProjects()
41444148
// Saving is required so that we can fetch the baseline content for the next delta calculation.
41454149
File.WriteAllText(sourcePath, source2, Encoding.UTF8);
41464150
var mvidB2 = EmitAndLoadLibraryToDebuggee(projectBId, source2, sourceFilePath: sourcePath, assemblyName: "A", targetFramework: TargetFramework.Net90);
4147-
debuggingSession.UpdateBaselines(solution, rebuiltProjects: [projectBId]);
41484151

41494152
// no changes have been made:
41504153

0 commit comments

Comments
 (0)