Skip to content

Fix performance related issues #692

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.

Sign up for GitHub

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
merged 8 commits into from
Jan 19, 2017
Merged

Fix performance related issues #692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b5620bd
Update GetCommandInfo method and cache results
Jan 18, 2017
a5c12c0
Make UseCmdletCorreclty use updated getcommandinfo
Jan 18, 2017
9c6e03c
Make UseShouldProcessCorrectly use updated getcommandinfo
Jan 18, 2017
2fae803
Add runspace pool to improve getcommandinfo perf
Jan 18, 2017
585cca9
Use concurrent dictionary for commandinfo cache
Jan 18, 2017
20d862e
Set max runspaces to 3 for getcommandinfo runspacepool
Jan 18, 2017
b660cc3
Remove redundant checks from some helper methods
Jan 18, 2017
8726c10
Revert adding runspace pool and concurrent dictionary
Jan 19, 2017
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
105 changes: 70 additions & 35 deletions Engine/Helper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public class Helper
private readonly static Version minSupportedPSVersion = new Version(3, 0);
private Dictionary<string, Dictionary<string, object>> ruleArguments;
private PSVersionTable psVersionTable;
private Dictionary<string, CommandInfo> commandInfoCache;

#endregion

Expand Down Expand Up @@ -146,6 +147,7 @@ public void Initialize()
KeywordBlockDictionary = new Dictionary<String, List<Tuple<int, int>>>(StringComparer.OrdinalIgnoreCase);
VariableAnalysisDictionary = new Dictionary<Ast, VariableAnalysis>();
ruleArguments = new Dictionary<string, Dictionary<string, object>>(StringComparer.OrdinalIgnoreCase);
commandInfoCache = new Dictionary<string, CommandInfo>(StringComparer.OrdinalIgnoreCase);

IEnumerable<CommandInfo> aliases = this.invokeCommand.GetCommands("*", CommandTypes.Alias, true);

Expand Down Expand Up @@ -594,12 +596,13 @@ public string VariableNameWithoutScope(VariablePath variablePath)
/// <returns></returns>
public bool HasSplattedVariable(CommandAst cmdAst)
{
if (cmdAst == null || cmdAst.CommandElements == null)
{
return false;
}

return cmdAst.CommandElements.Any(cmdElem => cmdElem is VariableExpressionAst && (cmdElem as VariableExpressionAst).Splatted);
return cmdAst != null
&& cmdAst.CommandElements != null
&& cmdAst.CommandElements.Any(cmdElem =>
{
var varExprAst = cmdElem as VariableExpressionAst;
return varExprAst != null && varExprAst.Splatted;
});
}

/// <summary>
Expand All @@ -610,12 +613,16 @@ public bool HasSplattedVariable(CommandAst cmdAst)
/// <returns></returns>
public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositional = false)
{
if (cmdAst == null || cmdAst.GetCommandName() == null)
if (cmdAst == null)
{
return false;
}

CommandInfo commandInfo = GetCommandInfo(GetCmdletNameFromAlias(cmdAst.GetCommandName())) ?? GetCommandInfo(cmdAst.GetCommandName());
var commandInfo = GetCommandInfo(cmdAst.GetCommandName());
if (commandInfo == null || (commandInfo.CommandType != System.Management.Automation.CommandTypes.Cmdlet))
{
return false;
}

IEnumerable<ParameterMetadata> switchParams = null;

Expand All @@ -642,29 +649,31 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio

foreach (CommandElementAst ceAst in cmdAst.CommandElements)
{
if (ceAst is CommandParameterAst)
var cmdParamAst = ceAst as CommandParameterAst;
if (cmdParamAst != null)
{
// Skip if it's a switch parameter
if (switchParams != null &&
switchParams.Any(
pm => String.Equals(
pm.Name,
cmdParamAst.ParameterName, StringComparison.OrdinalIgnoreCase)))
{
// Skip if it's a switch parameter
if (switchParams != null &&
switchParams.Any(pm => String.Equals(pm.Name, (ceAst as CommandParameterAst).ParameterName, StringComparison.OrdinalIgnoreCase)))
{
continue;
}

continue;
}

parameters += 1;
parameters += 1;

if ((ceAst as CommandParameterAst).Argument != null)
{
arguments += 1;
}

}
else
if (cmdParamAst.Argument != null)
{
arguments += 1;
}

}
else
{
arguments += 1;
}
}

// if not the first element in a pipeline, increase the number of arguments by 1
Expand All @@ -685,6 +694,23 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio
}


/// <summary>
/// Get a CommandInfo object of the given command name
/// </summary>
/// <returns>Returns null if command does not exists</returns>
private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType = null)
{
using (var ps = System.Management.Automation.PowerShell.Create())
{
var cmdInfo = ps.AddCommand("Get-Command")
.AddArgument(cmdName)
.AddParameter("ErrorAction", "SilentlyContinue")
.Invoke<CommandInfo>()
.FirstOrDefault();
return cmdInfo;
}
}

/// <summary>
/// Given a command's name, checks whether it exists
/// </summary>
Expand All @@ -693,19 +719,28 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio
/// <returns></returns>
public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null)
{
CommandTypes defaultCmdType = CommandTypes.Alias
| CommandTypes.Cmdlet
| CommandTypes.ExternalScript
| CommandTypes.Filter
| CommandTypes.Function
| CommandTypes.Script
| CommandTypes.Workflow;
#if !PSV3
defaultCmdType |= CommandTypes.Configuration;
#endif
if (string.IsNullOrWhiteSpace(name))
{
return null;
}

// check if it is an alias
string cmdletName = Helper.Instance.GetCmdletNameFromAlias(name);
if (string.IsNullOrWhiteSpace(cmdletName))
{
cmdletName = name;
}

lock (getCommandLock)
{
return this.invokeCommand.GetCommand(name, commandType ?? defaultCmdType);
if (commandInfoCache.ContainsKey(cmdletName))
{
return commandInfoCache[cmdletName];
}

var commandInfo = GetCommandInfoInternal(cmdletName, commandType);
commandInfoCache.Add(cmdletName, commandInfo);
return commandInfo;
}
}

Expand Down
12 changes: 5 additions & 7 deletions Rules/UseCmdletCorrectly.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
if (cmdAst.GetCommandName() == null) continue;

// Checks mandatory parameters.
if (!IsMandatoryParameterExisted(cmdAst))
if (!MandatoryParameterExists(cmdAst))
{
yield return new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.UseCmdletCorrectlyError, cmdAst.GetCommandName()),
cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
Expand All @@ -68,7 +68,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
/// </summary>
/// <param name="cmdAst"></param>
/// <returns></returns>
private bool IsMandatoryParameterExisted(CommandAst cmdAst)
private bool MandatoryParameterExists(CommandAst cmdAst)
{
CommandInfo cmdInfo = null;
List<ParameterMetadata> mandParams = new List<ParameterMetadata>();
Expand All @@ -88,9 +88,7 @@ private bool IsMandatoryParameterExisted(CommandAst cmdAst)

#region Compares parameter list and mandatory parameter list.

cmdInfo = Helper.Instance.GetCommandInfo(Helper.Instance.GetCmdletNameFromAlias(cmdAst.GetCommandName()))
?? Helper.Instance.GetCommandInfo(cmdAst.GetCommandName());

cmdInfo = Helper.Instance.GetCommandInfo(cmdAst.GetCommandName());
if (cmdInfo == null || (cmdInfo.CommandType != System.Management.Automation.CommandTypes.Cmdlet))
{
return true;
Expand All @@ -109,7 +107,7 @@ private bool IsMandatoryParameterExisted(CommandAst cmdAst)
// If cannot find any mandatory parameter, it's not necessary to do a further check for current cmdlet.
try
{
int noOfParamSets = cmdInfo.ParameterSets.Count;
int noOfParamSets = cmdInfo.ParameterSets.Count;
foreach (ParameterMetadata pm in cmdInfo.Parameters.Values)
{
int count = 0;
Expand Down Expand Up @@ -163,7 +161,7 @@ private bool IsMandatoryParameterExisted(CommandAst cmdAst)

return returnValue;
}

/// <summary>
/// GetName: Retrieves the name of this rule.
/// </summary>
Expand Down
25 changes: 1 addition & 24 deletions Rules/UseShouldProcessCorrectly.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,29 +297,6 @@ private bool DeclaresSupportsShouldProcess(FunctionDefinitionAst ast)
return Helper.Instance.GetNamedArgumentAttributeValue(shouldProcessAttribute);
}

/// <summary>
/// Get a CommandInfo object of the given command name
/// </summary>
/// <returns>Returns null if command does not exists</returns>
private CommandInfo GetCommandInfo(string cmdName)
{
try
{
using (var ps = System.Management.Automation.PowerShell.Create())
{
var cmdInfo = ps.AddCommand("Get-Command")
.AddArgument(cmdName)
.Invoke<CommandInfo>()
.FirstOrDefault();
return cmdInfo;
}
}
catch (System.Management.Automation.CommandNotFoundException)
{
return null;
}
}

/// <summary>
/// Checks if the given command supports ShouldProcess
/// </summary>
Expand All @@ -331,7 +308,7 @@ private bool SupportsShouldProcess(string cmdName)
return false;
}

var cmdInfo = GetCommandInfo(cmdName);
var cmdInfo = Helper.Instance.GetCommandInfo(cmdName);
if (cmdInfo == null)
{
return false;
Expand Down