Skip to content

Commit

Permalink
Merge pull request #63196 from dibarbet/revert_rename_oop_changes
Browse files Browse the repository at this point in the history
Revert "Merge pull request #62854 from CyrusNajmabadi/renameOOP5"
  • Loading branch information
dibarbet committed Aug 4, 2022
2 parents 33c7f5a + 865502a commit 8c1eb8e
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 86 deletions.
15 changes: 1 addition & 14 deletions src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,6 @@ private set
/// </summary>
public InlineRenameFileRenameInfo FileRenameInfo { get; }

/// <summary>
/// Task used to hold a session alive with the OOP server. This allows us to pin the initial solution snapshot
/// over on the oop side, which is valuable for preventing it from constantly being dropped/synced on every
/// conflict resolution step.
/// </summary>
private readonly Task _keepAliveSessionTask;

/// <summary>
/// The task which computes the main rename locations against the original workspace
/// snapshot.
Expand All @@ -103,7 +96,7 @@ private set
/// The cancellation token for most work being done by the inline rename session. This
/// includes the <see cref="_allRenameLocationsTask"/> tasks.
/// </summary>
private readonly CancellationTokenSource _cancellationTokenSource = new();
private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource();

/// <summary>
/// This task is a continuation of the <see cref="_allRenameLocationsTask"/> that is the result of computing
Expand Down Expand Up @@ -186,9 +179,6 @@ public InlineRenameSession(
FileRenameInfo = InlineRenameFileRenameInfo.NotAllowed;
}

// Open a session to oop, syncing our solution to it and pinning it there. The connection will close once
// _cancellationTokenSource is canceled (which we always do when the session is finally ended).
_keepAliveSessionTask = Renamer.CreateRemoteKeepAliveSessionAsync(_baseSolution, _cancellationTokenSource.Token);
InitializeOpenBuffers(triggerSpan);
}

Expand Down Expand Up @@ -314,9 +304,6 @@ private void UpdateReferenceLocationsTask()

_allRenameLocationsTask = _threadingContext.JoinableTaskFactory.RunAsync(async () =>
{
// Ensure that our keep-alive session is up and running.
await _keepAliveSessionTask.ConfigureAwait(false);
// Join prior work before proceeding, since it performs a required state update.
// https://github.com/dotnet/roslyn/pull/34254#discussion_r267024593
if (currentRenameLocationsTask != null)
Expand Down
19 changes: 2 additions & 17 deletions src/Workspaces/Core/Portable/Rename/IRemoteRenamerService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,8 @@ internal interface IRemoteRenamerService
internal interface ICallback // : IRemoteOptionsCallback<CodeCleanupOptions>
{
ValueTask<CodeCleanupOptions> GetOptionsAsync(RemoteServiceCallbackId callbackId, string language, CancellationToken cancellationToken);
ValueTask KeepAliveAsync(RemoteServiceCallbackId callbackId, CancellationToken cancellationToken);
}

/// <summary>
/// Keeps alive this solution in the OOP process until the cancellation token is triggered. Used so that we can
/// call FindRenameLocationsAsync followed by many calls to ResolveConflictsAsync, knowing that things will stay
/// hydrated and alive on the OOP side.
/// </summary>
ValueTask KeepAliveAsync(
Checksum solutionChecksum,
RemoteServiceCallbackId callbackId,
CancellationToken cancellationToken);

/// <summary>
/// Runs the entire rename operation OOP and returns the final result. More efficient (due to less back and
/// forth marshaling) when the intermediary results of rename are not needed. To get the individual parts of
Expand Down Expand Up @@ -84,12 +73,6 @@ public RemoteRenamerServiceCallbackDispatcher()

public ValueTask<CodeCleanupOptions> GetOptionsAsync(RemoteServiceCallbackId callbackId, string language, CancellationToken cancellationToken)
=> ((RemoteOptionsProvider<CodeCleanupOptions>)GetCallback(callbackId)).GetOptionsAsync(language, cancellationToken);

public ValueTask KeepAliveAsync(RemoteServiceCallbackId callbackId, CancellationToken cancellationToken)
{
((TaskCompletionSource<bool>)GetCallback(callbackId)).TrySetResult(true);
return default;
}
}

[DataContract]
Expand Down Expand Up @@ -204,6 +187,8 @@ internal sealed class SerializableRenameLocations
[DataMember(Order = 0)]
public readonly SymbolRenameOptions Options;

// We use arrays so we can represent default immutable arrays.

[DataMember(Order = 1)]
public readonly ImmutableArray<SerializableRenameLocation> Locations;

Expand Down
32 changes: 0 additions & 32 deletions src/Workspaces/Core/Portable/Rename/Renamer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -219,37 +219,5 @@ private static async Task<ConflictResolution> RenameSymbolInCurrentProcessAsync(
var renameLocations = await SymbolicRenameLocations.FindLocationsInCurrentProcessAsync(symbol, solution, options, cleanupOptions, cancellationToken).ConfigureAwait(false);
return await ConflictResolver.ResolveSymbolicLocationConflictsInCurrentProcessAsync(renameLocations, newName, nonConflictSymbolKeys, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// Creates a session between the host and OOP, effectively pinning this <paramref name="solution"/> until the
/// supplied <paramref name="cancellationToken"/> is canceled.. By pinning the solution we ensure that all
/// calls to OOP for the same solution during the life of this rename session do not need to resync the
/// solution. Nor do they then need to rebuild any compilations they've already built due to the solution going
/// away and then coming back.
/// </summary>
internal static async Task CreateRemoteKeepAliveSessionAsync(
Solution solution,
CancellationToken cancellationToken)
{
var client = await RemoteHostClient.TryGetClientAsync(solution.Services, cancellationToken).ConfigureAwait(false);
if (client != null)
{
var taskCompletionSource = new TaskCompletionSource<bool>();
// make sure we transition to canceled if the caller cancels this work. This way even if OOP doesn't
// call back to complete us, we won't hang anyone awaiting on us.
cancellationToken.Register(static s => ((TaskCompletionSource<bool>)s!).TrySetResult(true), taskCompletionSource);

// Kick off the keep alive task. Once it has pinned the solution it will call back into the taskCompletionSource
// to mark it as completed so that things can proceed. It will terminate when the provided
// cancellation token is actually canceled.
var unused = client.TryInvokeAsync<IRemoteRenamerService>(
solution,
(service, solutionInfo, callbackId, cancellationToken) => service.KeepAliveAsync(solutionInfo, callbackId, cancellationToken),
callbackTarget: taskCompletionSource,
cancellationToken).AsTask();

await taskCompletionSource.Task.ConfigureAwait(false);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,18 @@
// 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;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.AddImport;
using Microsoft.CodeAnalysis.CodeCleanup;
using Microsoft.CodeAnalysis.Elfie.Diagnostics;
using Microsoft.CodeAnalysis.Formatting;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Rename;
using Microsoft.CodeAnalysis.Rename.ConflictEngine;
using Microsoft.CodeAnalysis.Simplification;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Remote
{
Expand All @@ -34,27 +38,6 @@ private CodeCleanupOptionsProvider GetClientOptionsProvider(RemoteServiceCallbac
=> new ClientCodeCleanupOptionsProvider(
(callbackId, language, cancellationToken) => _callback.InvokeAsync((callback, cancellationToken) => callback.GetOptionsAsync(callbackId, language, cancellationToken), cancellationToken), callbackId);

public ValueTask KeepAliveAsync(
Checksum solutionChecksum,
RemoteServiceCallbackId callbackId,
CancellationToken cancellationToken)
{
// First get the solution, ensuring that it is currently pinned.
return RunServiceAsync(solutionChecksum, async solution =>
{
// Once we have it, let our caller know so that it can proceed to it's next steps.
await _callback.InvokeAsync((callback, cancellationToken) =>
callback.KeepAliveAsync(callbackId, cancellationToken), cancellationToken).ConfigureAwait(false);
// Finally wait for our caller to tell us to cancel. That way we can release this solution and allow it
// to be collected if not needed anymore.
var taskCompletionSource = new TaskCompletionSource<bool>();
cancellationToken.Register(() => taskCompletionSource.TrySetCanceled(cancellationToken));
await taskCompletionSource.Task.ConfigureAwait(false);
}, cancellationToken);
}

public ValueTask<SerializableConflictResolution?> RenameSymbolAsync(
Checksum solutionChecksum,
RemoteServiceCallbackId callbackId,
Expand Down

0 comments on commit 8c1eb8e

Please sign in to comment.