Skip to content

Commit

Permalink
Merge pull request #19539 from CyrusNajmabadi/gotodefPerf
Browse files Browse the repository at this point in the history
Fix perf regression in gotodef.
  • Loading branch information
CyrusNajmabadi authored May 17, 2017
2 parents 3f35adf + 853efd2 commit 3fb20b5
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public static async Task<ClassifiedSpansAndHighlightSpan> ClassifyAsync(
// If the document span is providing us with the classified spans up front, then we
// can just use that. Otherwise, go back and actually classify the text for the line
// the document span is on.
if (documentSpan.Properties.TryGetValue(Key, out var value))
if (documentSpan.Properties != null &&
documentSpan.Properties.TryGetValue(Key, out var value))
{
return (ClassifiedSpansAndHighlightSpan)value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis.Host;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.FindUsages
{
Expand Down Expand Up @@ -47,7 +48,7 @@ public static DefinitionItem ToNonClassifiedDefinitionItem(
// to compute the classified spans for the locations of the definition. So it's totally
// fine to pass in CancellationToken.None and block on the result.
return ToDefinitionItemAsync(definition, solution, includeHiddenLocations,
includeClassifiedSpans: false, cancellationToken: CancellationToken.None).Result;
includeClassifiedSpans: false, cancellationToken: CancellationToken.None).WaitAndGetResult_CanCallOnBackground(CancellationToken.None);
}

public static Task<DefinitionItem> ToClassifiedDefinitionItemAsync(
Expand Down
23 changes: 21 additions & 2 deletions src/EditorFeatures/Core/GoToDefinition/GoToDefinitionHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,27 @@ public static bool TryGoToDefinition(
}

var definitions = ArrayBuilder<DefinitionItem>.GetInstance();
var definitionItem = symbol.ToClassifiedDefinitionItemAsync(
solution, includeHiddenLocations: true, cancellationToken: cancellationToken).WaitAndGetResult(cancellationToken);

// Going to a symbol may end up actually showing the symbol in the Find-Usages window.
// This happens when there is more than one location for the symbol (i.e. for partial
// symbols) and we don't know the best place to take you to.
//
// The FindUsages window supports showing the classified text for an item. It does this
// in two ways. Either the item can pass along its classified text (and the window will
// defer to that), or the item will have no classified text, and the window will compute
// it in the BG.
//
// Passing along the classified information is valuable for OOP scenarios where we want
// all that expensive computation done on the OOP side and not in the VS side.
//
// However, Go To Definition is all in-process, and is also synchronous. So we do not
// want to fetch the classifications here. It slows down the command and leads to a
// measurable delay in our perf tests.
//
// So, if we only have a single location to go to, this does no unnecessary work. And,
// if we do have multiple locations to show, it will just be done in the BG, unblocking
// this command thread so it can return the user faster.
var definitionItem = symbol.ToNonClassifiedDefinitionItem(solution, includeHiddenLocations: true);

if (thirdPartyNavigationAllowed)
{
Expand Down

0 comments on commit 3fb20b5

Please sign in to comment.