Signaturehelp cancellation and some caching (#1251)

* SignatureHelp cancellation & some caching in CommandHelpers

* add comment

* better language

* fixed wording

* address feedback from Keith and Patrick

* move to hashset, remove static ctr

Author: TylerLeonhardt

src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs

@@ -4,6 +4,7 @@
 //
 
 using System.Collections.Concurrent;
+using System.Collections.Generic;
 using System.Linq;
 using System.Management.Automation;
 using System.Threading.Tasks;
@@ -16,21 +17,43 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShellContext
     /// </summary>
     internal static class CommandHelpers
     {
-        private static readonly ConcurrentDictionary<string, bool> NounExclusionList =
-            new ConcurrentDictionary<string, bool>();
+        private static readonly HashSet<string> s_nounExclusionList = new HashSet<string>
+            {
+                // PowerShellGet v2 nouns
+                "CredsFromCredentialProvider",
+                "DscResource",
+                "InstalledModule",
+                "InstalledScript",
+                "PSRepository",
+                "RoleCapability",
+                "Script",
+                "ScriptFileInfo",
 
-        static CommandHelpers()
-        {
-            NounExclusionList.TryAdd("Module", true);
-            NounExclusionList.TryAdd("Script", true);
-            NounExclusionList.TryAdd("Package", true);
-            NounExclusionList.TryAdd("PackageProvider", true);
-            NounExclusionList.TryAdd("PackageSource", true);
-            NounExclusionList.TryAdd("InstalledModule", true);
-            NounExclusionList.TryAdd("InstalledScript", true);
-            NounExclusionList.TryAdd("ScriptFileInfo", true);
-            NounExclusionList.TryAdd("PSRepository", true);
-        }
+                // PackageManagement nouns
+                "Package",
+                "PackageProvider",
+                "PackageSource",
+            };
+
+        // This is used when a noun exists in multiple modules (for example, "Command" is used in Microsoft.PowerShell.Core and also PowerShellGet)
+        private static readonly HashSet<string> s_cmdletExclusionList = new HashSet<string>
+            {
+                // Commands in PowerShellGet with conflicting nouns
+                "Find-Command",
+                "Find-Module",
+                "Install-Module",
+                "Publish-Module",
+                "Save-Module",
+                "Uninstall-Module",
+                "Update-Module",
+                "Update-ModuleManifest",
+            };
+
+        private static readonly ConcurrentDictionary<string, CommandInfo> s_commandInfoCache =
+            new ConcurrentDictionary<string, CommandInfo>();
+
+        private static readonly ConcurrentDictionary<string, string> s_synopsisCache =
+            new ConcurrentDictionary<string, string>();
 
         /// <summary>
         /// Gets the CommandInfo instance for a command with a particular name.
@@ -45,12 +68,19 @@ public static async Task<CommandInfo> GetCommandInfoAsync(
             Validate.IsNotNull(nameof(commandName), commandName);
             Validate.IsNotNull(nameof(powerShellContext), powerShellContext);
 
-            // Make sure the command's noun isn't blacklisted.  This is
-            // currently necessary to make sure that Get-Command doesn't
-            // load PackageManagement or PowerShellGet because they cause
+            // If we have a CommandInfo cached, return that.
+            if (s_commandInfoCache.TryGetValue(commandName, out CommandInfo cmdInfo))
+            {
+                return cmdInfo;
+            }
+
+            // Make sure the command's noun or command's name isn't in the exclusion lists.
+            // This is currently necessary to make sure that Get-Command doesn't
+            // load PackageManagement or PowerShellGet v2 because they cause
             // a major slowdown in IntelliSense.
             var commandParts = commandName.Split('-');
-            if (commandParts.Length == 2 && NounExclusionList.ContainsKey(commandParts[1]))
+            if ((commandParts.Length == 2 && s_nounExclusionList.Contains(commandParts[1]))
+                    || s_cmdletExclusionList.Contains(commandName))
             {
                 return null;
             }
@@ -60,10 +90,18 @@ public static async Task<CommandInfo> GetCommandInfoAsync(
             command.AddArgument(commandName);
             command.AddParameter("ErrorAction", "Ignore");
 
-            return (await powerShellContext.ExecuteCommandAsync<PSObject>(command, sendOutputToHost: false, sendErrorToHost: false).ConfigureAwait(false))
+            CommandInfo commandInfo = (await powerShellContext.ExecuteCommandAsync<PSObject>(command, sendOutputToHost: false, sendErrorToHost: false).ConfigureAwait(false))
                 .Select(o => o.BaseObject)
                 .OfType<CommandInfo>()
                 .FirstOrDefault();
+
+            // Only cache CmdletInfos since they're exposed in binaries they are likely to not change throughout the session.
+            if (commandInfo.CommandType == CommandTypes.Cmdlet)
+            {
+                s_commandInfoCache.TryAdd(commandName, commandInfo);
+            }
+
+            return commandInfo;
         }
 
         /// <summary>
@@ -87,6 +125,15 @@ public static async Task<string> GetCommandSynopsisAsync(
                 return string.Empty;
             }
 
+            // If we have a synopsis cached, return that.
+            // NOTE: If the user runs Update-Help, it's possible that this synopsis will be out of date.
+            // Given the perf increase of doing this, and the simple workaround of restarting the extension,
+            // this seems worth it.
+            if (s_synopsisCache.TryGetValue(commandInfo.Name, out string synopsis))
+            {
+                return synopsis;
+            }
+
             PSCommand command = new PSCommand()
                 .AddCommand(@"Microsoft.PowerShell.Core\Get-Help")
                 // We use .Name here instead of just passing in commandInfo because
@@ -102,6 +149,12 @@ public static async Task<string> GetCommandSynopsisAsync(
                 (string)helpObject?.Properties["synopsis"].Value ??
                 string.Empty;
 
+            // Only cache cmdlet infos because since they're exposed in binaries, the can never change throughout the session.
+            if (commandInfo.CommandType == CommandTypes.Cmdlet)
+            {
+                s_synopsisCache.TryAdd(commandInfo.Name, synopsisString);
+            }
+
             // Ignore the placeholder value for this field
             if (string.Equals(synopsisString, "SHORT DESCRIPTION", System.StringComparison.CurrentCultureIgnoreCase))
             {

src/PowerShellEditorServices/Services/TextDocument/Handlers/HoverHandler.cs

@@ -22,20 +22,17 @@ internal class HoverHandler : IHoverHandler
         private readonly ILogger _logger;
         private readonly SymbolsService _symbolsService;
         private readonly WorkspaceService _workspaceService;
-        private readonly PowerShellContextService _powerShellContextService;
 
         private HoverCapability _capability;
 
         public HoverHandler(
             ILoggerFactory factory,
             SymbolsService symbolsService,
-            WorkspaceService workspaceService,
-            PowerShellContextService powerShellContextService)
+            WorkspaceService workspaceService)
         {
             _logger = factory.CreateLogger<HoverHandler>();
             _symbolsService = symbolsService;
             _workspaceService = workspaceService;
-            _powerShellContextService = powerShellContextService;
         }
 
         public HoverRegistrationOptions GetRegistrationOptions()

src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs

@@ -3,8 +3,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 //
 
-using System;
-using System.Linq;
+using System.Collections.Generic;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
@@ -20,7 +19,6 @@ namespace Microsoft.PowerShell.EditorServices.Handlers
 {
     internal class SignatureHelpHandler : ISignatureHelpHandler
     {
-        private static readonly SignatureInformation[] s_emptySignatureResult = Array.Empty<SignatureInformation>();
         private readonly ILogger _logger;
         private readonly SymbolsService _symbolsService;
         private readonly WorkspaceService _workspaceService;
@@ -52,6 +50,12 @@ public SignatureHelpRegistrationOptions GetRegistrationOptions()
 
         public async Task<SignatureHelp> Handle(SignatureHelpParams request, CancellationToken cancellationToken)
         {
+            if (cancellationToken.IsCancellationRequested)
+            {
+                _logger.LogDebug("SignatureHelp request canceled for file: {0}", request.TextDocument.Uri);
+                return new SignatureHelp();
+            }
+
             ScriptFile scriptFile = _workspaceService.GetFile(request.TextDocument.Uri);
 
             ParameterSetSignatures parameterSets =
@@ -61,28 +65,26 @@ await _symbolsService.FindParameterSetsInFileAsync(
                     (int) request.Position.Character + 1,
                     _powerShellContextService).ConfigureAwait(false);
 
-            SignatureInformation[] signatures = s_emptySignatureResult;
+            if (parameterSets == null)
+            {
+                return new SignatureHelp();
+            }
 
-            if (parameterSets != null)
+            var signatures = new SignatureInformation[parameterSets.Signatures.Length];
+            for (int i = 0; i < signatures.Length; i++)
             {
-                signatures = new SignatureInformation[parameterSets.Signatures.Length];
-                for (int i = 0; i < signatures.Length; i++)
+                var parameters = new List<ParameterInformation>();
+                foreach (ParameterInfo param in parameterSets.Signatures[i].Parameters)
                 {
-                    var parameters = new ParameterInformation[parameterSets.Signatures[i].Parameters.Count()];
-                    int j = 0;
-                    foreach (ParameterInfo param in parameterSets.Signatures[i].Parameters)
-                    {
-                        parameters[j] = CreateParameterInfo(param);
-                        j++;
-                    }
-
-                    signatures[i] = new SignatureInformation
-                    {
-                        Label = parameterSets.CommandName + " " + parameterSets.Signatures[i].SignatureText,
-                        Documentation = null,
-                        Parameters = parameters,
-                    };
+                    parameters.Add(CreateParameterInfo(param));
                 }
+
+                signatures[i] = new SignatureInformation
+                {
+                    Label = parameterSets.CommandName + " " + parameterSets.Signatures[i].SignatureText,
+                    Documentation = null,
+                    Parameters = parameters,
+                };
             }
 
             return new SignatureHelp