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

Pass runningProject to GetUpdateAsync and get projectToRebuild and projectToRestart back #75503

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 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
6 changes: 3 additions & 3 deletions eng/Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
<!--
Subset of Microsoft.VisualStudio.Sdk meta-package (run `csi generate-vssdk-versions.csx` to update based on VSSDK meta-package).
-->
<PackageVersion Include="Microsoft.ServiceHub.Framework" Version="4.5.31" />
<PackageVersion Include="Microsoft.ServiceHub.Framework" Version="4.5.35" />
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be updated without updating Microsoft.VisualStudio.SDK. Probably better to bump Microsoft.VisualStudio.SDK in a separate PR.

Copy link
Author

@LittleLittleCloud LittleLittleCloud Oct 18, 2024

Choose a reason for hiding this comment

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

How to determine the right VS SDK version to use. I saw the current version is 17.10.234-preview.1 but that one is not available on nuget.org?

<PackageVersion Include="Microsoft.VisualStudio.Composition" Version="17.10.37" />
<PackageVersion Include="Microsoft.VisualStudio.Composition.Analyzers" Version="17.10.37" />
<PackageVersion Include="Microsoft.VisualStudio.CoreUtility" Version="17.10.191" />
Expand All @@ -104,7 +104,7 @@
<PackageVersion Include="Microsoft.VisualStudio.Language.StandardClassification" Version="17.10.191" />
<PackageVersion Include="Microsoft.VisualStudio.LanguageServer.Client" Version="17.10.124" />
<PackageVersion Include="Microsoft.VisualStudio.RemoteControl" Version="16.3.52" />
<PackageVersion Include="Microsoft.VisualStudio.RpcContracts" Version="17.10.21" />
<PackageVersion Include="Microsoft.VisualStudio.RpcContracts" Version="17.11.8" />
<PackageVersion Include="Microsoft.VisualStudio.Shell.15.0" Version="17.10.40170" />
<PackageVersion Include="Microsoft.VisualStudio.Telemetry" Version="17.11.8" />
<PackageVersion Include="Microsoft.VisualStudio.Text.Data" Version="17.10.191" />
Expand All @@ -121,7 +121,7 @@
<!--
VS Debugger
-->
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Contracts" Version="17.12.0-beta.24403.1" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Contracts" Version="17.13.0-beta.24518.1" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Engine-implementation" Version="17.13.1100701-preview" />
<PackageVersion Include="Microsoft.VisualStudio.Debugger.Metadata-implementation" Version="17.13.1100701-preview" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Composition;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.BrokeredServices;
Expand All @@ -20,7 +22,7 @@
namespace Microsoft.CodeAnalysis.EditAndContinue;

[Shared]
[Export(typeof(IManagedHotReloadLanguageService))]
[Export(typeof(IManagedHotReloadLanguageService2))]
LittleLittleCloud marked this conversation as resolved.
Show resolved Hide resolved
[Export(typeof(IEditAndContinueSolutionProvider))]
[Export(typeof(EditAndContinueLanguageService))]
[ExportMetadata("UIContext", EditAndContinueUIContext.EncCapableProjectExistsInWorkspaceUIContextString)]
Expand Down Expand Up @@ -309,7 +311,7 @@ public async ValueTask<bool> HasChangesAsync(string? sourceFilePath, Cancellatio
}
}

public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(CancellationToken cancellationToken)
public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(ImmutableArray<string> runningProjects, CancellationToken cancellationToken)
{
if (_disabled)
{
Expand All @@ -321,6 +323,10 @@ public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(CancellationToke
var activeStatementSpanProvider = GetActiveStatementSpanProvider(solution);
var result = await GetDebuggingSession().EmitSolutionUpdateAsync(solution, activeStatementSpanProvider, cancellationToken).ConfigureAwait(false);

var projectsToRestart = new HashSet<Project>();
LittleLittleCloud marked this conversation as resolved.
Show resolved Hide resolved
var projectsToRebuild = new HashSet<Project>();
result.GetProjectsToRebuildAndRestart(solution, (project) => runningProjects.Contains(project.FilePath), projectsToRestart, projectsToRebuild);

// Only store the solution if we have any changes to apply, otherwise CommitUpdatesAsync/DiscardUpdatesAsync won't be called.
if (result.ModuleUpdates.Status == ModuleUpdateStatus.Ready)
{
Expand All @@ -329,6 +335,12 @@ public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(CancellationToke

UpdateApplyChangesDiagnostics(result.Diagnostics);

return new ManagedHotReloadUpdates(result.ModuleUpdates.Updates.FromContract(), result.GetAllDiagnostics().FromContract());
var projectsToRestartString = projectsToRestart.Select(p => p.FilePath!).AsImmutable();
var projectsToReBuildArray = projectsToRebuild.Select(p => p.FilePath!).AsImmutable();

return new ManagedHotReloadUpdates(result.ModuleUpdates.Updates.FromContract(), result.GetAllDiagnostics().FromContract(), projectsToReBuildArray, projectsToRestartString);
}

public ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(CancellationToken cancellationToken)
=> GetUpdatesAsync([], cancellationToken);
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.VisualStudio.Debugger.Contracts.HotReload;
Expand All @@ -13,7 +14,7 @@ namespace Microsoft.CodeAnalysis.EditAndContinue;
/// TODO (https://github.com/dotnet/roslyn/issues/72713):
/// Once debugger is updated to use the brokered service, this class should be removed and <see cref="EditAndContinueLanguageService"/> should be exported directly.
/// </summary>
internal sealed partial class ManagedEditAndContinueLanguageServiceBridge(EditAndContinueLanguageService service) : IManagedHotReloadLanguageService
internal sealed partial class ManagedEditAndContinueLanguageServiceBridge(EditAndContinueLanguageService service) : IManagedHotReloadLanguageService2
{
public ValueTask StartSessionAsync(CancellationToken cancellationToken)
=> service.StartSessionAsync(cancellationToken);
Expand All @@ -33,6 +34,9 @@ public ValueTask OnCapabilitiesChangedAsync(CancellationToken cancellationToken)
public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(CancellationToken cancellationToken)
=> (await service.GetUpdatesAsync(cancellationToken).ConfigureAwait(false));

public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(ImmutableArray<string> projects, CancellationToken cancellationToken)
=> (await service.GetUpdatesAsync(projects, cancellationToken).ConfigureAwait(false));

public ValueTask CommitUpdatesAsync(CancellationToken cancellationToken)
=> service.CommitUpdatesAsync(cancellationToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ await localWorkspace.ChangeSolutionAsync(localWorkspace.CurrentSolution
};
};

var updates = await localService.GetUpdatesAsync(CancellationToken.None);
var updates = await localService.GetUpdatesAsync([], CancellationToken.None);

Assert.Equal(++observedDiagnosticVersion, diagnosticRefresher.GlobalStateVersion);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,127 @@

return builder.ToImmutableAndClear();
}

Copy link
Member

Choose a reason for hiding this comment

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

Not sure why this is shown as added. Did you rebase against the latest main?

Copy link
Author

Choose a reason for hiding this comment

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

Not quite follow, what you mean here

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Yeah that's true, would be better to find out a way to clean it up

Copy link
Member

Choose a reason for hiding this comment

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

Let me look into that

private IEnumerable<Project> GetProjectsContainingBlockingRudeEdits(Solution solution)
=> RudeEdits
.Where(static e => e.Severity == DiagnosticSeverity.Error && e.ProjectId is not null)
.Select(static e => e.ProjectId)
.Distinct()
.OrderBy(static id => id)
.Select(solution.GetRequiredProject);

Check failure on line 84 in src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs#L84

src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs(84,17): error CS8622: (NETCORE_ENGINEERING_TELEMETRY=Build) Nullability of reference types in type of parameter 'projectId' of 'Project ISolutionExtensions.GetRequiredProject(Solution solution, ProjectId projectId)' doesn't match the target delegate 'Func<ProjectId?, Project>' (possibly because of nullability attributes).

Check failure on line 84 in src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs

View check run for this annotation

Azure Pipelines / roslyn-CI (Correctness Correctness_Analyzers)

src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs#L84

src/Features/Core/Portable/EditAndContinue/EmitSolutionUpdateResults.cs(84,17): error CS8622: (NETCORE_ENGINEERING_TELEMETRY=Build) Nullability of reference types in type of parameter 'projectId' of 'Project ISolutionExtensions.GetRequiredProject(Solution solution, ProjectId projectId)' doesn't match the target delegate 'Func<ProjectId?, Project>' (possibly because of nullability attributes).

/// <summary>
/// Returns projects that need to be rebuilt and/or restarted due to blocking rude edits in order to apply changes.
/// </summary>
/// <param name="isRunningProject">Identifies projects that have been launched.</param>
/// <param name="projectsToRestart">Running projects that have to be restarted.</param>
/// <param name="projectsToRebuild">Projects whose source have been updated and need to be rebuilt.</param>
public void GetProjectsToRebuildAndRestart(
Solution solution,
Func<Project, bool> isRunningProject,
ISet<Project> projectsToRestart,
ISet<Project> projectsToRebuild)
{
var graph = solution.GetProjectDependencyGraph();

// First, find all running projects that transitively depend on projects with rude edits.
// These will need to be rebuilt and restarted. In order to rebuilt these projects
// all their transitive references must either be free of source changes or be rebuilt as well.
// This may add more running projects to the set of projects we need to restart.
// We need to repeat this process until we find a fixed point.

using var _1 = ArrayBuilder<Project>.GetInstance(out var traversalStack);

projectsToRestart.Clear();
projectsToRebuild.Clear();

foreach (var projectWithRudeEdit in GetProjectsContainingBlockingRudeEdits(solution))
{
if (AddImpactedRunningProjects(projectsToRestart, projectWithRudeEdit))
{
projectsToRebuild.Add(projectWithRudeEdit);
}
}

// At this point the restart set contains all running projects directly affected by rude edits.
// Next, find projects that were successfully updated and affect running projects.

if (ModuleUpdates.Updates.IsEmpty || projectsToRestart.IsEmpty())
{
return;
}

// The set of updated projects is usually much smaller then the number of all projects in the solution.
// We iterate over this set updating the reset set until no new project is added to the reset set.
// Once a project is determined to affect a running process, all running processes that
// reference this project are added to the reset set. The project is then removed from updated
// project set as it can't contribute any more running projects to the reset set.
// If an updated project does not affect reset set in a given iteration, it stays in the set
// because it may affect reset set later on, after another running project is added to it.

using var _2 = PooledHashSet<Project>.GetInstance(out var updatedProjects);
using var _3 = ArrayBuilder<Project>.GetInstance(out var updatedProjectsToRemove);
foreach (var update in ModuleUpdates.Updates)
{
updatedProjects.Add(solution.GetRequiredProject(update.ProjectId));
}

using var _4 = ArrayBuilder<Project>.GetInstance(out var impactedProjects);

while (true)
{
Debug.Assert(updatedProjectsToRemove.Count == 0);

foreach (var updatedProject in updatedProjects)
{
if (AddImpactedRunningProjects(impactedProjects, updatedProject) &&
impactedProjects.Any(projectsToRestart.Contains))
{
projectsToRestart.AddRange(impactedProjects);
updatedProjectsToRemove.Add(updatedProject);
projectsToRebuild.Add(updatedProject);
}

impactedProjects.Clear();
}

if (updatedProjectsToRemove is [])
{
// none of the remaining updated projects affect restart set:
break;
}

updatedProjects.RemoveAll(updatedProjectsToRemove);
updatedProjectsToRemove.Clear();
}

return;

bool AddImpactedRunningProjects(ICollection<Project> impactedProjects, Project initialProject)
{
Debug.Assert(traversalStack.Count == 0);
traversalStack.Push(initialProject);

var added = false;

while (traversalStack.Count > 0)
{
var project = traversalStack.Pop();
if (isRunningProject(project))
{
impactedProjects.Add(project);
added = true;
}

foreach (var referencingProjectId in graph.GetProjectsThatDirectlyDependOnThisProject(project.Id))
{
traversalStack.Push(solution.GetRequiredProject(referencingProjectId));
}
}

return added;
}
}
}

public static readonly EmitSolutionUpdateResults Empty = new()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Immutable;
using System.ComponentModel.Composition;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -43,6 +44,9 @@ public ValueTask OnCapabilitiesChangedAsync(CancellationToken cancellationToken)
public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(CancellationToken cancellationToken)
=> (await service.GetUpdatesAsync(cancellationToken).ConfigureAwait(false)).FromContract();

public async ValueTask<ManagedHotReloadUpdates> GetUpdatesAsync(ImmutableArray<string> projects, CancellationToken cancellationToken)
=> (await service.GetUpdatesAsync(cancellationToken).ConfigureAwait(false)).FromContract();

public ValueTask CommitUpdatesAsync(CancellationToken cancellationToken)
=> service.CommitUpdatesAsync(cancellationToken);

Expand Down
Loading