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.

Sign up for GitHub

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

Always try to use cached data for import completion even if outdated #59760

Merged
merged 22 commits into from
Mar 22, 2022
Merged

Always try to use cached data for import completion even if outdated #59760

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
427d9ea
Merge remote-tracking branch 'dotnet/main' into MultiPhaseCompletion
genlu Feb 9, 2022
ce6a216
Merge branch 'sessionData' into MultiPhaseCompletion2
genlu Feb 17, 2022
d84cca7
Merge remote-tracking branch 'dotnet/main' into MultiPhaseCompletion2
genlu Feb 24, 2022
e65f7fd
Use cached data for completion of unimported types
genlu Feb 25, 2022
67c6dad
Use cached data for completion of unimported extension methods
genlu Feb 25, 2022
c635be6
Try to refresh import completion cache in the background whenever com…
genlu Feb 25, 2022
bae71ac
Use AsyncBatchingWorkQueue for background cache updates
genlu Feb 26, 2022
e663da0
Fix tests
genlu Feb 28, 2022
9785979
clean up
genlu Mar 1, 2022
9053574
Fix tests
genlu Mar 3, 2022
f8f9782
Make sure background task to update cache always run
genlu Mar 3, 2022
cd6c665
Merge remote-tracking branch 'dotnet/main' into UseCache
genlu Mar 3, 2022
ea57f09
Expose ForceExpandedCompletionIndexCreation option to O#
genlu Mar 3, 2022
478bf38
Merge remote-tracking branch 'dotnet/main' into UseCache
genlu Mar 9, 2022
e760f26
Don't use Workspace.CurrentSolution when refreshing cache in background
genlu Mar 9, 2022
205766f
Use AsynchronousOperationListener for background workqueues
genlu Mar 10, 2022
f77a0ad
Update src/Tools/ExternalAccess/OmniSharp/Completion/OmniSharpComplet…
genlu Mar 10, 2022
534dca0
Merge remote-tracking branch 'dotnet/main' into UseCache
genlu Mar 15, 2022
5ecff82
Pass ThreadingContext.DisposalToken to workqueue for cache refresh
genlu Mar 15, 2022
c44946e
Remove namespace imports
genlu Mar 16, 2022
072defb
Refactoring
genlu Mar 18, 2022
ebe077f
pass the listener to the constructor
genlu Mar 21, 2022
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
11 changes: 11 additions & 0 deletions src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6765,6 +6765,7 @@ namespace NS2
]]></Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), False)

' trigger completion with import completion disabled
Expand Down Expand Up @@ -6833,6 +6834,7 @@ namespace NS2
]]></Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

' trigger completion with import completion enabled
Expand Down Expand Up @@ -8247,6 +8249,7 @@ namespace B
}
} </Document>)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

state.SendInvokeCompletionList()
Expand Down Expand Up @@ -8566,6 +8569,8 @@ public class AA
}
}</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

Dim expectedText = $"
Expand Down Expand Up @@ -8612,6 +8617,7 @@ public class AA
}
}</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)
state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

state.SendInvokeCompletionList()
Expand Down Expand Up @@ -8666,6 +8672,7 @@ public class AA
}</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(
New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

Expand Down Expand Up @@ -8715,6 +8722,7 @@ public class AA
}</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(
New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

Expand Down Expand Up @@ -8758,6 +8766,7 @@ public class AA
}</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(
New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

Expand Down Expand Up @@ -8808,6 +8817,7 @@ namespace Bar1
}</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(
New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

Expand Down Expand Up @@ -8858,6 +8868,7 @@ public unsafe class AA
}</Document>,
showCompletionInArgumentLists:=showCompletionInArgumentLists)

state.Workspace.GlobalOptions.SetGlobalOption(New OptionKey(CompletionOptionsStorage.ForceExpandedCompletionIndexCreation), True)
state.Workspace.GlobalOptions.SetGlobalOption(
New OptionKey(CompletionOptionsStorage.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp), True)

Expand Down
5 changes: 4 additions & 1 deletion ...ion/Providers/ImportCompletionProvider/AbstractExtensionMethodImportCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ protected override bool ShouldProvideCompletion(CompletionContext completionCont
protected override void LogCommit()
=> CompletionProvidersLogger.LogCommitOfExtensionMethodImportCompletionItem();

protected override Task WarmUpCacheAsync(Document document)
=> ExtensionMethodImportCompletionHelper.WarmUpCacheAsync(document, CancellationToken.None);

protected override async Task AddCompletionItemsAsync(
CompletionContext completionContext,
SyntaxContext syntaxContext,
Expand All @@ -49,7 +52,7 @@ protected override async Task AddCompletionItemsAsync(
receiverTypeSymbol,
namespaceInScope,
inferredTypes,
forceIndexCreation: completionContext.CompletionOptions.ForceExpandedCompletionIndexCreation,
forceCacheCreation: completionContext.CompletionOptions.ForceExpandedCompletionIndexCreation,
hideAdvancedMembers: completionContext.CompletionOptions.HideAdvancedMembers,
cancellationToken).ConfigureAwait(false);

Expand Down
4 changes: 4 additions & 0 deletions ...ortable/Completion/Providers/ImportCompletionProvider/AbstractImportCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ internal abstract class AbstractImportCompletionProvider : LSPCompletionProvider
{
protected abstract Task<ImmutableArray<string>> GetImportedNamespacesAsync(SyntaxContext syntaxContext, CancellationToken cancellationToken);
protected abstract bool ShouldProvideCompletion(CompletionContext completionContext, SyntaxContext syntaxContext);
protected abstract Task WarmUpCacheAsync(Document document);
protected abstract Task AddCompletionItemsAsync(CompletionContext completionContext, SyntaxContext syntaxContext, HashSet<string> namespacesInScope, CancellationToken cancellationToken);
protected abstract bool IsFinalSemicolonOfUsingOrExtern(SyntaxNode directive, SyntaxToken token);
protected abstract Task<bool> ShouldProvideParenthesisCompletionAsync(Document document, CompletionItem item, char? commitKey, CancellationToken cancellationToken);
Expand All @@ -47,6 +48,9 @@ public override async Task ProvideCompletionsAsync(CompletionContext completionC
var syntaxContext = await CreateContextAsync(document, completionContext.Position, cancellationToken).ConfigureAwait(false);
if (!ShouldProvideCompletion(completionContext, syntaxContext))
{
// Trigger a backgound task to warm up cache and return immediately.
// This won't cause multiple cache update to run simultaneously because WarmUpCacheAsync is implemented by AsyncBatchingWorkQueue.
_ = Task.Run(() => WarmUpCacheAsync(document));
genlu marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn't need the Task.Run. You shoudl just add an item to the work queue.

return;
}

Expand Down
6 changes: 6 additions & 0 deletions ...ble/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ protected override void LogCommit()

protected abstract ImmutableArray<AliasDeclarationTypeNode> GetAliasDeclarationNodes(SyntaxNode node);

protected override Task WarmUpCacheAsync(Document document)
{
var typeImportCompletionService = document.GetRequiredLanguageService<ITypeImportCompletionService>();
return typeImportCompletionService.WarmUpCacheAsync(document.Project, CancellationToken.None);
}

protected override async Task AddCompletionItemsAsync(CompletionContext completionContext, SyntaxContext syntaxContext, HashSet<string> namespacesInScope, CancellationToken cancellationToken)
{
using (Logger.LogBlock(FunctionId.Completion_TypeImportCompletionProvider_GetCompletionItemsAsync, cancellationToken))
Expand Down
15 changes: 13 additions & 2 deletions ...tion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionService.CacheEntry.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ internal abstract partial class AbstractTypeImportCompletionService
{
private readonly struct CacheEntry
{
public SymbolKey AssemblySymbolKey { get; }

public string Language { get; }

public Checksum Checksum { get; }
Expand All @@ -31,11 +33,13 @@ private readonly struct CacheEntry
private int PublicItemCount { get; }

private CacheEntry(
SymbolKey assemblySymbolKey,
Checksum checksum,
string language,
ImmutableArray<TypeImportCompletionItemInfo> items,
int publicItemCount)
{
AssemblySymbolKey = assemblySymbolKey;
Checksum = checksum;
Language = language;

Expand All @@ -44,14 +48,18 @@ private CacheEntry(
}

public ImmutableArray<CompletionItem> GetItemsForContext(
Compilation originCompilation,
string language,
string genericTypeSuffix,
bool isInternalsVisible,
bool isAttributeContext,
bool isCaseSensitive,
bool hideAdvancedMembers)
{
if (AssemblySymbolKey.Resolve(originCompilation).Symbol is not IAssemblySymbol assemblySymbol)
return ImmutableArray<CompletionItem>.Empty;

var isSameLanguage = Language == language;
var isInternalsVisible = originCompilation.Assembly.IsSameAssemblyOrHasFriendAccessTo(assemblySymbol);
using var _ = ArrayBuilder<CompletionItem>.GetInstance(out var builder);

// PERF: try set the capacity upfront to avoid allocation from Resize
Expand Down Expand Up @@ -122,6 +130,7 @@ static CompletionItem GetAppropriateAttributeItem(CompletionItem attributeItem,

public class Builder : IDisposable
{
private readonly SymbolKey _assemblySymbolKey;
private readonly string _language;
private readonly string _genericTypeSuffix;
private readonly Checksum _checksum;
Expand All @@ -131,8 +140,9 @@ public class Builder : IDisposable

private readonly ArrayBuilder<TypeImportCompletionItemInfo> _itemsBuilder;

public Builder(Checksum checksum, string language, string genericTypeSuffix, EditorBrowsableInfo editorBrowsableInfo)
public Builder(SymbolKey assemblySymbolKey, Checksum checksum, string language, string genericTypeSuffix, EditorBrowsableInfo editorBrowsableInfo)
{
_assemblySymbolKey = assemblySymbolKey;
_checksum = checksum;
_language = language;
_genericTypeSuffix = genericTypeSuffix;
Expand All @@ -144,6 +154,7 @@ public Builder(Checksum checksum, string language, string genericTypeSuffix, Edi
public CacheEntry ToReferenceCacheEntry()
{
return new CacheEntry(
_assemblySymbolKey,
_checksum,
_language,
_itemsBuilder.ToImmutable(),
Expand Down
Loading