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

Perform code-lens 'find' operations serially. #51395

Merged
merged 8 commits into from
Feb 23, 2021
Merged
Show file tree
Hide file tree
Changes from 4 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
21 changes: 17 additions & 4 deletions src/EditorFeatures/Test2/FindReferences/FindReferencesTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,11 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.FindReferences
Await TestStreamingFeature(element, searchSingleFileOnly, uiVisibleOnly, host)
End Function

Private Shared Async Function TestStreamingFeature(element As XElement,
searchSingleFileOnly As Boolean,
uiVisibleOnly As Boolean,
host As TestHost) As Task
Private Shared Async Function TestStreamingFeature(
element As XElement,
searchSingleFileOnly As Boolean,
uiVisibleOnly As Boolean,
host As TestHost) As Task
' We don't support testing features that only expect partial results.
If searchSingleFileOnly OrElse uiVisibleOnly Then
Return
Expand Down Expand Up @@ -254,7 +255,19 @@ Namespace Microsoft.CodeAnalysis.Editor.UnitTests.FindReferences
Optional uiVisibleOnly As Boolean = False,
Optional options As FindReferencesSearchOptions = Nothing) As Task

Await TestAPI(definition, host, parallel:=False, searchSingleFileOnly, uiVisibleOnly, options)
Await TestAPI(definition, host, parallel:=True, searchSingleFileOnly, uiVisibleOnly, options)
End Function

Private Async Function TestAPI(
definition As XElement,
host As TestHost,
parallel As Boolean,
searchSingleFileOnly As Boolean,
uiVisibleOnly As Boolean,
options As FindReferencesSearchOptions) As Task
options = If(options, FindReferencesSearchOptions.Default)
options = options.With(parallel:=parallel)
Using workspace = TestWorkspace.Create(definition, composition:=s_composition.WithTestHostParts(host))
workspace.SetTestLogger(AddressOf _outputHelper.WriteLine)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ protected override async Task UpdateReferencesAsync(
//
// This can save a lot of extra time spent finding callers, especially for methods with
// high fan-out (like IDisposable.Dispose()).
var findRefsOptions = FindReferencesSearchOptions.Default.WithCascade(false);
var findRefsOptions = FindReferencesSearchOptions.Default.With(cascade: false);
var references = await SymbolFinder.FindReferencesAsync(
implMember, solution, findRefsOptions, cancellationToken).ConfigureAwait(false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ internal sealed class CodeLensReferencesService : ICodeLensReferencesService
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
memberOptions: SymbolDisplayMemberOptions.IncludeContainingType);

// Do not perform the finding operation in parallel. We're running ephemerally in the BG and do not want to
// saturate the system with work that then slows the user down.
private static readonly FindReferencesSearchOptions s_nonParallelSearch =
FindReferencesSearchOptions.Default.With(parallel: false);

private static async Task<T?> FindAsync<T>(Solution solution, DocumentId documentId, SyntaxNode syntaxNode,
Func<CodeLensFindReferencesProgress, Task<T>> onResults, Func<CodeLensFindReferencesProgress, Task<T>> onCapped,
int searchCap, CancellationToken cancellationToken) where T : struct
Expand All @@ -49,8 +54,8 @@ internal sealed class CodeLensReferencesService : ICodeLensReferencesService
using var progress = new CodeLensFindReferencesProgress(symbol, syntaxNode, searchCap, cancellationToken);
try
{
await SymbolFinder.FindReferencesAsync(symbol, solution, progress, null,
progress.CancellationToken).ConfigureAwait(false);
await SymbolFinder.FindReferencesAsync(
symbol, solution, progress, documents: null, s_nonParallelSearch, progress.CancellationToken).ConfigureAwait(false);

return await onResults(progress).ConfigureAwait(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ protected static async Task<ImmutableArray<ReferenceLocation>> GetReferenceLocat
// Do not cascade when finding references to this local. Cascading can cause us to find linked
// references as well which can throw things off for us. For inline variable, we only care about the
// direct real references in this project context.
var options = FindReferencesSearchOptions.Default.WithCascade(cascade: false);
var options = FindReferencesSearchOptions.Default.With(cascade: false);

var findReferencesResult = await SymbolFinder.FindReferencesAsync(
local, document.Project.Solution, options, cancellationToken).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ internal partial class FindReferencesSearchEngine
private readonly CancellationToken _cancellationToken;
private readonly FindReferencesSearchOptions _options;

/// <summary>
/// Scheduler to run our tasks on. If we're in <see cref="FindReferencesSearchOptions.Parallel"/> mode, we'll
/// run all our tasks concurrently. Otherwise, we will run them serially.
/// </summary>
private readonly TaskScheduler _scheduler;
Copy link
Member

Choose a reason for hiding this comment

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

💡 At this point, this could be a property.


public FindReferencesSearchEngine(
Solution solution,
IImmutableSet<Document>? documents,
Expand All @@ -45,6 +51,9 @@ public FindReferencesSearchEngine(
_options = options;

_progressTracker = progress.ProgressTracker;

var pair = new ConcurrentExclusiveSchedulerPair();
_scheduler = _options.Parallel ? pair.ConcurrentScheduler : pair.ExclusiveScheduler;
sharwell marked this conversation as resolved.
Show resolved Hide resolved
}

public async Task FindReferencesAsync(ISymbol symbol)
Expand Down Expand Up @@ -87,7 +96,7 @@ private async Task ProcessAsync(ProjectToDocumentMap projectToDocumentMap)
using var _ = ArrayBuilder<Task>.GetInstance(out var tasks);

foreach (var (project, documentMap) in projectToDocumentMap)
tasks.Add(Task.Run(() => ProcessProjectAsync(project, documentMap), _cancellationToken));
tasks.Add(Task.Factory.StartNew(() => ProcessProjectAsync(project, documentMap), _cancellationToken, TaskCreationOptions.None, _scheduler).Unwrap());

await Task.WhenAll(tasks).ConfigureAwait(false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ private async Task<ProjectToDocumentMap> CreateProjectToDocumentMapAsync(Project
{
foreach (var (symbol, finder) in projectQueue)
{
tasks.Add(Task.Run(() =>
DetermineDocumentsToSearchAsync(project, symbol, finder), _cancellationToken));
tasks.Add(Task.Factory.StartNew(() =>
DetermineDocumentsToSearchAsync(project, symbol, finder), _cancellationToken, TaskCreationOptions.None, _scheduler).Unwrap());
}
}

Expand Down Expand Up @@ -139,7 +139,7 @@ private async Task DetermineAllSymbolsCoreAsync(
using var _ = ArrayBuilder<Task>.GetInstance(out var finderTasks);
foreach (var f in _finders)
{
finderTasks.Add(Task.Run(async () =>
finderTasks.Add(Task.Factory.StartNew(async () =>
{
using var _ = ArrayBuilder<Task>.GetInstance(out var symbolTasks);

Expand All @@ -159,7 +159,7 @@ private async Task DetermineAllSymbolsCoreAsync(
_cancellationToken.ThrowIfCancellationRequested();

await Task.WhenAll(symbolTasks).ConfigureAwait(false);
}, _cancellationToken));
}, _cancellationToken, TaskCreationOptions.None, _scheduler).Unwrap());
}

await Task.WhenAll(finderTasks).ConfigureAwait(false);
Expand All @@ -177,7 +177,8 @@ private void AddSymbolTasks(
{
Contract.ThrowIfNull(child);
_cancellationToken.ThrowIfCancellationRequested();
symbolTasks.Add(Task.Run(() => DetermineAllSymbolsCoreAsync(child, result), _cancellationToken));
symbolTasks.Add(Task.Factory.StartNew(
() => DetermineAllSymbolsCoreAsync(child, result), _cancellationToken, TaskCreationOptions.None, _scheduler).Unwrap());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ private async Task ProcessProjectAsync(
foreach (var (document, documentQueue) in documentMap)
{
if (document.Project == project)
documentTasks.Add(Task.Run(() => ProcessDocumentQueueAsync(document, documentQueue), _cancellationToken));
documentTasks.Add(Task.Factory.StartNew(() => ProcessDocumentQueueAsync(document, documentQueue), _cancellationToken, TaskCreationOptions.None, _scheduler).Unwrap());
}

await Task.WhenAll(documentTasks).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Runtime.Serialization;
using Microsoft.CodeAnalysis.Shared.Extensions;

Expand All @@ -15,7 +13,8 @@ internal sealed class FindReferencesSearchOptions
public static readonly FindReferencesSearchOptions Default =
new(
associatePropertyReferencesWithSpecificAccessor: false,
cascade: true);
cascade: true,
parallel: true);

/// <summary>
/// When searching for property, associate specific references we find to the relevant
Expand All @@ -39,26 +38,52 @@ internal sealed class FindReferencesSearchOptions
[DataMember(Order = 1)]
public bool Cascade { get; }

/// <summary>
/// Whether or not we should perform the find operation in parallel. This can produce results more quickly, but
/// at the cost of more system resources.
/// </summary>
/// <remarks>
/// Features that run automatically should consider setting this to <see langword="false"/> to avoid
/// unnecessarily impacting the user while they are doing other work.
/// </remarks>
[DataMember(Order = 2)]
public bool Parallel { get; }

public FindReferencesSearchOptions(
bool associatePropertyReferencesWithSpecificAccessor,
bool cascade)
bool cascade,
bool parallel)
{
AssociatePropertyReferencesWithSpecificAccessor = associatePropertyReferencesWithSpecificAccessor;
Cascade = cascade;
Parallel = parallel;
}

public FindReferencesSearchOptions WithAssociatePropertyReferencesWithSpecificAccessor(bool associatePropertyReferencesWithSpecificAccessor)
=> new(associatePropertyReferencesWithSpecificAccessor, Cascade);
public FindReferencesSearchOptions With(
Optional<bool> associatePropertyReferencesWithSpecificAccessor = default,
Optional<bool> cascade = default,
Optional<bool> parallel = default)
{
var newAssociatePropertyReferencesWithSpecificAccessor = associatePropertyReferencesWithSpecificAccessor.HasValue ? associatePropertyReferencesWithSpecificAccessor.Value : AssociatePropertyReferencesWithSpecificAccessor;
var newCascade = cascade.HasValue ? cascade.Value : Cascade;
var newParallel = parallel.HasValue ? parallel.Value : Parallel;

if (newAssociatePropertyReferencesWithSpecificAccessor == AssociatePropertyReferencesWithSpecificAccessor &&
newCascade == Cascade &&
newParallel == Parallel)
{
return this;
}

public FindReferencesSearchOptions WithCascade(bool cascade)
=> new(AssociatePropertyReferencesWithSpecificAccessor, cascade);
return new FindReferencesSearchOptions(newAssociatePropertyReferencesWithSpecificAccessor, newCascade, newParallel);
}

/// <summary>
/// For IDE features, if the user starts searching on an accessor, then we want to give
/// results associated with the specific accessor. Otherwise, if they search on a property,
/// then associate everything with the property.
/// </summary>
public static FindReferencesSearchOptions GetFeatureOptionsForStartingSymbol(ISymbol symbol)
=> Default.WithAssociatePropertyReferencesWithSpecificAccessor(symbol.IsPropertyAccessor());
=> Default.With(associatePropertyReferencesWithSpecificAccessor: symbol.IsPropertyAccessor());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ protected override async Task<ImmutableArray<Document>> DetermineDocumentsToSear
// defer to the Property finder to find these docs and combine them with the result.
var propertyDocuments = await ReferenceFinders.Property.DetermineDocumentsToSearchAsync(
property, project, documents,
options.WithAssociatePropertyReferencesWithSpecificAccessor(false),
options.With(associatePropertyReferencesWithSpecificAccessor: false),
cancellationToken).ConfigureAwait(false);

result = result.AddRange(propertyDocuments);
Expand All @@ -77,7 +77,7 @@ protected override async ValueTask<ImmutableArray<FinderLocation>> FindReference
{
var propertyReferences = await ReferenceFinders.Property.FindReferencesInDocumentAsync(
property, document, semanticModel,
options.WithAssociatePropertyReferencesWithSpecificAccessor(false),
options.With(associatePropertyReferencesWithSpecificAccessor: false),
cancellationToken).ConfigureAwait(false);

var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public static async Task<IEnumerable<ReferencedSymbol>> FindReferencesAsync(
FindReferencesSearchOptions.Default, cancellationToken).ConfigureAwait(false);
}

private static async Task<ImmutableArray<ReferencedSymbol>> FindReferencesAsync(
internal static async Task<ImmutableArray<ReferencedSymbol>> FindReferencesAsync(
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
ISymbol symbol,
Solution solution,
IFindReferencesProgress progress,
Expand Down