Some improvement for import completion

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/ExtensionMethodImportCompletionProviderTests.cs

@@ -26,16 +26,20 @@ public class ExtensionMethodImportCompletionProviderTests : AbstractCSharpComple
 
         private bool? ShowImportCompletionItemsOptionValue { get; set; } = true;
 
+        // -1 would disable timebox, whereas 0 means always timeout.
+        private int TimeoutInMilliseconds { get; set; } = -1;
+
         private bool IsExpandedCompletion { get; set; } = true;
 
-        private bool HideAdvancedMembers { get; set; } = false;
+        private bool HideAdvancedMembers { get; set; }
 
         protected override OptionSet WithChangedOptions(OptionSet options)
         {
             return options
                 .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, ShowImportCompletionItemsOptionValue)
                 .WithChangedOption(CompletionServiceOptions.IsExpandedCompletion, IsExpandedCompletion)
-                .WithChangedOption(CompletionOptions.HideAdvancedMembers, LanguageNames.CSharp, HideAdvancedMembers);
+                .WithChangedOption(CompletionOptions.HideAdvancedMembers, LanguageNames.CSharp, HideAdvancedMembers)
+                .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, TimeoutInMilliseconds);
         }
 
         protected override TestComposition GetComposition()
@@ -1884,6 +1888,44 @@ await VerifyImportItemIsAbsentAsync(
             }
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        public async Task TestTimeBox()
+        {
+            var file1 = @"
+using System;
+
+namespace Foo
+{
+    public static class ExtensionClass
+    {
+        public static bool ExtentionMethod(this int x)
+            => true;
+    }
+}";
+            var file2 = @"
+using System;
+
+namespace Baz
+{
+    public class Bat
+    {
+        public void M(int x)
+        {
+            x.$$
+        }
+    }
+}";
+
+            IsExpandedCompletion = false;
+            TimeoutInMilliseconds = 0; //timeout immediately
+            var markup = GetMarkup(file2, file1, ReferenceType.None);
+
+            await VerifyImportItemIsAbsentAsync(
+                 markup,
+                 "ExtentionMethod",
+                 inlineDescription: "Foo");
+        }
+
         private Task VerifyImportItemExistsAsync(string markup, string expectedItem, int glyph, string inlineDescription, string displayTextSuffix = null, string expectedDescriptionOrNull = null)
             => VerifyItemExistsAsync(markup, expectedItem, displayTextSuffix: displayTextSuffix, glyph: glyph, inlineDescription: inlineDescription, expectedDescriptionOrNull: expectedDescriptionOrNull);
 

src/EditorFeatures/Core/Implementation/IntelliSense/AsyncCompletion/ItemManager.cs

@@ -5,15 +5,13 @@
 #nullable disable
 
 using System;
-using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Globalization;
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Completion;
-using Microsoft.CodeAnalysis.Completion.Providers;
 using Microsoft.CodeAnalysis.PatternMatching;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
@@ -176,8 +174,23 @@ private FilteredCompletionModel UpdateCompletionList(
 
             // Use a monotonically increasing integer to keep track the original alphabetical order of each item.
             var currentIndex = 0;
-            var builder = ArrayBuilder<MatchResult>.GetInstance();
 
+            using var _ = ArrayBuilder<MatchResult>.GetInstance(out var builder);
+            if (KeepAllItemsInTheList(initialRoslynTriggerKind, filterText) && !needToFilterExpanded && !needToFilter)
+            {
+                // PERF: Conditionally set the capacity of the ArrayBuilder, i.e. if we think it's likely that the filtering result
+                // would be similar in size to the initial list, in which case, setting the capacity would avoid calls to Resize later.
+                // Otherwise, don't set capacity as an attempt to avoid allocating large array which might unnecessarily end up in LOH.
+                // The trade-off of not setting capacity is when we are wrong (large amount of items post-filtering), more allocation
+                // would occur because of Resize, but in reality we believe such cost is much lower than the benefit of not allocating
+                // large array upfront everytime we refresh completion list.
+                // We say it's likely that we end up with a list of similar size after filtering if:
+                // 1. We need to show items even if they don't match filter text
+                // 2. No filter is selected
+                builder.EnsureCapacity(data.InitialSortedList.Length);
+            }
+
+            // Filter items based on the selected filters and matching.
             foreach (var item in data.InitialSortedList)
             {
                 cancellationToken.ThrowIfCancellationRequested();
@@ -218,7 +231,7 @@ private FilteredCompletionModel UpdateCompletionList(
             // to `MatchResult` to achieve this.
             builder.Sort(MatchResult.SortingComparer);
 
-            var initialListOfItemsToBeIncluded = builder.ToImmutableAndFree();
+            var initialListOfItemsToBeIncluded = builder.ToImmutable();
 
             var showCompletionItemFilters = options?.GetOption(CompletionOptions.ShowCompletionItemFilters, document.Project.Language) ?? true;
 
@@ -570,6 +583,21 @@ private static RoslynCompletionItem GetOrAddRoslynCompletionItem(VSCompletionIte
             return roslynItem;
         }
 
+        // If the item didn't match the filter text, we still keep it in the list
+        // if one of two things is true:
+        //  1. The user has typed nothing or only typed a single character.  In this case they might
+        //     have just typed the character to get completion.  Filtering out items
+        //     here is not desirable.
+        //
+        //  2. They brought up completion with ctrl-j or through deletion.  In these
+        //     cases we just always keep all the items in the list.
+        private static bool KeepAllItemsInTheList(CompletionTriggerKind initialTriggerKind, string filterText)
+        {
+            return filterText.Length <= 1 ||
+                initialTriggerKind == CompletionTriggerKind.Invoke ||
+                initialTriggerKind == CompletionTriggerKind.Deletion;
+        }
+
         private static bool TryCreateMatchResult(
             CompletionHelper completionHelper,
             VSCompletionItem item,
@@ -601,31 +629,25 @@ private static bool TryCreateMatchResult(
                 recentItems,
                 patternMatch);
 
-            // If the item didn't match the filter text, we still keep it in the list
-            // if one of two things is true:
-            //
-            //  1. The user has typed nothing or only typed a single character.  In this case they might
-            //     have just typed the character to get completion.  Filtering out items
-            //     here is not desirable.
-            //
-            //  2. They brought up completion with ctrl-j or through deletion.  In these
-            //     cases we just always keep all the items in the list.
-            if (matchedFilterText ||
-                initialTriggerKind == CompletionTriggerKind.Deletion ||
-                initialTriggerKind == CompletionTriggerKind.Invoke ||
-                filterText.Length <= 1)
+            if (matchedFilterText || KeepAllItemsInTheList(initialTriggerKind, filterText))
             {
                 matchResult = new MatchResult(
                     roslynItem, item, matchedFilterText: matchedFilterText,
-                    patternMatch: patternMatch, index: currentIndex++, GetHighlightedSpans());
+                    patternMatch: patternMatch, index: currentIndex++, GetHighlightedSpans(completionHelper, item, filterText, highlightMatchingPortions, roslynItem, patternMatch));
 
                 return true;
             }
 
             matchResult = default;
             return false;
 
-            ImmutableArray<Span> GetHighlightedSpans()
+            static ImmutableArray<Span> GetHighlightedSpans(
+                CompletionHelper completionHelper,
+                VSCompletionItem item,
+                string filterText,
+                bool highlightMatchingPortions,
+                RoslynCompletionItem roslynItem,
+                PatternMatch? patternMatch)
             {
                 if (!highlightMatchingPortions)
                 {
@@ -647,7 +669,7 @@ ImmutableArray<Span> GetHighlightedSpans()
                     // Since VS item's display text is created as Prefix + DisplayText + Suffix, 
                     // we can calculate the highlighted span by adding an offset that is the length of the Prefix.
                     return patternMatch.Value.MatchedSpans
-                        .SelectAsArray(s => s.MoveTo(roslynItem.DisplayTextPrefix?.Length ?? 0).ToSpan());
+                        .SelectAsArray(s_highlightSpanGetter, roslynItem);
                 }
 
                 // If there's no match for Roslyn item's filter text which is identical to its display text,
@@ -656,6 +678,10 @@ ImmutableArray<Span> GetHighlightedSpans()
             }
         }
 
+        // PERF: Create a singleton to avoid lambda allocation on hot path
+        private static readonly Func<TextSpan, RoslynCompletionItem, Span> s_highlightSpanGetter
+            = (span, item) => span.MoveTo(item.DisplayTextPrefix?.Length ?? 0).ToSpan();
+
         private static bool MatchesFilterText(
             RoslynCompletionItem item,
             string filterText,

src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb

@@ -6090,6 +6090,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, False)))
 
                 ' trigger completion with import completion disabled
@@ -6160,6 +6161,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 ' trigger completion with import completion enabled
@@ -6203,6 +6205,7 @@ namespace NS1
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 ' trigger completion with import completion enabled
@@ -6638,6 +6641,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6712,6 +6716,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6786,6 +6791,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6860,6 +6866,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6935,6 +6942,7 @@ namespace NS2
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -6980,6 +6988,7 @@ namespace OtherNS
 
                 Dim workspace = state.Workspace
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendInvokeCompletionList()
@@ -7099,6 +7108,7 @@ namespace NS2
                 Await state.AssertNoCompletionSession()
 
                 workspace.TryApplyChanges(workspace.CurrentSolution.WithOptions(workspace.Options _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, -1) _
                     .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)))
 
                 state.SendTypeChars("mytask")

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/ExtensionMethodImportCompletionProviderTests.vb

@@ -14,14 +14,16 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Completion.Complet
 
         Private Property IsExpandedCompletion As Boolean = True
 
-        Private Property ShowImportCompletionItemsOptionValue As Boolean = True
-
         ' -1 would disable timebox, whereas 0 means always timeout.
         Private Property TimeoutInMilliseconds As Integer = -1
 
+        Private Property ShowImportCompletionItemsOptionValue As Boolean = True
+
         Protected Overrides Function WithChangedOptions(options As OptionSet) As OptionSet
             Return options _
-                .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.VisualBasic, ShowImportCompletionItemsOptionValue).WithChangedOption(CompletionServiceOptions.IsExpandedCompletion, IsExpandedCompletion)
+                .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.VisualBasic, ShowImportCompletionItemsOptionValue) _
+                .WithChangedOption(CompletionServiceOptions.IsExpandedCompletion, IsExpandedCompletion) _
+                .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForExtensionMethodImportCompletion, TimeoutInMilliseconds)
         End Function
 
         Protected Overrides Function GetComposition() As TestComposition

src/Features/Core/Portable/Completion/CompletionServiceOptions.cs

@@ -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 Microsoft.CodeAnalysis.Options;
 
 namespace Microsoft.CodeAnalysis.Completion
@@ -21,5 +19,12 @@ public static readonly Option2<bool> IsExpandedCompletion
         /// </summary>
         public static readonly Option2<bool> DisallowAddingImports
             = new(nameof(CompletionServiceOptions), nameof(DisallowAddingImports), defaultValue: false);
+
+        /// <summary>
+        /// Timeout value used for time-boxing completion of unimported extension methods.
+        /// Value less than 0 means no timebox; value == 0 means immediate timeout (for testing purpose)
+        /// </summary>
+        public static readonly Option2<int> TimeoutInMillisecondsForExtensionMethodImportCompletion
+            = new(nameof(CompletionServiceOptions), nameof(TimeoutInMillisecondsForExtensionMethodImportCompletion), defaultValue: 500);
     }
 }