Skip to content

Commit

Permalink
Merge pull request #51395 from CyrusNajmabadi/serialCodeLens
Browse files Browse the repository at this point in the history
Perform code-lens 'find' operations serially.
  • Loading branch information
CyrusNajmabadi authored Feb 23, 2021
2 parents a3753af + d4fa992 commit cd4e071
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 27 deletions.
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, explicit:=False, searchSingleFileOnly, uiVisibleOnly, options)
Await TestAPI(definition, host, explicit:=True, searchSingleFileOnly, uiVisibleOnly, options)
End Function

Private Async Function TestAPI(
definition As XElement,
host As TestHost,
explicit As Boolean,
searchSingleFileOnly As Boolean,
uiVisibleOnly As Boolean,
options As FindReferencesSearchOptions) As Task
options = If(options, FindReferencesSearchOptions.Default)
options = options.With(explicit:=explicit)
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
10 changes: 8 additions & 2 deletions src/Features/Core/Portable/CodeLens/CodeLensReferencesService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ internal sealed class CodeLensReferencesService : ICodeLensReferencesService
typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces,
memberOptions: SymbolDisplayMemberOptions.IncludeContainingType);

// Set ourselves as an implicit invocation of FindReferences. This will cause the finding operation to operate
// in serial, not 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(@explicit: 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 +55,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,13 @@ 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.Explicit"/> mode, we'll
/// run all our tasks concurrently. Otherwise, we will run them serially using <see cref="s_exclusiveScheduler"/>
/// </summary>
private readonly TaskScheduler _scheduler;
private static readonly TaskScheduler s_exclusiveScheduler = new ConcurrentExclusiveSchedulerPair().ExclusiveScheduler;

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

_progressTracker = progress.ProgressTracker;

// If we're an explicit invocation, just defer to the threadpool to execute all our work in parallel to get
// things done as quickly as possible. If we're running implicitly, then use a
// ConcurrentExclusiveSchedulerPair's exclusive scheduler as that's the most built-in way in the TPL to get
// will run things serially.
_scheduler = _options.Explicit ? TaskScheduler.Default : s_exclusiveScheduler;
}

public async Task FindReferencesAsync(ISymbol symbol)
Expand Down Expand Up @@ -87,7 +100,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,
@explicit: 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 this find ref operation was explicitly invoked or not. If explicit invoked, the find
/// references operation may use more resources to get the results faster.
/// </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 Explicit { get; }

public FindReferencesSearchOptions(
bool associatePropertyReferencesWithSpecificAccessor,
bool cascade)
bool cascade,
bool @explicit)
{
AssociatePropertyReferencesWithSpecificAccessor = associatePropertyReferencesWithSpecificAccessor;
Cascade = cascade;
Explicit = @explicit;
}

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

if (newAssociatePropertyReferencesWithSpecificAccessor == AssociatePropertyReferencesWithSpecificAccessor &&
newCascade == Cascade &&
newExplicit == Explicit)
{
return this;
}

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

/// <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(
ISymbol symbol,
Solution solution,
IFindReferencesProgress progress,
Expand Down

0 comments on commit cd4e071

Please sign in to comment.