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
20 changes: 10 additions & 10 deletions src/EditorFeatures/CSharpTest/NavigateTo/NavigateToTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1375,7 +1375,7 @@ public void VisibleMethod_Generated() { }
</Document>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand Down Expand Up @@ -1450,7 +1450,7 @@ public partial class Inner { }
</Document>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand Down Expand Up @@ -1484,7 +1484,7 @@ public partial class Inner2 { }
</Document>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand Down Expand Up @@ -1517,7 +1517,7 @@ public partial class Inner2 { }
</Document>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand Down Expand Up @@ -1547,7 +1547,7 @@ public void VisibleMethod2() { }
</Document>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand Down Expand Up @@ -1582,7 +1582,7 @@ public class Inner { }
</Document>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand All @@ -1609,7 +1609,7 @@ public partial class Outer
</Document>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand All @@ -1636,7 +1636,7 @@ public class Inner {}
</Document>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand All @@ -1662,7 +1662,7 @@ public class C
</DocumentFromSourceGenerator>
</Project>
</Workspace>
""", composition: EditorTestCompositions.EditorFeatures);
""", composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand Down Expand Up @@ -1693,7 +1693,7 @@ public partial class C
}
""",
},
composition: EditorTestCompositions.EditorFeatures);
composition: DefaultComposition);

_provider = CreateProvider(workspace);
_aggregator = new NavigateToTestAggregator(_provider);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,14 +305,14 @@ private protected static CodeActionResolveData CreateCodeActionResolveData(strin
private protected Task<TestLspServer> CreateTestLspServerAsync(string markup, bool mutatingLspWorkspace, LSP.ClientCapabilities clientCapabilities, bool callInitialized = true)
=> CreateTestLspServerAsync([markup], LanguageNames.CSharp, mutatingLspWorkspace, new InitializationOptions { ClientCapabilities = clientCapabilities, CallInitialized = callInitialized });

private protected Task<TestLspServer> CreateTestLspServerAsync(string markup, bool mutatingLspWorkspace, InitializationOptions? initializationOptions = null)
=> CreateTestLspServerAsync([markup], LanguageNames.CSharp, mutatingLspWorkspace, initializationOptions);
private protected Task<TestLspServer> CreateTestLspServerAsync(string markup, bool mutatingLspWorkspace, InitializationOptions? initializationOptions = null, List<Type>? extraExportedTypes = null)
=> CreateTestLspServerAsync([markup], LanguageNames.CSharp, mutatingLspWorkspace, initializationOptions, extraExportedTypes: extraExportedTypes);

private protected Task<TestLspServer> CreateTestLspServerAsync(string[] markups, bool mutatingLspWorkspace, InitializationOptions? initializationOptions = null)
=> CreateTestLspServerAsync(markups, LanguageNames.CSharp, mutatingLspWorkspace, initializationOptions);
private protected Task<TestLspServer> CreateTestLspServerAsync(string[] markups, bool mutatingLspWorkspace, InitializationOptions? initializationOptions = null, List<Type>? extraExportedTypes = null)
=> CreateTestLspServerAsync(markups, LanguageNames.CSharp, mutatingLspWorkspace, initializationOptions, extraExportedTypes: extraExportedTypes);

private protected Task<TestLspServer> CreateVisualBasicTestLspServerAsync(string markup, bool mutatingLspWorkspace, InitializationOptions? initializationOptions = null)
=> CreateTestLspServerAsync([markup], LanguageNames.VisualBasic, mutatingLspWorkspace, initializationOptions);
private protected Task<TestLspServer> CreateVisualBasicTestLspServerAsync(string markup, bool mutatingLspWorkspace, InitializationOptions? initializationOptions = null, List<Type>? extraExportedTypes = null)
=> CreateTestLspServerAsync([markup], LanguageNames.VisualBasic, mutatingLspWorkspace, initializationOptions, extraExportedTypes: extraExportedTypes);

private protected Task<TestLspServer> CreateTestLspServerAsync(
string[] markups, string languageName, bool mutatingLspWorkspace, InitializationOptions? initializationOptions, List<Type>? excludedTypes = null, List<Type>? extraExportedTypes = null, bool commonReferences = true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using System.Windows.Media;
using System.Windows.Media.Imaging;
Expand All @@ -20,11 +21,11 @@
using Microsoft.CodeAnalysis.Editor.Wpf;
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.NavigateTo;
using Microsoft.CodeAnalysis.Remote.Testing;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Test.Utilities;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.Composition;
using Microsoft.VisualStudio.Language.NavigateTo.Interfaces;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.PatternMatching;
Expand All @@ -39,9 +40,9 @@ namespace Microsoft.CodeAnalysis.Editor.UnitTests.NavigateTo
[UseExportProvider]
public abstract class AbstractNavigateToTests
{
protected static readonly TestComposition DefaultComposition = EditorTestCompositions.EditorFeatures;
protected static readonly TestComposition FirstVisibleComposition = EditorTestCompositions.EditorFeatures.AddParts(typeof(FirstDocIsVisibleDocumentTrackingService.Factory));
protected static readonly TestComposition FirstActiveAndVisibleComposition = EditorTestCompositions.EditorFeatures.AddParts(typeof(FirstDocIsActiveAndVisibleDocumentTrackingService.Factory));
protected static readonly TestComposition DefaultComposition = EditorTestCompositions.EditorFeatures.AddParts(typeof(TestWorkspaceNavigateToSearchHostService));
protected static readonly TestComposition FirstVisibleComposition = EditorTestCompositions.EditorFeatures.AddParts(typeof(TestWorkspaceNavigateToSearchHostService), typeof(FirstDocIsVisibleDocumentTrackingService.Factory));
protected static readonly TestComposition FirstActiveAndVisibleComposition = EditorTestCompositions.EditorFeatures.AddParts(typeof(TestWorkspaceNavigateToSearchHostService), typeof(FirstDocIsActiveAndVisibleDocumentTrackingService.Factory));

protected INavigateToItemProvider _provider;
protected NavigateToTestAggregator _aggregator;
Expand Down Expand Up @@ -296,5 +297,15 @@ public IWorkspaceService CreateService(HostWorkspaceServices workspaceServices)
=> new FirstDocIsActiveAndVisibleDocumentTrackingService(workspaceServices.Workspace);
}
}

[ExportWorkspaceService(typeof(IWorkspaceNavigateToSearcherHostService))]
[PartNotDiscoverable, Shared]
[method: ImportingConstructor]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
protected sealed class TestWorkspaceNavigateToSearchHostService() : IWorkspaceNavigateToSearcherHostService
{
public ValueTask<bool> IsFullyLoadedAsync(CancellationToken cancellationToken)
=> new(true);
}
}
}
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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ internal interface INavigateToSearcherHost
ValueTask<bool> IsFullyLoadedAsync(CancellationToken cancellationToken);
}

internal interface IWorkspaceNavigateToSearcherHostService : IWorkspaceService
{
ValueTask<bool> IsFullyLoadedAsync(CancellationToken cancellationToken);
}

internal class DefaultNavigateToSearchHost(
Solution solution,
IAsynchronousOperationListener asyncListener,
Expand All @@ -49,6 +54,10 @@ internal class DefaultNavigateToSearchHost(

public async ValueTask<bool> IsFullyLoadedAsync(CancellationToken cancellationToken)
{
var workspaceService = _solution.Workspace.Services.GetService<IWorkspaceNavigateToSearcherHostService>();
if (workspaceService != null)
return await workspaceService.IsFullyLoadedAsync(cancellationToken).ConfigureAwait(false);

var service = _solution.Services.GetRequiredService<IWorkspaceStatusService>();

// We consider ourselves fully loaded when both the project system has completed loaded
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
Loading