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

Jump to bottom

F5 Hot Reload #52101

Merged
merged 7 commits into from
Mar 25, 2021
Merged

F5 Hot Reload #52101

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5125bdc
Move debugging service to debugging session
tmat Mar 23, 2021
6ba743a
Remove warnings for edits made while app is running
tmat Mar 23, 2021
251d382
Open edit sessions for both running and break states
tmat Mar 23, 2021
67ee3a3
Test fixes
tmat Mar 23, 2021
a739d6a
Add test
tmat Mar 24, 2021
eecc4c7
Fix tests
tmat Mar 24, 2021
53f9825
Trigger document reanalysis after EmitSolutionUpdateAsync
tmat Mar 24, 2021
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
245 changes: 121 additions & 124 deletions src/EditorFeatures/Test/EditAndContinue/EditAndContinueWorkspaceServiceTests.cs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/EditorFeatures/Test/EditAndContinue/EditSessionActiveStatementsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public Validator(
debuggingSession.Test_SetNonRemappableRegions(nonRemappableRegions ?? ImmutableDictionary<ManagedMethodId, ImmutableArray<NonRemappableRegion>>.Empty);

var telemetry = new EditSessionTelemetry();
EditSession = new EditSession(debuggingSession, telemetry);
EditSession = new EditSession(debuggingSession, telemetry, inBreakState: true);
}

public void Dispose()
Expand Down
36 changes: 17 additions & 19 deletions src/EditorFeatures/Test/EditAndContinue/RemoteEditAndContinueServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,15 @@ await proxy.StartDebuggingSessionAsync(

Assert.True(called);

// StartEditSession
// BreakStateEntered

mockEncService.StartEditSessionImpl = () => { };

await proxy.StartEditSessionAsync(CancellationToken.None).ConfigureAwait(false);
mockEncService.BreakStateEnteredImpl = (out ImmutableArray<DocumentId> documentsToReanalyze) =>
{
documentsToReanalyze = ImmutableArray.Create(document.Id);
};

VerifyReanalyzeInvocation(ImmutableArray<DocumentId>.Empty);
await proxy.BreakStateEnteredAsync(mockDiagnosticService.Object, CancellationToken.None).ConfigureAwait(false);
VerifyReanalyzeInvocation(ImmutableArray.Create(document.Id));

var activeStatement = (await remoteDebuggeeModuleMetadataProvider!.GetActiveStatementsAsync(CancellationToken.None).ConfigureAwait(false)).Single();
Assert.Equal(as1.ActiveInstruction, activeStatement.ActiveInstruction);
Expand All @@ -169,21 +171,14 @@ await proxy.StartDebuggingSessionAsync(
var availability = await remoteDebuggeeModuleMetadataProvider!.GetAvailabilityAsync(moduleId1, CancellationToken.None).ConfigureAwait(false);
Assert.Equal(new ManagedEditAndContinueAvailability(ManagedEditAndContinueAvailabilityStatus.NotAllowedForModule, "can't do enc"), availability);

// EndEditSession
// EndDebuggingSession

mockEncService.EndEditSessionImpl = (out ImmutableArray<DocumentId> documentsToReanalyze) =>
mockEncService.EndDebuggingSessionImpl = (out ImmutableArray<DocumentId> documentsToReanalyze) =>
{
documentsToReanalyze = ImmutableArray.Create(document.Id);
};

await proxy.EndEditSessionAsync(mockDiagnosticService.Object, CancellationToken.None).ConfigureAwait(false);
VerifyReanalyzeInvocation(ImmutableArray.Create(document.Id));

// EndDebuggingSession

mockEncService.EndDebuggingSessionImpl = () => { };

await proxy.EndDebuggingSessionAsync(diagnosticUpdateSource, CancellationToken.None).ConfigureAwait(false);
await proxy.EndDebuggingSessionAsync(diagnosticUpdateSource, mockDiagnosticService.Object, CancellationToken.None).ConfigureAwait(false);
VerifyReanalyzeInvocation(ImmutableArray.Create(document.Id));
Assert.Equal(1, emitDiagnosticsClearedCount);
emitDiagnosticsClearedCount = 0;
Expand Down Expand Up @@ -267,10 +262,13 @@ await proxy.StartDebuggingSessionAsync(

// CommitSolutionUpdate

called = false;
mockEncService.CommitSolutionUpdateImpl = () => called = true;
await proxy.CommitSolutionUpdateAsync(CancellationToken.None).ConfigureAwait(false);
Assert.True(called);
mockEncService.CommitSolutionUpdateImpl = (out ImmutableArray<DocumentId> documentsToReanalyze) =>
{
documentsToReanalyze = ImmutableArray.Create(document.Id);
};

await proxy.CommitSolutionUpdateAsync(mockDiagnosticService.Object, CancellationToken.None).ConfigureAwait(false);
VerifyReanalyzeInvocation(ImmutableArray.Create(document.Id));

// DiscardSolutionUpdate

Expand Down
37 changes: 18 additions & 19 deletions src/EditorFeatures/TestUtilities/EditAndContinue/MockEditAndContinueWorkspaceService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace Microsoft.CodeAnalysis.EditAndContinue.UnitTests
{
internal delegate void EndEditSession(out ImmutableArray<DocumentId> documentsToReanalyze);
internal delegate void ActionOut<T>(out T arg);

internal class MockEditAndContinueWorkspaceService : IEditAndContinueWorkspaceService
{
Expand All @@ -21,35 +21,39 @@ internal class MockEditAndContinueWorkspaceService : IEditAndContinueWorkspaceSe

public Func<Document, DocumentActiveStatementSpanProvider, ImmutableArray<(LinePositionSpan, ActiveStatementFlags)>>? GetAdjustedActiveStatementSpansImpl;
public Action<Solution, IManagedEditAndContinueDebuggerService, bool>? StartDebuggingSessionImpl;
public Action? StartEditSessionImpl;
public Action? EndDebuggingSessionImpl;
public EndEditSession? EndEditSessionImpl;

public ActionOut<ImmutableArray<DocumentId>>? EndDebuggingSessionImpl;
public Func<Solution, SolutionActiveStatementSpanProvider, string?, bool>? HasChangesImpl;
public Func<Solution, SolutionActiveStatementSpanProvider, EmitSolutionUpdateResults>? EmitSolutionUpdateImpl;
public Func<Solution, ManagedInstructionId, bool?>? IsActiveStatementInExceptionRegionImpl;
public Action<Document>? OnSourceFileUpdatedImpl;
public Action? CommitSolutionUpdateImpl;
public ActionOut<ImmutableArray<DocumentId>>? CommitSolutionUpdateImpl;
public ActionOut<ImmutableArray<DocumentId>>? BreakStateEnteredImpl;
public Action? DiscardSolutionUpdateImpl;
public Func<Document, DocumentActiveStatementSpanProvider, ImmutableArray<Diagnostic>>? GetDocumentDiagnosticsImpl;

public void CommitSolutionUpdate()
=> CommitSolutionUpdateImpl?.Invoke();
public void BreakStateEntered(out ImmutableArray<DocumentId> documentsToReanalyze)
{
documentsToReanalyze = ImmutableArray<DocumentId>.Empty;
BreakStateEnteredImpl?.Invoke(out documentsToReanalyze);
}

public void CommitSolutionUpdate(out ImmutableArray<DocumentId> documentsToReanalyze)
{
documentsToReanalyze = ImmutableArray<DocumentId>.Empty;
CommitSolutionUpdateImpl?.Invoke(out documentsToReanalyze);
}

public void DiscardSolutionUpdate()
=> DiscardSolutionUpdateImpl?.Invoke();

public ValueTask<EmitSolutionUpdateResults> EmitSolutionUpdateAsync(Solution solution, SolutionActiveStatementSpanProvider activeStatementSpanProvider, CancellationToken cancellationToken)
=> new((EmitSolutionUpdateImpl ?? throw new NotImplementedException()).Invoke(solution, activeStatementSpanProvider));

public void EndDebuggingSession()
{
EndDebuggingSessionImpl?.Invoke();
}

public void EndEditSession(out ImmutableArray<DocumentId> documentsToReanalyze)
public void EndDebuggingSession(out ImmutableArray<DocumentId> documentsToReanalyze)
{
documentsToReanalyze = ImmutableArray<DocumentId>.Empty;
EndEditSessionImpl?.Invoke(out documentsToReanalyze);
EndDebuggingSessionImpl?.Invoke(out documentsToReanalyze);
}

public ValueTask<ImmutableArray<ImmutableArray<(LinePositionSpan, ActiveStatementFlags)>>> GetBaseActiveStatementSpansAsync(Solution solution, ImmutableArray<DocumentId> documentIds, CancellationToken cancellationToken)
Expand Down Expand Up @@ -78,10 +82,5 @@ public ValueTask StartDebuggingSessionAsync(Solution solution, IManagedEditAndCo
StartDebuggingSessionImpl?.Invoke(solution, debuggerService, captureMatchingDocuments);
return default;
}

public void StartEditSession()
{
StartEditSessionImpl?.Invoke();
}
}
}
3 changes: 3 additions & 0 deletions src/Features/Core/Portable/EditAndContinue/ActiveStatementsMap.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ namespace Microsoft.CodeAnalysis.EditAndContinue
{
internal readonly struct ActiveStatementsMap
{
public static readonly ActiveStatementsMap Empty =
new(ImmutableDictionary<DocumentId, ImmutableArray<ActiveStatement>>.Empty, ImmutableDictionary<ManagedInstructionId, ActiveStatement>.Empty);

/// <summary>
/// Groups active statements by document.
/// Multiple documents point to the same set of active statements if they are linked to the same underlying source file.
Expand Down
40 changes: 29 additions & 11 deletions src/Features/Core/Portable/EditAndContinue/EditAndContinueWorkspaceService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,22 @@ public async ValueTask StartDebuggingSessionAsync(Solution solution, IManagedEdi

var previousSession = Interlocked.CompareExchange(ref _debuggingSession, new DebuggingSession(solution, debuggerService, _compilationOutputsProvider, initialDocumentStates), null);
Contract.ThrowIfFalse(previousSession == null, "New debugging session can't be started until the existing one has ended.");

StartEditSession(inBreakState: false);
}

public void StartEditSession()
private void StartEditSession(bool inBreakState)
{
var debuggingSession = _debuggingSession;
Contract.ThrowIfNull(debuggingSession, "Edit session can only be started during debugging session");

var newSession = new EditSession(debuggingSession, _editSessionTelemetry);
var newSession = new EditSession(debuggingSession, _editSessionTelemetry, inBreakState);

var previousSession = Interlocked.CompareExchange(ref _editSession, newSession, null);
Contract.ThrowIfFalse(previousSession == null, "New edit session can't be started until the existing one has ended.");
}

public void EndEditSession(out ImmutableArray<DocumentId> documentsToReanalyze)
private void EndEditSession(out ImmutableArray<DocumentId> documentsToReanalyze)
{
// first, publish null session:
var session = Interlocked.Exchange(ref _editSession, null);
Expand All @@ -119,13 +121,19 @@ public void EndEditSession(out ImmutableArray<DocumentId> documentsToReanalyze)
// clear all reported rude edits:
documentsToReanalyze = session.GetDocumentsWithReportedDiagnostics();

_debuggingSessionTelemetry.LogEditSession(_editSessionTelemetry.GetDataAndClear());
// TODO: report a separate telemetry data for hot reload sessions to preserve the semantics of the current telemetry data
Copy link
Member

Choose a reason for hiding this comment

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

i woudl love threading asserts on all these methods (i.e. AssertIsForeground/AssertIsBackground/ThisCanBeCalledOnAnyThread. It makes reading and understanding potential threading/sync/consistency issues more simply.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? There is no thread affinity. By default we don't put attributes on methods that don't have affinity.

if (session.InBreakState)
{
_debuggingSessionTelemetry.LogEditSession(_editSessionTelemetry.GetDataAndClear());
}

session.Dispose();
}

public void EndDebuggingSession()
public void EndDebuggingSession(out ImmutableArray<DocumentId> documentsToReanalyze)
{
EndEditSession(out documentsToReanalyze);

var debuggingSession = Interlocked.Exchange(ref _debuggingSession, null);
Contract.ThrowIfNull(debuggingSession, "Debugging session has not started.");

Expand All @@ -137,6 +145,13 @@ public void EndDebuggingSession()
debuggingSession.Dispose();
}

public void BreakStateEntered(out ImmutableArray<DocumentId> documentsToReanalyze)
{
// Document analyses must be recalculated to account for active statements.
EndEditSession(out documentsToReanalyze);
StartEditSession(inBreakState: true);
}

internal static bool SupportsEditAndContinue(Project project)
=> project.LanguageServices.GetService<IEditAndContinueAnalyzer>() != null;

Expand Down Expand Up @@ -281,14 +296,17 @@ public async ValueTask<EmitSolutionUpdateResults> EmitSolutionUpdateAsync(
return new EmitSolutionUpdateResults(solutionUpdate.ModuleUpdates, solutionUpdate.Diagnostics);
}

public void CommitSolutionUpdate()
public void CommitSolutionUpdate(out ImmutableArray<DocumentId> documentsToReanalyze)
{
var editSession = _editSession;
Contract.ThrowIfNull(editSession);

var pendingUpdate = editSession.RetrievePendingUpdate();
editSession.DebuggingSession.CommitSolutionUpdate(pendingUpdate);
editSession.ChangesApplied();

// restart edit session with no active statements (switching to run mode):
EndEditSession(out documentsToReanalyze);
StartEditSession(inBreakState: false);
tmat marked this conversation as resolved.
Show resolved Hide resolved
}

public void DiscardSolutionUpdate()
Expand All @@ -306,7 +324,7 @@ public void DiscardSolutionUpdate()
public async ValueTask<ImmutableArray<ImmutableArray<(LinePositionSpan, ActiveStatementFlags)>>> GetBaseActiveStatementSpansAsync(Solution solution, ImmutableArray<DocumentId> documentIds, CancellationToken cancellationToken)
{
var editSession = _editSession;
if (editSession == null)
if (editSession == null || !editSession.InBreakState)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is editSession ever null here now? Other than when EnC isn't active at all, but these calls shouldn't happen then, right? This could be a contract check perhaps?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll follow up with refactoring that cleans up edit session nullability. That can wait for Preview 3 though.

{
return default;
}
Expand Down Expand Up @@ -339,7 +357,7 @@ public void DiscardSolutionUpdate()
public async ValueTask<ImmutableArray<(LinePositionSpan, ActiveStatementFlags)>> GetAdjustedActiveStatementSpansAsync(Document document, DocumentActiveStatementSpanProvider activeStatementSpanProvider, CancellationToken cancellationToken)
{
var editSession = _editSession;
if (editSession == null)
if (editSession == null || !editSession.InBreakState)
{
return default;
}
Expand Down Expand Up @@ -373,7 +391,7 @@ public void DiscardSolutionUpdate()
// It is allowed to call this method before entering or after exiting break mode. In fact, the VS debugger does so.
// We return null since there the concept of active statement only makes sense during break mode.
var editSession = _editSession;
if (editSession == null)
if (editSession == null || !editSession.InBreakState)
{
return null;
}
Expand Down Expand Up @@ -431,7 +449,7 @@ public void DiscardSolutionUpdate()
try
{
var editSession = _editSession;
if (editSession == null)
if (editSession == null || !editSession.InBreakState)
{
return null;
}
Expand Down
24 changes: 10 additions & 14 deletions src/Features/Core/Portable/EditAndContinue/EditSession.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ internal sealed class EditSession : IDisposable
/// </summary>
internal readonly EditAndContinueDocumentAnalysesCache Analyses;

internal readonly bool InBreakState;

/// <summary>
/// A <see cref="DocumentId"/> is added whenever EnC analyzer reports
/// rude edits or module diagnostics. At the end of the session we ask the diagnostic analyzer to reanalyze
Expand All @@ -54,18 +56,23 @@ internal sealed class EditSession : IDisposable
private readonly object _documentsWithReportedDiagnosticsGuard = new();

private PendingSolutionUpdate? _pendingUpdate;
private bool _changesApplied;

internal EditSession(
DebuggingSession debuggingSession,
EditSessionTelemetry telemetry)
EditSessionTelemetry telemetry,
bool inBreakState)
{
DebuggingSession = debuggingSession;
Telemetry = telemetry;

_nonRemappableRegions = debuggingSession.NonRemappableRegions;

BaseActiveStatements = new AsyncLazy<ActiveStatementsMap>(cancellationToken => GetBaseActiveStatementsAsync(cancellationToken), cacheResult: true);
InBreakState = inBreakState;

BaseActiveStatements = inBreakState ?
new AsyncLazy<ActiveStatementsMap>(cancellationToken => GetBaseActiveStatementsAsync(cancellationToken), cacheResult: true) :
new AsyncLazy<ActiveStatementsMap>(ActiveStatementsMap.Empty);
tmat marked this conversation as resolved.
Show resolved Hide resolved

Analyses = new EditAndContinueDocumentAnalysesCache(BaseActiveStatements);
}

Expand Down Expand Up @@ -476,11 +483,6 @@ public async ValueTask<bool> HasChangesAsync(Solution solution, SolutionActiveSt
{
try
{
if (_changesApplied)
{
return false;
}

var baseSolution = DebuggingSession.LastCommittedSolution;
if (baseSolution.HasNoChanges(solution))
{
Expand Down Expand Up @@ -1128,11 +1130,5 @@ internal PendingSolutionUpdate RetrievePendingUpdate()
Contract.ThrowIfNull(pendingUpdate);
return pendingUpdate;
}

internal void ChangesApplied()
{
Debug.Assert(!_changesApplied);
_changesApplied = true;
}
}
}
9 changes: 3 additions & 6 deletions src/Features/Core/Portable/EditAndContinue/IEditAndContinueWorkspaceService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@
// 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.Generic;
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.Debugger.Contracts.EditAndContinue;
Expand All @@ -19,15 +17,14 @@ internal interface IEditAndContinueWorkspaceService : IWorkspaceService, IActive
ValueTask<bool> HasChangesAsync(Solution solution, SolutionActiveStatementSpanProvider activeStatementSpanProvider, string? sourceFilePath, CancellationToken cancellationToken);
ValueTask<EmitSolutionUpdateResults> EmitSolutionUpdateAsync(Solution solution, SolutionActiveStatementSpanProvider activeStatementSpanProvider, CancellationToken cancellationToken);

void CommitSolutionUpdate();
void CommitSolutionUpdate(out ImmutableArray<DocumentId> documentsToReanalyze);
tmat marked this conversation as resolved.
Show resolved Hide resolved
void DiscardSolutionUpdate();

void OnSourceFileUpdated(Document document);

ValueTask StartDebuggingSessionAsync(Solution solution, IManagedEditAndContinueDebuggerService debuggerService, bool captureMatchingDocuments, CancellationToken cancellationToken);
void StartEditSession();
void EndEditSession(out ImmutableArray<DocumentId> documentsToReanalyze);
void EndDebuggingSession();
void BreakStateEntered(out ImmutableArray<DocumentId> documentsToReanalyze);
void EndDebuggingSession(out ImmutableArray<DocumentId> documentsToReanalyze);

ValueTask<bool?> IsActiveStatementInExceptionRegionAsync(Solution solution, ManagedInstructionId instructionId, CancellationToken cancellationToken);
ValueTask<LinePositionSpan?> GetCurrentActiveStatementPositionAsync(Solution solution, SolutionActiveStatementSpanProvider activeStatementSpanProvider, ManagedInstructionId instructionId, CancellationToken cancellationToken);
Expand Down
Loading