Improve performance with better CommandInfo cache locking making Invoke-ScriptAnalyzer nearly twice as fast

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,

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;

Engine/Formatter.cs

@@ -31,7 +31,7 @@ public static string Format<TCmdlet>(
             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[]

Engine/Helper.cs

@@ -4,13 +4,16 @@
 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;
 using System.IO;
 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<string, Dictionary<string, object>> ruleArguments;
         private PSVersionTable psVersionTable;
-        private Dictionary<CommandLookupKey, CommandInfo> commandInfoCache;
+        private ConcurrentDictionary<CommandLookupKey, CommandInfo> commandInfoCache;
+        private ConcurrentDictionary<CommandLookupKey, Task<CommandInfo>> commandInfoLookupItemsInProcess;
 
         #endregion
 
@@ -121,12 +123,9 @@ private Helper()
         /// An IOutputWriter instance for use in writing output
         /// to the PowerShell environment.
         /// </param>
-        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<string, Dictionary<string, object>>(StringComparer.OrdinalIgnoreCase);
             if (commandInfoCache == null)
             {
-                commandInfoCache = new Dictionary<CommandLookupKey, CommandInfo>();
+                commandInfoCache = new ConcurrentDictionary<CommandLookupKey, CommandInfo>();
+            }
+            if (commandInfoLookupItemsInProcess == null)
+            {
+                commandInfoLookupItemsInProcess = new ConcurrentDictionary<CommandLookupKey, Task<CommandInfo>>();
             }
 
             IEnumerable<CommandInfo> aliases = this.invokeCommand.GetCommands("*", CommandTypes.Alias, true);
@@ -652,31 +655,69 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanTwoPositiona
 
 
         /// <summary>
-        /// 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.
         /// </summary>
         /// <returns>Returns null if command does not exists</returns>
-        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<CommandInfo>(() => 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<CommandInfo> 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<CommandInfo> GetCommandInfoInternal(string cmdName, CommandTypes? commandType)
         {
             using (var ps = System.Management.Automation.PowerShell.Create())
             {
                 var psCommand = ps.AddCommand("Get-Command")
                     .AddParameter("Name", cmdName)
                     .AddParameter("ErrorAction", "SilentlyContinue");
 
-                if(commandType!=null)
+                if (commandType != null)
                 {
                     psCommand.AddParameter("CommandType", commandType);
                 }
 
-                var commandInfo = psCommand.Invoke<CommandInfo>()
-                         .FirstOrDefault();
+                var commandInfo = psCommand.Invoke<CommandInfo>().FirstOrDefault();
 
-                return commandInfo;
+                return Task.FromResult(commandInfo);
             }
         }
 
         /// <summary>
-
         ///  Legacy method, new callers should use <see cref="GetCommandInfo"/> 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);
         }
 
         /// <summary>
@@ -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);
         }
 
         /// <summary>

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();