diff --git a/Engine/Commands/GetScriptAnalyzerRuleCommand.cs b/Engine/Commands/GetScriptAnalyzerRuleCommand.cs index 60ee1c607..bfe421e1f 100644 --- a/Engine/Commands/GetScriptAnalyzerRuleCommand.cs +++ b/Engine/Commands/GetScriptAnalyzerRuleCommand.cs @@ -84,8 +84,7 @@ protected override void BeginProcessing() // Initialize helper Helper.Instance = new Helper( - SessionState.InvokeCommand, - this); + SessionState.InvokeCommand); Helper.Instance.Initialize(); string[] rulePaths = Helper.ProcessCustomRulePaths(customRulePath, diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index c51fc9f38..7328eda5a 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -266,8 +266,7 @@ protected override void BeginProcessing() } #endif Helper.Instance = new Helper( - SessionState.InvokeCommand, - this); + SessionState.InvokeCommand); Helper.Instance.Initialize(); var psVersionTable = this.SessionState.PSVariable.GetValue("PSVersionTable") as Hashtable; diff --git a/Engine/Formatter.cs b/Engine/Formatter.cs index c07d7c5ef..c0845df07 100644 --- a/Engine/Formatter.cs +++ b/Engine/Formatter.cs @@ -31,7 +31,7 @@ public static string Format( ValidateNotNull(settings, "settings"); ValidateNotNull(cmdlet, "cmdlet"); - Helper.Instance = new Helper(cmdlet.SessionState.InvokeCommand, cmdlet); + Helper.Instance = new Helper(cmdlet.SessionState.InvokeCommand); Helper.Instance.Initialize(); var ruleOrder = new string[] diff --git a/Engine/Helper.cs b/Engine/Helper.cs index b093d9c0f..e3f34e2ab 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -4,6 +4,7 @@ using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Globalization; @@ -11,6 +12,8 @@ using System.Linq; using System.Management.Automation; using System.Management.Automation.Language; +using System.Threading; +using System.Threading.Tasks; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer { @@ -22,13 +25,12 @@ public class Helper { #region Private members - private CommandInvocationIntrinsics invokeCommand; - private IOutputWriter outputWriter; - private Object getCommandLock = new object(); + private readonly CommandInvocationIntrinsics invokeCommand; private readonly static Version minSupportedPSVersion = new Version(3, 0); private Dictionary> ruleArguments; private PSVersionTable psVersionTable; - private Dictionary commandInfoCache; + private ConcurrentDictionary commandInfoCache; + private ConcurrentDictionary> commandInfoLookupItemsInProcess; #endregion @@ -121,12 +123,9 @@ private Helper() /// An IOutputWriter instance for use in writing output /// to the PowerShell environment. /// - public Helper( - CommandInvocationIntrinsics invokeCommand, - IOutputWriter outputWriter) + public Helper(CommandInvocationIntrinsics invokeCommand) { this.invokeCommand = invokeCommand; - this.outputWriter = outputWriter; } #region Methods @@ -142,7 +141,11 @@ public void Initialize() ruleArguments = new Dictionary>(StringComparer.OrdinalIgnoreCase); if (commandInfoCache == null) { - commandInfoCache = new Dictionary(); + commandInfoCache = new ConcurrentDictionary(); + } + if (commandInfoLookupItemsInProcess == null) + { + commandInfoLookupItemsInProcess = new ConcurrentDictionary>(); } IEnumerable aliases = this.invokeCommand.GetCommands("*", CommandTypes.Alias, true); @@ -652,10 +655,50 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanTwoPositiona /// - /// Get a CommandInfo object of the given command name + /// Get a CommandInfo object of the given command name using sophisticated caching. + /// Uses the commandInfoCache for performance optimisation. /// /// Returns null if command does not exists - private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType) + private CommandInfo GetCommandInfoInternalWithCommandCache(CommandLookupKey commandLookupKey, string cmdName, CommandTypes? commandType) + { + // Check if CommandInfo is cached + if (commandInfoCache.TryGetValue(commandLookupKey, out CommandInfo cachedCommandInfo)) + { + return cachedCommandInfo; + } + + var commandInfoLookupTask = new Task(() => GetCommandInfoInternal(cmdName, commandType).Result); + if (commandInfoLookupItemsInProcess.TryAdd(commandLookupKey, commandInfoLookupTask)) + { + commandInfoLookupTask.Start(); + commandInfoLookupTask.Wait(); + commandInfoCache.TryAdd(commandLookupKey, commandInfoLookupTask.Result); + commandInfoLookupItemsInProcess.TryRemove(commandLookupKey, out _); + return commandInfoLookupTask.Result; + } + else + { + // Already processing a request for the same commandLookupKey + if (commandInfoLookupItemsInProcess.TryGetValue(commandLookupKey, out Task runningCommandInfoLookupTask)) + { + runningCommandInfoLookupTask.Wait(); + commandInfoCache.TryAdd(commandLookupKey, runningCommandInfoLookupTask.Result); + return runningCommandInfoLookupTask.Result; + } + else + { + // The CommandInfoLookup task in commandInfoLookupItemsInProcess has just finished now and must therefore be in the cache + if (commandInfoCache.TryGetValue(commandLookupKey, out CommandInfo commandInfo)) + { + return commandInfo; + } + // This case should never happen but is being catered for by just calling the function directly + return GetCommandInfoInternal(cmdName, commandType).Result; + } + } + } + + private Task GetCommandInfoInternal(string cmdName, CommandTypes? commandType) { using (var ps = System.Management.Automation.PowerShell.Create()) { @@ -663,20 +706,18 @@ private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? command .AddParameter("Name", cmdName) .AddParameter("ErrorAction", "SilentlyContinue"); - if(commandType!=null) + if (commandType != null) { psCommand.AddParameter("CommandType", commandType); } - var commandInfo = psCommand.Invoke() - .FirstOrDefault(); + var commandInfo = psCommand.Invoke().FirstOrDefault(); - return commandInfo; + return Task.FromResult(commandInfo); } } /// - /// 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. @@ -701,17 +742,7 @@ public CommandInfo GetCommandInfoLegacy(string name, CommandTypes? commandType = } 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 GetCommandInfoInternalWithCommandCache(key, cmdletName, commandType); } /// @@ -728,17 +759,7 @@ public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = 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 GetCommandInfoInternalWithCommandCache(key, name, commandType); } /// diff --git a/Engine/ScriptAnalyzer.cs b/Engine/ScriptAnalyzer.cs index d7d091588..a3456b13b 100644 --- a/Engine/ScriptAnalyzer.cs +++ b/Engine/ScriptAnalyzer.cs @@ -159,8 +159,7 @@ public void Initialize( //initialize helper Helper.Instance = new Helper( - runspace.SessionStateProxy.InvokeCommand, - outputWriter); + runspace.SessionStateProxy.InvokeCommand); Helper.Instance.Initialize();