Skip to content

Signaturehelp cancellation and some caching #1251

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.

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

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,45 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShellContext
/// </summary>
internal static class CommandHelpers
{
private static readonly ConcurrentDictionary<string, bool> NounExclusionList =
private static readonly ConcurrentDictionary<string, bool> s_nounExclusionList =
new ConcurrentDictionary<string, bool>();

// 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<string, bool> s_cmdletExclusionList =
new ConcurrentDictionary<string, bool>();

private static readonly ConcurrentDictionary<string, CommandInfo> s_commandInfoCache =
new ConcurrentDictionary<string, CommandInfo>();

private static readonly ConcurrentDictionary<string, string> s_synopsisCache =
new ConcurrentDictionary<string, string>();

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);
// 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);
}

/// <summary>
Expand All @@ -45,12 +70,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.ContainsKey(commandParts[1]))
|| s_cmdletExclusionList.ContainsKey(commandName))
{
return null;
}
Expand All @@ -60,10 +92,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>
Expand All @@ -87,6 +127,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
Expand All @@ -102,6 +151,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))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Expand All @@ -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];
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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
Expand Down