From bf46f94387c3060b5fb5921c05815731428564c7 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Tue, 7 Apr 2020 15:17:52 -0700 Subject: [PATCH 1/6] SignatureHelp cancellation & some caching in CommandHelpers --- .../Utilities/CommandHelpers.cs | 56 +++++++++++++++---- .../TextDocument/Handlers/HoverHandler.cs | 5 +- .../Handlers/SignatureHelpHandler.cs | 43 +++++++------- 3 files changed, 69 insertions(+), 35 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs index 7e09735d6..48ddc96b2 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs @@ -16,20 +16,26 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShellContext /// internal static class CommandHelpers { - private static readonly ConcurrentDictionary NounExclusionList = + private static readonly ConcurrentDictionary s_nounExclusionList = new ConcurrentDictionary(); + private static readonly ConcurrentDictionary s_commandInfoCache = + new ConcurrentDictionary(); + + private static readonly ConcurrentDictionary s_synopsisCache = + new ConcurrentDictionary(); + 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); + s_nounExclusionList.TryAdd("Module", true); + s_nounExclusionList.TryAdd("Script", true); + s_nounExclusionList.TryAdd("Package", true); + s_nounExclusionList.TryAdd("PackageProvider", true); + s_nounExclusionList.TryAdd("PackageSource", true); + s_nounExclusionList.TryAdd("InstalledModule", true); + s_nounExclusionList.TryAdd("InstalledScript", true); + s_nounExclusionList.TryAdd("ScriptFileInfo", true); + s_nounExclusionList.TryAdd("PSRepository", true); } /// @@ -45,12 +51,18 @@ public static async Task GetCommandInfoAsync( Validate.IsNotNull(nameof(commandName), commandName); Validate.IsNotNull(nameof(powerShellContext), powerShellContext); + // If we have a CommandInfo cached, return that. + if (s_commandInfoCache.TryGetValue(commandName, out CommandInfo cmdInfo)) + { + return cmdInfo; + } + // 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 // a major slowdown in IntelliSense. var commandParts = commandName.Split('-'); - if (commandParts.Length == 2 && NounExclusionList.ContainsKey(commandParts[1])) + if (commandParts.Length == 2 && s_nounExclusionList.ContainsKey(commandParts[1])) { return null; } @@ -60,10 +72,18 @@ public static async Task GetCommandInfoAsync( command.AddArgument(commandName); command.AddParameter("ErrorAction", "Ignore"); - return (await powerShellContext.ExecuteCommandAsync(command, sendOutputToHost: false, sendErrorToHost: false).ConfigureAwait(false)) + CommandInfo commandInfo = (await powerShellContext.ExecuteCommandAsync(command, sendOutputToHost: false, sendErrorToHost: false).ConfigureAwait(false)) .Select(o => o.BaseObject) .OfType() .FirstOrDefault(); + + // Only cache CmdletInfos since they're exposed in binaries and can never change throughout the session. + if (commandInfo.CommandType == CommandTypes.Cmdlet) + { + s_commandInfoCache.TryAdd(commandName, commandInfo); + } + + return commandInfo; } /// @@ -87,6 +107,12 @@ public static async Task GetCommandSynopsisAsync( return string.Empty; } + // If we have a synopsis cached, return that. + 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 +128,12 @@ public static async Task 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)) { diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/HoverHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/HoverHandler.cs index e6094a73e..1c01ace54 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/HoverHandler.cs +++ b/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(); _symbolsService = symbolsService; _workspaceService = workspaceService; - _powerShellContextService = powerShellContextService; } public HoverRegistrationOptions GetRegistrationOptions() diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs index f21ca09b0..47d23b1d7 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs @@ -20,7 +20,6 @@ namespace Microsoft.PowerShell.EditorServices.Handlers { internal class SignatureHelpHandler : ISignatureHelpHandler { - private static readonly SignatureInformation[] s_emptySignatureResult = Array.Empty(); private readonly ILogger _logger; private readonly SymbolsService _symbolsService; private readonly WorkspaceService _workspaceService; @@ -52,6 +51,12 @@ public SignatureHelpRegistrationOptions GetRegistrationOptions() public async Task 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 +66,28 @@ await _symbolsService.FindParameterSetsInFileAsync( (int) request.Position.Character + 1, _powerShellContextService).ConfigureAwait(false); - SignatureInformation[] signatures = s_emptySignatureResult; + if (parameterSets == null) + { + return new SignatureHelp(); + } - if (parameterSets != null) + SignatureInformation[] 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 ParameterInformation[parameterSets.Signatures[i].Parameters.Count()]; + int j = 0; + 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[j] = CreateParameterInfo(param); + j++; } + + signatures[i] = new SignatureInformation + { + Label = parameterSets.CommandName + " " + parameterSets.Signatures[i].SignatureText, + Documentation = null, + Parameters = parameters, + }; } return new SignatureHelp From 19fca4c155858bb606f75e0ba10f3c7fb2f5a80c Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Tue, 7 Apr 2020 15:19:46 -0700 Subject: [PATCH 2/6] add comment --- .../Services/PowerShellContext/Utilities/CommandHelpers.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs index 48ddc96b2..ebc98d57f 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs @@ -108,6 +108,9 @@ public static async Task GetCommandSynopsisAsync( } // 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; From c4305549b3982d473a17a4caf0b5783d55d76275 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Tue, 7 Apr 2020 15:22:15 -0700 Subject: [PATCH 3/6] better language --- .../Services/PowerShellContext/Utilities/CommandHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs index ebc98d57f..8881a2033 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs @@ -57,7 +57,7 @@ public static async Task GetCommandInfoAsync( return cmdInfo; } - // Make sure the command's noun isn't blacklisted. This is + // Make sure the command's noun isn't in the exclusion list. This is // currently necessary to make sure that Get-Command doesn't // load PackageManagement or PowerShellGet because they cause // a major slowdown in IntelliSense. From 45d9e0886e773ba43d6da6926124c81792e60459 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Tue, 7 Apr 2020 15:23:06 -0700 Subject: [PATCH 4/6] fixed wording --- .../Services/PowerShellContext/Utilities/CommandHelpers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs index 8881a2033..332549707 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs @@ -77,7 +77,7 @@ public static async Task GetCommandInfoAsync( .OfType() .FirstOrDefault(); - // Only cache CmdletInfos since they're exposed in binaries and can never change throughout the session. + // 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); From bdcfcdcddbe286f372632489718d8c2b8cc68bc7 Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 8 Apr 2020 09:29:12 -0700 Subject: [PATCH 5/6] address feedback from Keith and Patrick --- .../Utilities/CommandHelpers.cs | 38 ++++++++++++++----- .../Handlers/SignatureHelpHandler.cs | 11 ++---- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs index 332549707..604f542c9 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs +++ b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs @@ -19,6 +19,10 @@ internal static class CommandHelpers private static readonly ConcurrentDictionary s_nounExclusionList = new ConcurrentDictionary(); + // 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 ConcurrentDictionary s_cmdletExclusionList = + new ConcurrentDictionary(); + private static readonly ConcurrentDictionary s_commandInfoCache = new ConcurrentDictionary(); @@ -27,15 +31,30 @@ internal static class CommandHelpers static CommandHelpers() { - s_nounExclusionList.TryAdd("Module", true); + // PowerShellGet v2 nouns + s_nounExclusionList.TryAdd("CredsFromCredentialProvider", true); + s_nounExclusionList.TryAdd("DscResource", true); + s_nounExclusionList.TryAdd("InstalledModule", true); + s_nounExclusionList.TryAdd("InstalledScript", true); + s_nounExclusionList.TryAdd("PSRepository", true); + s_nounExclusionList.TryAdd("RoleCapability", true); s_nounExclusionList.TryAdd("Script", true); + s_nounExclusionList.TryAdd("ScriptFileInfo", true); + + // PackageManagement nouns s_nounExclusionList.TryAdd("Package", true); s_nounExclusionList.TryAdd("PackageProvider", true); s_nounExclusionList.TryAdd("PackageSource", true); - s_nounExclusionList.TryAdd("InstalledModule", true); - s_nounExclusionList.TryAdd("InstalledScript", true); - s_nounExclusionList.TryAdd("ScriptFileInfo", true); - s_nounExclusionList.TryAdd("PSRepository", true); + + // Cmdlet's in PowerShellGet with conflicting nouns + s_cmdletExclusionList.TryAdd("Find-Command", true); + s_cmdletExclusionList.TryAdd("Find-Module", true); + s_cmdletExclusionList.TryAdd("Install-Module", true); + s_cmdletExclusionList.TryAdd("Publish-Module", true); + s_cmdletExclusionList.TryAdd("Save-Module", true); + s_cmdletExclusionList.TryAdd("Uninstall-Module", true); + s_cmdletExclusionList.TryAdd("Update-Module", true); + s_cmdletExclusionList.TryAdd("Update-ModuleManifest", true); } /// @@ -57,12 +76,13 @@ public static async Task GetCommandInfoAsync( return cmdInfo; } - // Make sure the command's noun isn't in the exclusion list. This is - // currently necessary to make sure that Get-Command doesn't - // load PackageManagement or PowerShellGet because they cause + // 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 && s_nounExclusionList.ContainsKey(commandParts[1])) + if ((commandParts.Length == 2 && s_nounExclusionList.ContainsKey(commandParts[1])) + || s_cmdletExclusionList.ContainsKey(commandName)) { return null; } diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs index 47d23b1d7..8b106a2f5 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/SignatureHelpHandler.cs +++ b/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; @@ -71,15 +70,13 @@ await _symbolsService.FindParameterSetsInFileAsync( return new SignatureHelp(); } - SignatureInformation[] signatures = new SignatureInformation[parameterSets.Signatures.Length]; + var signatures = new SignatureInformation[parameterSets.Signatures.Length]; for (int i = 0; i < signatures.Length; i++) { - var parameters = new ParameterInformation[parameterSets.Signatures[i].Parameters.Count()]; - int j = 0; + var parameters = new List(); foreach (ParameterInfo param in parameterSets.Signatures[i].Parameters) { - parameters[j] = CreateParameterInfo(param); - j++; + parameters.Add(CreateParameterInfo(param)); } signatures[i] = new SignatureInformation From ff09fe00cf28fbcdb7208f2be5e90d5e9f67540c Mon Sep 17 00:00:00 2001 From: Tyler Leonhardt Date: Wed, 8 Apr 2020 09:59:51 -0700 Subject: [PATCH 6/6] move to hashset, remove static ctr --- .../Utilities/CommandHelpers.cs | 66 +++++++++---------- 1 file changed, 32 insertions(+), 34 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs index 604f542c9..598e44f29 100644 --- a/src/PowerShellEditorServices/Services/PowerShellContext/Utilities/CommandHelpers.cs +++ b/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,12 +17,37 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShellContext /// internal static class CommandHelpers { - private static readonly ConcurrentDictionary s_nounExclusionList = - new ConcurrentDictionary(); + private static readonly HashSet s_nounExclusionList = new HashSet + { + // PowerShellGet v2 nouns + "CredsFromCredentialProvider", + "DscResource", + "InstalledModule", + "InstalledScript", + "PSRepository", + "RoleCapability", + "Script", + "ScriptFileInfo", + + // 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 ConcurrentDictionary s_cmdletExclusionList = - new ConcurrentDictionary(); + private static readonly HashSet s_cmdletExclusionList = new HashSet + { + // 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 s_commandInfoCache = new ConcurrentDictionary(); @@ -29,34 +55,6 @@ internal static class CommandHelpers private static readonly ConcurrentDictionary s_synopsisCache = new ConcurrentDictionary(); - static CommandHelpers() - { - // PowerShellGet v2 nouns - s_nounExclusionList.TryAdd("CredsFromCredentialProvider", true); - s_nounExclusionList.TryAdd("DscResource", true); - s_nounExclusionList.TryAdd("InstalledModule", true); - s_nounExclusionList.TryAdd("InstalledScript", true); - s_nounExclusionList.TryAdd("PSRepository", true); - s_nounExclusionList.TryAdd("RoleCapability", true); - s_nounExclusionList.TryAdd("Script", true); - s_nounExclusionList.TryAdd("ScriptFileInfo", true); - - // PackageManagement nouns - s_nounExclusionList.TryAdd("Package", true); - s_nounExclusionList.TryAdd("PackageProvider", true); - s_nounExclusionList.TryAdd("PackageSource", true); - - // Cmdlet's in PowerShellGet with conflicting nouns - s_cmdletExclusionList.TryAdd("Find-Command", true); - s_cmdletExclusionList.TryAdd("Find-Module", true); - s_cmdletExclusionList.TryAdd("Install-Module", true); - s_cmdletExclusionList.TryAdd("Publish-Module", true); - s_cmdletExclusionList.TryAdd("Save-Module", true); - s_cmdletExclusionList.TryAdd("Uninstall-Module", true); - s_cmdletExclusionList.TryAdd("Update-Module", true); - s_cmdletExclusionList.TryAdd("Update-ModuleManifest", true); - } - /// /// Gets the CommandInfo instance for a command with a particular name. /// @@ -81,8 +79,8 @@ public static async Task GetCommandInfoAsync( // load PackageManagement or PowerShellGet v2 because they cause // a major slowdown in IntelliSense. var commandParts = commandName.Split('-'); - if ((commandParts.Length == 2 && s_nounExclusionList.ContainsKey(commandParts[1])) - || s_cmdletExclusionList.ContainsKey(commandName)) + if ((commandParts.Length == 2 && s_nounExclusionList.Contains(commandParts[1])) + || s_cmdletExclusionList.Contains(commandName)) { return null; }