Skip to content
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

Speedup cold runs of PSSA by using a runspace pool and parallelizing the slowest rule (AvoidAlias) #1178

10 changes: 7 additions & 3 deletions Engine/CommandInfoCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Concurrent;
using System.Management.Automation;
using System.Linq;
using System.Management.Automation.Runspaces;

namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
{
Expand All @@ -14,16 +15,17 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
internal class CommandInfoCache
{
private readonly ConcurrentDictionary<CommandLookupKey, Lazy<CommandInfo>> _commandInfoCache;

private readonly Helper _helperInstance;
private readonly RunspacePool _runspacePool;

/// <summary>
/// Create a fresh command info cache instance.
/// </summary>
public CommandInfoCache(Helper pssaHelperInstance)
public CommandInfoCache(Helper pssaHelperInstance, RunspacePool runspacePool)
{
_commandInfoCache = new ConcurrentDictionary<CommandLookupKey, Lazy<CommandInfo>>();
_helperInstance = pssaHelperInstance;
_runspacePool = runspacePool;
}

/// <summary>
Expand Down Expand Up @@ -67,10 +69,12 @@ public CommandInfo GetCommandInfoLegacy(string commandOrAliasName, CommandTypes?
/// Get a CommandInfo object of the given command name
/// </summary>
/// <returns>Returns null if command does not exists</returns>
private static CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType)
private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType)
{
using (var ps = System.Management.Automation.PowerShell.Create())
{
ps.RunspacePool = _runspacePool;

ps.AddCommand("Get-Command")
.AddParameter("Name", cmdName)
.AddParameter("ErrorAction", "SilentlyContinue");
Expand Down
40 changes: 11 additions & 29 deletions Engine/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Linq;
using System.Management.Automation;
using System.Management.Automation.Language;
using System.Management.Automation.Runspaces;

namespace Microsoft.Windows.PowerShell.ScriptAnalyzer
{
Expand All @@ -29,6 +30,7 @@ public class Helper
private PSVersionTable psVersionTable;

private readonly Lazy<CommandInfoCache> _commandInfoCacheLazy;
private readonly RunspacePool _runSpacePool;

#endregion

Expand Down Expand Up @@ -113,7 +115,11 @@ internal set
/// </summary>
private Helper()
{
_commandInfoCacheLazy = new Lazy<CommandInfoCache>(() => new CommandInfoCache(pssaHelperInstance: this));
// There are 5 rules that use the CommandInfo cache but one rule (AvoidAlias) makes parallel queries.
// Therefore 10 runspaces was a heuristic measure where no more speed improvement was seen.
_runSpacePool = RunspaceFactory.CreateRunspacePool(1, 10);
_runSpacePool.Open();
_commandInfoCacheLazy = new Lazy<CommandInfoCache>(() => new CommandInfoCache(pssaHelperInstance: this, runspacePool: _runSpacePool));
}

/// <summary>
Expand Down Expand Up @@ -299,11 +305,12 @@ public PSModuleInfo GetModuleManifest(string filePath, out IEnumerable<ErrorReco
Collection<PSObject> psObj = null;
using (var ps = System.Management.Automation.PowerShell.Create())
{
ps.RunspacePool = _runSpacePool;
ps.AddCommand("Test-ModuleManifest")
.AddParameter("Path", filePath)
.AddParameter("WarningAction", ActionPreference.SilentlyContinue);
try
{
ps.AddCommand("Test-ModuleManifest");
ps.AddParameter("Path", filePath);
ps.AddParameter("WarningAction", ActionPreference.SilentlyContinue);
psObj = ps.Invoke();
}
catch (CmdletInvocationException e)
Expand Down Expand Up @@ -653,31 +660,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanTwoPositiona
return moreThanTwoPositional ? argumentsWithoutProcedingParameters > 2 : argumentsWithoutProcedingParameters > 0;
}


/// <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)
{
using (var ps = System.Management.Automation.PowerShell.Create())
{
var psCommand = ps.AddCommand("Get-Command")
.AddParameter("Name", cmdName)
.AddParameter("ErrorAction", "SilentlyContinue");

if(commandType!=null)
{
psCommand.AddParameter("CommandType", commandType);
}

var commandInfo = psCommand.Invoke<CommandInfo>()
.FirstOrDefault();

return 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.
Expand Down
65 changes: 48 additions & 17 deletions Rules/AvoidAlias.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
using System.Globalization;
using System.Linq;
using System.Management.Automation;
using System.Threading.Tasks;
using System.Collections.Concurrent;

namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
{
Expand Down Expand Up @@ -95,7 +97,8 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
// Finds all CommandAsts.
IEnumerable<Ast> foundAsts = ast.FindAll(testAst => testAst is CommandAst, true);

// Iterates all CommandAsts and check the command name.
// Iterates all CommandAsts and check the command name. Expensive operations are run in background tasks
var tasks = new List<Task<DiagnosticRecord>>();
foreach (CommandAst cmdAst in foundAsts)
{
// Check if the command ast should be ignored
Expand Down Expand Up @@ -128,26 +131,54 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
suggestedCorrections: GetCorrectionExtent(cmdAst, cmdletNameIfCommandNameWasAlias));
}

var isNativeCommand = Helper.Instance.GetCommandInfo(commandName, CommandTypes.Application | CommandTypes.ExternalScript) != null;
if (!isNativeCommand)
// Checking for implicit 'Get-' aliasing is done in a background task as it can be quite expensive during a cold-start
tasks.Add(Task<DiagnosticRecord>.Run(() =>
{
var commdNameWithGetPrefix = $"Get-{commandName}";
var cmdletNameIfCommandWasMissingGetPrefix = Helper.Instance.GetCommandInfo($"Get-{commandName}");
if (cmdletNameIfCommandWasMissingGetPrefix != null)
{
yield return new DiagnosticRecord(
string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesMissingGetPrefixError, commandName, commdNameWithGetPrefix),
GetCommandExtent(cmdAst),
GetName(),
DiagnosticSeverity.Warning,
fileName,
commandName,
suggestedCorrections: GetCorrectionExtent(cmdAst, commdNameWithGetPrefix));
}
return CheckForImplicitGetAliasing(commandName, cmdAst, fileName);
}));
}
foreach(Task<DiagnosticRecord> task in tasks)
{
DiagnosticRecord diagnosticRecordResult = task.Result;
if (diagnosticRecordResult != null)
{
yield return task.Result;
}
}
}

/// <summary>
/// If one omitts, 'Get-' for a command, PowerShell will pre-pend it if such a command exists, therefore relying on such implicit aliasing is not desirable.
/// </summary>
/// <param name="commandName"></param>
/// <param name="commandAst"></param>
/// <param name="fileName"></param>
/// <returns></returns>
private DiagnosticRecord CheckForImplicitGetAliasing(string commandName, CommandAst commandAst, string fileName)
{
bool isNativeCommand = Helper.Instance.GetCommandInfo(commandName, CommandTypes.Application | CommandTypes.ExternalScript) != null;
if (isNativeCommand)
{
return null;
}
var commdNameWithGetPrefix = $"Get-{commandName}";
var cmdletNameIfCommandWasMissingGetPrefix = Helper.Instance.GetCommandInfo($"Get-{commandName}");
if (cmdletNameIfCommandWasMissingGetPrefix == null)
{
return null;
}

var diagnosticRecord = new DiagnosticRecord(
string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesMissingGetPrefixError, commandName, commdNameWithGetPrefix),
GetCommandExtent(commandAst),
GetName(),
DiagnosticSeverity.Warning,
fileName,
commandName,
suggestedCorrections: GetCorrectionExtent(commandAst, commdNameWithGetPrefix));
return diagnosticRecord;
}

/// <summary>
/// Checks commandast of the form "[commandElement0] = [CommandElement2]". This typically occurs in a DSC configuration.
/// </summary>
Expand Down Expand Up @@ -264,4 +295,4 @@ public string GetSourceName()
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
}
}
}
}