Skip to content

Commit

Permalink
Merge pull request #48356 from sharwell/perf-1224834
Browse files Browse the repository at this point in the history
Implement low-hanging performance improvements from feedback investigation
  • Loading branch information
sharwell authored Oct 7, 2020
2 parents 055d05c + 4646331 commit bc85a40
Show file tree
Hide file tree
Showing 39 changed files with 263 additions and 129 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public SemanticModelReuseWorkspaceService(Workspace _)
{
}

public Task<SemanticModel> ReuseExistingSpeculativeModelAsync(Document document, SyntaxNode node, CancellationToken cancellationToken)
public ValueTask<SemanticModel> ReuseExistingSpeculativeModelAsync(Document document, SyntaxNode node, CancellationToken cancellationToken)
{
// TODO: port the GetSemanticModelForNodeAsync implementation from Workspaces layer,
// which currently relies on a bunch of internal APIs.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Licensed to the .NET Foundation under one or more agreements.
// 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.Concurrent;
using Roslyn.Utilities;
using Xunit;

namespace Microsoft.CodeAnalysis.UnitTests.InternalUtilities
{
public class ConcurrentDictionaryExtensionsTests
{
[Fact]
public void TestAdd()
{
var dictionary = new ConcurrentDictionary<int, int>();
dictionary.Add(0, 0);
Assert.Equal(0, dictionary[0]);

Assert.Throws<ArgumentException>(() => dictionary.Add(0, 0));
}

[Fact]
public void TestGetOrAdd()
{
var first = new object();
var second = new object();

var dictionary = new ConcurrentDictionary<int, object>();
Assert.Same(first, dictionary.GetOrAdd(0, static (key, arg) => arg, first));
Assert.Same(first, dictionary[0]);
Assert.Single(dictionary);
Assert.Same(first, dictionary.GetOrAdd(0, static (key, arg) => arg, second));
Assert.Same(first, dictionary[0]);
Assert.Single(dictionary);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// 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.Concurrent;
using Microsoft.CodeAnalysis.PooledObjects;

namespace Roslyn.Utilities
{
Expand All @@ -19,8 +21,22 @@ public static void Add<K, V>(this ConcurrentDictionary<K, V> dict, K key, V valu
{
if (!dict.TryAdd(key, value))
{
throw new System.ArgumentException("adding a duplicate");
throw new ArgumentException("adding a duplicate", nameof(key));
}
}

public static TValue GetOrAdd<TKey, TArg, TValue>(this ConcurrentDictionary<TKey, TValue> dictionary, TKey key, Func<TKey, TArg, TValue> valueFactory, TArg factoryArgument)
where TKey : notnull
{
#if NETCOREAPP
return dictionary.GetOrAdd(key, valueFactory, factoryArgument);
#else
if (dictionary.TryGetValue(key, out var value))
return value;

using var _ = PooledDelegates.GetPooledFunction(valueFactory, factoryArgument, out var boundFunction);
return dictionary.GetOrAdd(key, boundFunction);
#endif
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public override string ToString()

internal AddedParameter GetAddedParameter(Document document)
{
var semanticModel = document.GetRequiredSemanticModelAsync(CancellationToken.None).Result;
var semanticModel = document.GetRequiredSemanticModelAsync(CancellationToken.None).AsTask().Result;

var type = document.Project.Language switch
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ protected override Task<ImmutableArray<Document>> DetermineDocumentsToSearchAsyn
return Task.FromResult(project.Documents.ToImmutableArray());
}

protected override async Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
protected override async ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
IMethodSymbol methodSymbol,
Document document,
SemanticModel semanticModel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public Task PopulateIndicesAsync(CancellationToken cancellationToken)
foreach (var peReference in GetAllRelevantPeReferences())
{
tasks.Add(Task.Run(()
=> SymbolTreeInfo.GetInfoForMetadataReferenceAsync(Solution, peReference, loadOnly: false, cancellationToken), cancellationToken));
=> SymbolTreeInfo.GetInfoForMetadataReferenceAsync(Solution, peReference, loadOnly: false, cancellationToken).AsTask(), cancellationToken));
}

return Task.WhenAll(tasks.ToImmutable());
Expand All @@ -100,7 +100,7 @@ public Task PopulateIndicesAsync(CancellationToken cancellationToken)
tasks.Add(Task.Run(() => GetExtensionMethodSymbolsFromPeReferenceAsync(
peReference,
forceIndexCreation,
cancellationToken), cancellationToken));
cancellationToken).AsTask(), cancellationToken));
}

foreach (var project in GetAllRelevantProjects())
Expand Down Expand Up @@ -184,7 +184,7 @@ private ImmutableArray<PortableExecutableReference> GetAllRelevantPeReferences()
: GetExtensionMethodsForSymbolsFromDifferentCompilation(matchingMethodSymbols, cancellationToken);
}

private async Task<ImmutableArray<IMethodSymbol>?> GetExtensionMethodSymbolsFromPeReferenceAsync(
private async ValueTask<ImmutableArray<IMethodSymbol>?> GetExtensionMethodSymbolsFromPeReferenceAsync(
PortableExecutableReference peReference,
bool forceIndexCreation,
CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public SymbolTreeInfoCacheService(
_metadataIdToInfo = metadataIdToInfo;
}

public async Task<SymbolTreeInfo> TryGetMetadataSymbolTreeInfoAsync(
public async ValueTask<SymbolTreeInfo> TryGetMetadataSymbolTreeInfoAsync(
Solution solution,
PortableExecutableReference reference,
CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ private void FormatDocumentCreatedFromTemplate(IVsHierarchy hierarchy, uint item

// Organize using directives
addedDocument = ThreadHelper.JoinableTaskFactory.Run(() => OrganizeUsingsCreatedFromTemplateAsync(addedDocument, cancellationToken));
rootToFormat = ThreadHelper.JoinableTaskFactory.Run(() => addedDocument.GetRequiredSyntaxRootAsync(cancellationToken));
rootToFormat = ThreadHelper.JoinableTaskFactory.Run(() => addedDocument.GetRequiredSyntaxRootAsync(cancellationToken).AsTask());

// Format document
var unformattedText = addedDocument.GetTextSynchronously(cancellationToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ internal class AddParameterDialogViewModel : AbstractNotifyPropertyChanged
public AddParameterDialogViewModel(Document document, int positionForTypeBinding)
{
_notificationService = document.Project.Solution.Workspace.Services.GetService<INotificationService>();
_semanticModel = document.GetRequiredSemanticModelAsync(CancellationToken.None).WaitAndGetResult_CanCallOnBackground(CancellationToken.None);
_semanticModel = document.GetRequiredSemanticModelAsync(CancellationToken.None).AsTask().WaitAndGetResult_CanCallOnBackground(CancellationToken.None);

TypeIsEmptyImage = Visibility.Visible;
TypeBindsImage = Visibility.Collapsed;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ namespace Microsoft.CodeAnalysis.ExternalAccess.Pythia.Api
internal static class PythiaDocumentExtensions
{
public static Task<SemanticModel> GetSemanticModelForNodeAsync(this Document document, SyntaxNode? node, CancellationToken cancellationToken)
=> DocumentExtensions.ReuseExistingSpeculativeModelAsync(document, node, cancellationToken);
=> DocumentExtensions.ReuseExistingSpeculativeModelAsync(document, node, cancellationToken).AsTask();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.LanguageServices;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;
Expand All @@ -32,7 +33,7 @@ public static SymbolInfo GetSymbolInfo(SemanticModel model, SyntaxNode node, Can
return model.GetSymbolInfo(node, cancellationToken);
}

return nodeCache.GetOrAdd(node, n => model.GetSymbolInfo(n, cancellationToken));
return nodeCache.GetOrAdd(node, static (n, arg) => arg.model.GetSymbolInfo(n, arg.cancellationToken), (model, cancellationToken));
}

public static IAliasSymbol GetAliasInfo(
Expand Down Expand Up @@ -84,25 +85,34 @@ public static ImmutableArray<SyntaxToken> GetIdentifierOrGlobalNamespaceTokensWi
syntaxFacts, root, sourceText, key, cancellationToken));
}

[PerformanceSensitive("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1224834", AllowCaptures = false)]
private static ImmutableArray<SyntaxToken> GetIdentifierOrGlobalNamespaceTokensWithText(
ISyntaxFactsService syntaxFacts, SyntaxNode root, SourceText sourceText,
string text, CancellationToken cancellationToken)
{
// identifier is not escaped
if (sourceText != null)
{
return GetTokensFromText(syntaxFacts, root, sourceText, text, IsCandidate, cancellationToken);
// identifier is not escaped
Func<SyntaxToken, ISyntaxFactsService, string, bool> isCandidate = static (t, syntaxFacts, text) => IsCandidate(t, syntaxFacts, text);
return GetTokensFromText(syntaxFacts, root, sourceText, text, isCandidate, cancellationToken);
}
else
{
// identifier is escaped
using var _ = PooledDelegates.GetPooledFunction<SyntaxToken, (ISyntaxFactsService syntaxFacts, string text), bool>(
static (t, arg) => IsCandidate(t, arg.syntaxFacts, arg.text),
(syntaxFacts, text),
out var isCandidate);

// identifier is escaped
return root.DescendantTokens(descendIntoTrivia: true).Where(IsCandidate).ToImmutableArray();
return root.DescendantTokens(descendIntoTrivia: true).Where(isCandidate).ToImmutableArray();
}

bool IsCandidate(SyntaxToken t)
static bool IsCandidate(SyntaxToken t, ISyntaxFactsService syntaxFacts, string text)
=> syntaxFacts.IsGlobalNamespaceKeyword(t) || (syntaxFacts.IsIdentifier(t) && syntaxFacts.TextMatch(t.ValueText, text));
}

private static ImmutableArray<SyntaxToken> GetTokensFromText(
ISyntaxFactsService syntaxFacts, SyntaxNode root, SourceText content, string text, Func<SyntaxToken, bool> candidate, CancellationToken cancellationToken)
ISyntaxFactsService syntaxFacts, SyntaxNode root, SourceText content, string text, Func<SyntaxToken, ISyntaxFactsService, string, bool> candidate, CancellationToken cancellationToken)
{
if (text.Length == 0)
{
Expand All @@ -120,7 +130,7 @@ private static ImmutableArray<SyntaxToken> GetTokensFromText(

var token = root.FindToken(index, findInsideTrivia: true);
var span = token.Span;
if (!token.IsMissing && span.Start == index && span.Length == text.Length && candidate(token))
if (!token.IsMissing && span.Start == index && span.Length == text.Length && candidate(token, syntaxFacts, text))
{
result.Add(token);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ protected override Task<ImmutableArray<Document>> DetermineDocumentsToSearchAsyn
return Task.FromResult(ImmutableArray.Create(document));
}

protected override async Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
protected override async ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
TSymbol symbol,
Document document,
SemanticModel semanticModel,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public abstract Task<ImmutableArray<ISymbol>> DetermineCascadedSymbolsAsync(
public abstract Task<ImmutableArray<Document>> DetermineDocumentsToSearchAsync(
ISymbol symbol, Project project, IImmutableSet<Document> documents, FindReferencesSearchOptions options, CancellationToken cancellationToken);

public abstract Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
public abstract ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
ISymbol symbol, Document document, SemanticModel semanticModel, FindReferencesSearchOptions options, CancellationToken cancellationToken);

protected static bool TryGetNameWithoutAttributeSuffix(
Expand Down Expand Up @@ -149,7 +149,7 @@ protected static Task<ImmutableArray<Document>> FindDocumentsAsync(
protected static bool IdentifiersMatch(ISyntaxFactsService syntaxFacts, string name, SyntaxToken token)
=> syntaxFacts.IsIdentifier(token) && syntaxFacts.TextMatch(token.ValueText, name);

protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingIdentifierAsync(
protected static ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingIdentifierAsync(
ISymbol symbol,
string identifier,
Document document,
Expand All @@ -161,7 +161,7 @@ protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentUs
cancellationToken: cancellationToken);
}

protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingIdentifierAsync(
protected static ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingIdentifierAsync(
ISymbol symbol,
string identifier,
Document document,
Expand All @@ -174,7 +174,7 @@ protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentUs
symbol, identifier, document, semanticModel, symbolsMatch, cancellationToken);
}

protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingIdentifierAsync(
protected static ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingIdentifierAsync(
ISymbol symbol,
string identifier,
Document document,
Expand All @@ -188,7 +188,8 @@ protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentUs
docCommentId, findInGlobalSuppressions, cancellationToken);
}

protected static async Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingIdentifierAsync(
[PerformanceSensitive("https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1224834", OftenCompletesSynchronously = true)]
protected static async ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingIdentifierAsync(
string identifier,
Document document,
SemanticModel semanticModel,
Expand Down Expand Up @@ -900,7 +901,7 @@ protected abstract Task<ImmutableArray<Document>> DetermineDocumentsToSearchAsyn
TSymbol symbol, Project project, IImmutableSet<Document> documents,
FindReferencesSearchOptions options, CancellationToken cancellationToken);

protected abstract Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
protected abstract ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
TSymbol symbol, Document document, SemanticModel semanticModel,
FindReferencesSearchOptions options, CancellationToken cancellationToken);

Expand All @@ -920,13 +921,13 @@ public override Task<ImmutableArray<Document>> DetermineDocumentsToSearchAsync(
: SpecializedTasks.EmptyImmutableArray<Document>();
}

public override Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
public override ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
ISymbol symbol, Document document, SemanticModel semanticModel,
FindReferencesSearchOptions options, CancellationToken cancellationToken)
{
return symbol is TSymbol typedSymbol && CanFind(typedSymbol)
? FindReferencesInDocumentAsync(typedSymbol, document, semanticModel, options, cancellationToken)
: SpecializedTasks.EmptyImmutableArray<FinderLocation>();
: new ValueTask<ImmutableArray<FinderLocation>>(ImmutableArray<FinderLocation>.Empty);
}

public override Task<ImmutableArray<ISymbol>> DetermineCascadedSymbolsAsync(
Expand Down Expand Up @@ -959,7 +960,7 @@ protected virtual Task<ImmutableArray<ISymbol>> DetermineCascadedSymbolsAsync(
return SpecializedTasks.EmptyImmutableArray<ISymbol>();
}

protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingSymbolNameAsync(
protected static ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentUsingSymbolNameAsync(
TSymbol symbol,
Document document,
SemanticModel semanticModel,
Expand Down Expand Up @@ -1002,7 +1003,7 @@ protected static ValueTask<ImmutableArray<FinderLocation>> FindReferencesInToken
cancellationToken);
}

protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
protected static ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
TSymbol symbol,
Document document,
SemanticModel semanticModel,
Expand All @@ -1014,7 +1015,7 @@ protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAs
findParentNode: null, cancellationToken: cancellationToken);
}

protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
protected static ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
TSymbol symbol,
Document document,
SemanticModel semanticModel,
Expand All @@ -1028,7 +1029,7 @@ protected static Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAs
symbolsMatchAsync, docCommentId, findInGlobalSuppressions, cancellationToken);
}

protected static async Task<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
protected static async ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
Document document,
SemanticModel semanticModel,
Func<SyntaxToken, bool> tokensMatch,
Expand Down
Loading

0 comments on commit bc85a40

Please sign in to comment.