Skip to content

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

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
Jun 5, 2019
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b00ac0b
Speed up cold runs of PSSA by 20% by using a RunspacePool when queryi…
bergmeister Mar 16, 2019
587ac7b
Use runspace pool for rule helper for manifests as well
bergmeister Mar 16, 2019
f8e6114
Speed up cold starts of PSSA by another 35% by parallelising the expe…
bergmeister Mar 16, 2019
785cb3f
Apply suggestions from code review
rjmholt Apr 15, 2019
250e941
Reduce indentation level
bergmeister Apr 15, 2019
1528e99
use build pattern for less repeated code
bergmeister Apr 15, 2019
757e307
trigger ci
bergmeister Apr 16, 2019
29d99cb
Merge branch 'development' of https://github.com/PowerShell/PSScriptA…
bergmeister Jun 5, 2019
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
10 changes: 7 additions & 3 deletions Engine/CommandInfoCache.cs
View file
Open in desktop
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
View file
Open in desktop
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");
ps.AddParameter("Path", filePath);
ps.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
61 changes: 45 additions & 16 deletions Rules/AvoidAlias.cs
View file
Open in desktop
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,24 +131,50 @@ 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(var task in tasks)
{
var 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)
{
var isNativeCommand = Helper.Instance.GetCommandInfo(commandName, CommandTypes.Application | CommandTypes.ExternalScript) != null;
if (!isNativeCommand)
{
var commdNameWithGetPrefix = $"Get-{commandName}";
var cmdletNameIfCommandWasMissingGetPrefix = Helper.Instance.GetCommandInfo($"Get-{commandName}");
if (cmdletNameIfCommandWasMissingGetPrefix != 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;
}
}
return null;
}

/// <summary>
Expand Down