From a1f94f57afa113a85ca174ea2c82a18b6efc535b Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 5 Mar 2019 21:43:31 -0800 Subject: [PATCH 1/3] Redo command info cache changes --- Engine/CommandInfoCache.cs | 119 +++++++++++++++++++++++++++++++++++++ Engine/Helper.cs | 62 ++++--------------- 2 files changed, 131 insertions(+), 50 deletions(-) create mode 100644 Engine/CommandInfoCache.cs diff --git a/Engine/CommandInfoCache.cs b/Engine/CommandInfoCache.cs new file mode 100644 index 000000000..5af7316f9 --- /dev/null +++ b/Engine/CommandInfoCache.cs @@ -0,0 +1,119 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Concurrent; +using System.Management.Automation; +using System.Linq; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer +{ + /// + /// Provides threadsafe caching around CommandInfo lookups with `Get-Command -Name ...`. + /// + internal class CommandInfoCache + { + private readonly ConcurrentDictionary> _commandInfoCache; + + private readonly Helper _helperInstance; + + /// + /// Create a fresh command info cache instance. + /// + public CommandInfoCache(Helper pssaHelperInstance) + { + _commandInfoCache = new ConcurrentDictionary>(); + _helperInstance = pssaHelperInstance; + } + + /// + /// Retrieve a command info object about a command. + /// + /// Name of the command to get a commandinfo object for. + /// The alias of the command to be used in the cache key. If not given, uses the command name. + /// What types of command are needed. If omitted, all types are retrieved. + /// + public CommandInfo GetCommandInfo(string commandName, string aliasName = null, CommandTypes? commandTypes = null) + { + if (string.IsNullOrWhiteSpace(commandName)) + { + return null; + } + + // If alias name is given, we store the entry under that, but search with the command name + var key = new CommandLookupKey(aliasName ?? commandName, commandTypes); + + // Atomically either use PowerShell to query a command info object, or fetch it from the catch + return _commandInfoCache.GetOrAdd(key, new Lazy(() => GetCommandInfoInternal(commandName, commandTypes))).Value; + } + + /// + /// Retrieve a command info object about a command. + /// + /// Name of the command to get a commandinfo object for. + /// What types of command are needed. If omitted, all types are retrieved. + /// + [Obsolete("Alias lookup is expensive and should not be relied upon for command lookup")] + public CommandInfo GetCommandInfoLegacy(string commandOrAliasName, CommandTypes? commandTypes = null) + { + string commandName = _helperInstance.GetCmdletNameFromAlias(commandOrAliasName); + + return string.IsNullOrEmpty(commandName) + ? GetCommandInfo(commandOrAliasName, commandTypes: commandTypes) + : GetCommandInfo(commandName, aliasName: commandOrAliasName, commandTypes: commandTypes); + } + + /// + /// Get a CommandInfo object of the given command name + /// + /// Returns null if command does not exists + private static CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType) + { + using (var ps = System.Management.Automation.PowerShell.Create()) + { + ps.AddCommand("Get-Command") + .AddParameter("Name", cmdName) + .AddParameter("ErrorAction", "SilentlyContinue"); + + if(commandType != null) + { + ps.AddParameter("CommandType", commandType); + } + + return ps.Invoke() + .FirstOrDefault(); + } + } + + private struct CommandLookupKey : IEquatable + { + private readonly string Name; + + private readonly CommandTypes CommandTypes; + + internal CommandLookupKey(string name, CommandTypes? commandTypes) + { + Name = name; + CommandTypes = commandTypes ?? CommandTypes.All; + } + + public bool Equals(CommandLookupKey other) + { + return CommandTypes == other.CommandTypes + && Name.Equals(other.Name, StringComparison.OrdinalIgnoreCase); + } + + public override int GetHashCode() + { + // Algorithm from https://stackoverflow.com/questions/1646807/quick-and-simple-hash-code-combinations + unchecked + { + int hash = 17; + hash = hash * 31 + Name.ToUpperInvariant().GetHashCode(); + hash = hash * 31 + CommandTypes.GetHashCode(); + return hash; + } + } + } + } +} \ No newline at end of file diff --git a/Engine/Helper.cs b/Engine/Helper.cs index b093d9c0f..dd6868eb1 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -24,11 +24,11 @@ public class Helper private CommandInvocationIntrinsics invokeCommand; private IOutputWriter outputWriter; - private Object getCommandLock = new object(); private readonly static Version minSupportedPSVersion = new Version(3, 0); private Dictionary> ruleArguments; private PSVersionTable psVersionTable; - private Dictionary commandInfoCache; + + private readonly Lazy _commandInfoCacheLazy; #endregion @@ -100,6 +100,12 @@ internal set private string[] functionScopes = new string[] { "global:", "local:", "script:", "private:"}; private string[] variableScopes = new string[] { "global:", "local:", "script:", "private:", "variable:", ":"}; + + /// + /// Store of command info objects for commands. Memoizes results. + /// + private CommandInfoCache CommandInfoCache => _commandInfoCacheLazy.Value; + #endregion /// @@ -107,7 +113,7 @@ internal set /// private Helper() { - + _commandInfoCacheLazy = new Lazy(() => new CommandInfoCache(pssaHelperInstance: this)); } /// @@ -123,7 +129,7 @@ private Helper() /// public Helper( CommandInvocationIntrinsics invokeCommand, - IOutputWriter outputWriter) + IOutputWriter outputWriter) : this() { this.invokeCommand = invokeCommand; this.outputWriter = outputWriter; @@ -140,10 +146,6 @@ public void Initialize() KeywordBlockDictionary = new Dictionary>>(StringComparer.OrdinalIgnoreCase); VariableAnalysisDictionary = new Dictionary(); ruleArguments = new Dictionary>(StringComparer.OrdinalIgnoreCase); - if (commandInfoCache == null) - { - commandInfoCache = new Dictionary(); - } IEnumerable aliases = this.invokeCommand.GetCommands("*", CommandTypes.Alias, true); @@ -676,7 +678,6 @@ private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? command } /// - /// Legacy method, new callers should use instead. /// Given a command's name, checks whether it exists. It does not use the passed in CommandTypes parameter, which is a bug. /// But existing method callers are already depending on this behaviour and therefore this could not be simply fixed. @@ -688,30 +689,7 @@ private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? command [Obsolete] public CommandInfo GetCommandInfoLegacy(string name, CommandTypes? commandType = null) { - if (string.IsNullOrWhiteSpace(name)) - { - return null; - } - - // check if it is an alias - string cmdletName = Helper.Instance.GetCmdletNameFromAlias(name); - if (string.IsNullOrWhiteSpace(cmdletName)) - { - cmdletName = name; - } - - var key = new CommandLookupKey(name, commandType); - lock (getCommandLock) - { - if (commandInfoCache.ContainsKey(key)) - { - return commandInfoCache[key]; - } - - var commandInfo = GetCommandInfoInternal(cmdletName, commandType); - commandInfoCache.Add(key, commandInfo); - return commandInfo; - } + return CommandInfoCache.GetCommandInfoLegacy(commandOrAliasName: name, commandTypes: commandType); } /// @@ -722,23 +700,7 @@ public CommandInfo GetCommandInfoLegacy(string name, CommandTypes? commandType = /// public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null) { - if (string.IsNullOrWhiteSpace(name)) - { - return null; - } - - var key = new CommandLookupKey(name, commandType); - lock (getCommandLock) - { - if (commandInfoCache.ContainsKey(key)) - { - return commandInfoCache[key]; - } - - var commandInfo = GetCommandInfoInternal(name, commandType); - commandInfoCache.Add(key, commandInfo); - return commandInfo; - } + return CommandInfoCache.GetCommandInfo(name, commandTypes: commandType); } /// From f4f627a00734e1d08bf28bc2fa501165d4346c4d Mon Sep 17 00:00:00 2001 From: "Christoph Bergmeister [MVP]" Date: Tue, 12 Mar 2019 18:48:19 -0700 Subject: [PATCH 2/3] Fix comment typo Co-Authored-By: rjmholt --- Engine/CommandInfoCache.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Engine/CommandInfoCache.cs b/Engine/CommandInfoCache.cs index 5af7316f9..07df7c381 100644 --- a/Engine/CommandInfoCache.cs +++ b/Engine/CommandInfoCache.cs @@ -43,7 +43,7 @@ public CommandInfo GetCommandInfo(string commandName, string aliasName = null, C // If alias name is given, we store the entry under that, but search with the command name var key = new CommandLookupKey(aliasName ?? commandName, commandTypes); - // Atomically either use PowerShell to query a command info object, or fetch it from the catch + // Atomically either use PowerShell to query a command info object, or fetch it from the cache return _commandInfoCache.GetOrAdd(key, new Lazy(() => GetCommandInfoInternal(commandName, commandTypes))).Value; } @@ -116,4 +116,4 @@ public override int GetHashCode() } } } -} \ No newline at end of file +} From c2fab46bd453efc1a9168a8de931f066000f43b0 Mon Sep 17 00:00:00 2001 From: "Christoph Bergmeister [MVP]" Date: Wed, 13 Mar 2019 08:00:32 +0000 Subject: [PATCH 3/3] Trigger CI --- Engine/CommandInfoCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Engine/CommandInfoCache.cs b/Engine/CommandInfoCache.cs index 07df7c381..20fd211aa 100644 --- a/Engine/CommandInfoCache.cs +++ b/Engine/CommandInfoCache.cs @@ -75,7 +75,7 @@ private static CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? .AddParameter("Name", cmdName) .AddParameter("ErrorAction", "SilentlyContinue"); - if(commandType != null) + if (commandType != null) { ps.AddParameter("CommandType", commandType); }