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

Keep the solution pinned (on host and OOP) when making multiple calls between systems. #70765

Merged
merged 10 commits into from
Nov 11, 2023
2 changes: 1 addition & 1 deletion global.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"sdk": {
"version": "8.0.100-rc.1.23463.5",
"allowPrerelease": false,
"rollForward": "latestPatch"
"rollForward": "disable"
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
},
"tools": {
"dotnet": "8.0.100-rc.1.23463.5",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ public async Task SearchGeneratedDocumentsAsync(
var callback = new NavigateToSearchServiceCallback(onItemFound);

await client.TryInvokeAsync<IRemoteNavigateToSearchService>(
// Sync and search the full solution snapshot. While this function is called serially per project,
// we want to operate on the same solution snapshot on the OOP side per project so that we can
// benefit from things like cached compilations. If we produced different snapshots, those
// compilations would not be shared and we'd have to rebuild them.
solution,
(service, solutionInfo, callbackId, cancellationToken) =>
service.SearchGeneratedDocumentsAsync(solutionInfo, project.Id, searchPattern, kinds.ToImmutableArray(), callbackId, cancellationToken),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public async Task SearchDocumentAsync(
if (client != null)
{
var callback = new NavigateToSearchServiceCallback(onItemFound);
// Don't need to sync the full solution when searching a particular project.
// Don't need to sync the full solution when searching a single document. Just sync the project that doc is in.
await client.TryInvokeAsync<IRemoteNavigateToSearchService>(
document.Project,
(service, solutionInfo, callbackId, cancellationToken) =>
Expand Down Expand Up @@ -72,6 +72,9 @@ public async Task SearchProjectAsync(
var callback = new NavigateToSearchServiceCallback(onItemFound);

await client.TryInvokeAsync<IRemoteNavigateToSearchService>(
// Intentionally sync the full solution. When SearchProjectAsync is called, we're searching all
// projects (just in parallel). So best for them all to sync and share a single solution snapshot
// on the oop side.
solution,
(service, solutionInfo, callbackId, cancellationToken) =>
service.SearchProjectAsync(solutionInfo, project.Id, priorityDocumentIds, searchPattern, kinds.ToImmutableArray(), callbackId, cancellationToken),
Expand Down
33 changes: 14 additions & 19 deletions src/Features/Core/Portable/NavigateTo/NavigateToSearcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Internal.Log;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Remote;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.TestHooks;
Expand All @@ -35,6 +36,7 @@ internal class NavigateToSearcher
private readonly INavigateToSearchCallback _callback;
private readonly string _searchPattern;
private readonly IImmutableSet<string> _kinds;
private readonly IAsynchronousOperationListener _listener;
private readonly IStreamingProgressTracker _progress_doNotAccessDirectly;

private readonly Document? _activeDocument;
Expand All @@ -47,13 +49,15 @@ private NavigateToSearcher(
Solution solution,
INavigateToSearchCallback callback,
string searchPattern,
IImmutableSet<string> kinds)
IImmutableSet<string> kinds,
IAsynchronousOperationListener listener)
{
_host = host;
_solution = solution;
_callback = callback;
_searchPattern = searchPattern;
_kinds = kinds;
_listener = listener;
_progress_doNotAccessDirectly = new StreamingProgressTracker((current, maximum, ct) =>
{
callback.ReportProgress(current, maximum);
Expand All @@ -80,7 +84,7 @@ public static NavigateToSearcher Create(
INavigateToSearcherHost? host = null)
{
host ??= new DefaultNavigateToSearchHost(solution, asyncListener, disposalToken);
return new NavigateToSearcher(host, solution, callback, searchPattern, kinds);
return new NavigateToSearcher(host, solution, callback, searchPattern, kinds, asyncListener);
}

private async Task AddProgressItemsAsync(int count, CancellationToken cancellationToken)
Expand Down Expand Up @@ -112,6 +116,10 @@ public async Task SearchAsync(
{
using var navigateToSearch = Logger.LogBlock(FunctionId.NavigateTo_Search, KeyValueLogMessage.Create(LogType.UserAction), cancellationToken);

// We're potentially about to make many calls over to our OOP service to perform searches. Ensure the
// solution we're searching stays pinned between us and it while this is happening.
using var _ = RemoteKeepAliveSession.Create(_solution, _listener);

if (searchCurrentDocument)
{
await SearchCurrentDocumentAsync(cancellationToken).ConfigureAwait(false);
Expand Down Expand Up @@ -191,25 +199,12 @@ await AddProgressItemsAsync(
// we're fully loaded (and thus have all the information necessary to properly run generators).
if (searchRegularDocuments)
{
// We do at least two passes. One for cached docs. One for normal docs.
await AddProgressItemsAsync(
projectCount * 2,
cancellationToken).ConfigureAwait(false);

await AddProgressItemsAsync(projectCount, cancellationToken).ConfigureAwait(false);
await SearchCachedDocumentsAsync(orderedProjects, seenItems, cancellationToken).ConfigureAwait(false);

// If searching cached data returned any results, then we're done. We've at least shown some results
// to the user. That will hopefully serve them well enough until the solution fully loads.
if (seenItems.Count > 0)
return;

await SearchFullyLoadedProjectsAsync(orderedProjects, seenItems, cancellationToken).ConfigureAwait(false);

// Report a telemetry event to track if we found uncached items after failing to find cached items.
// In practice if we see that we are always finding uncached items, then it's likely something
// has broken in the caching system since we would expect to normally find values there. Specifically
// we expect: foundFullItems <<< not foundFullItems.
Logger.Log(FunctionId.NavigateTo_CacheItemsMiss, KeyValueLogMessage.Create(m => m["FoundFullItems"] = seenItems.Count > 0));
// Note: we only bother searching cached documents during this time. Telemetry shows no meaningful
// change if we do a full search after this point. That prevents us from showing the user a
// glacially slow progress meter as we load everything and end up finding nothing.
}
}
}
Expand Down
15 changes: 12 additions & 3 deletions src/Workspaces/Core/Portable/Remote/IRemoteKeepAliveService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ internal sealed class RemoteKeepAliveSession : IDisposable
{
private readonly CancellationTokenSource _cancellationTokenSource = new();

private RemoteKeepAliveSession(Solution solution, IAsynchronousOperationListener listener)
private RemoteKeepAliveSession(SolutionState solution, IAsynchronousOperationListener listener)
{
var cancellationToken = _cancellationTokenSource.Token;
var token = listener.BeginAsyncOperation(nameof(RemoteKeepAliveSession));
Expand Down Expand Up @@ -72,9 +72,18 @@ public void Dispose()
/// the same solution during the life of this 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>
/// <remarks>
/// The <paramref name="listener"/> is not strictly necessary for this type. This class functions just as an
/// optimization to hold onto data so it isn't resync'ed or recomputed. However, we still want to let the
/// system know when unobserved async work is kicked off in case we have any tooling that keep track of this for
/// any reason (for example for tracking down problems in testing scenarios).
/// </remarks>
public static RemoteKeepAliveSession Create(Solution solution, IAsynchronousOperationListener listener)
=> Create(solution.State, listener);

/// <inheritdoc cref="Create(Solution, IAsynchronousOperationListener)"/>
public static RemoteKeepAliveSession Create(
Solution solution,
IAsynchronousOperationListener listener)
SolutionState solution, IAsynchronousOperationListener listener)
{
return new RemoteKeepAliveSession(solution, listener);
}
Expand Down
11 changes: 10 additions & 1 deletion src/Workspaces/Core/Portable/Remote/RemoteHostClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,20 @@ public async ValueTask<Optional<TResult>> TryInvokeAsync<TService, TResult>(

// solution, no callback:

public async ValueTask<bool> TryInvokeAsync<TService>(
public ValueTask<bool> TryInvokeAsync<TService>(
Solution solution,
Func<TService, Checksum, CancellationToken, ValueTask> invocation,
CancellationToken cancellationToken)
where TService : class
{
return TryInvokeAsync(solution.State, invocation, cancellationToken);
}

public async ValueTask<bool> TryInvokeAsync<TService>(
SolutionState solution,
Func<TService, Checksum, CancellationToken, ValueTask> invocation,
CancellationToken cancellationToken)
where TService : class
{
using var connection = CreateConnection<TService>(callbackTarget: null);
return await connection.TryInvokeAsync(solution, invocation, cancellationToken).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Remote;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.SourceGeneration;
using Microsoft.CodeAnalysis.SourceGeneratorTelemetry;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -98,7 +99,12 @@ await generatorInfo.Documents.States.Values.SelectAsArrayAsync(
if (client is null)
return null;

// We're going to be making multiple calls over to OOP. No point in resyncing data multiple times. Keep a
// single connection, and keep this solution instance alive (and synced) on both sides of the connection
// throughout the calls.
var listenerProvider = solution.Services.ExportProvider.GetExports<IAsynchronousOperationListenerProvider>().First().Value;
using var connection = client.CreateConnection<IRemoteSourceGenerationService>(callbackTarget: null);
using var _ = RemoteKeepAliveSession.Create(solution, listenerProvider.GetListener(FeatureAttribute.Workspace));

// First, grab the info from our external host about the generated documents it has for this project.
var projectId = this.ProjectState.Id;
Expand Down
Loading