Skip to content

Make Foreach-Object 2x faster by reducing unnecessary allocations and boxing #10047

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
adityapatwardhan merged 4 commits into from
Jul 8, 2019
Merged

Make Foreach-Object 2x faster by reducing unnecessary allocations and boxing #10047

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c68cccc
Make foreach-object 2x faster by reducing unnecessary alloations and …
daxian-dbw Jul 2, 2019
e2ed212
Address CodeFactor issues
daxian-dbw Jul 2, 2019
a405ec0
Address Ilya's comments
daxian-dbw Jul 2, 2019
d519e7a
Revert the bool.tostring change in favor of a more general fix
daxian-dbw Jul 3, 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
1 change: 1 addition & 0 deletions Settings.StyleCop
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@
<Value>rs</Value>
<Value>ps</Value>
<Value>op</Value>
<Value>sb</Value>
</CollectionProperty>
</AnalyzerSettings>
</Analyzer>
Expand Down
7 changes: 5 additions & 2 deletions src/System.Management.Automation/engine/CommandInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,8 +442,11 @@ private MergedCommandParameterMetadata GetMergedCommandParameterMetadataSafely()
// that can mess up the runspace our CommandInfo object came from.

var runspace = (RunspaceBase)_context.CurrentRunspace;
if (!runspace.RunActionIfNoRunningPipelinesWithThreadCheck(
() => GetMergedCommandParameterMetadata(out result)))
if (runspace.CanRunActionInCurrentPipeline())
{
GetMergedCommandParameterMetadata(out result);
}
else
{
_context.Events.SubscribeEvent(
source: null,
Expand Down
19 changes: 11 additions & 8 deletions src/System.Management.Automation/engine/Utils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -806,8 +806,12 @@ internal static bool IsValidPSEditionValue(string editionValue)
{nameof(UpdatableHelp), @"Software\Policies\Microsoft\PowerShellCore\UpdatableHelp"},
{nameof(ConsoleSessionConfiguration), @"Software\Policies\Microsoft\PowerShellCore\ConsoleSessionConfiguration"}
};
private static readonly ConcurrentDictionary<Tuple<ConfigScope, string>, PolicyBase> s_cachedPoliciesFromRegistry =
new ConcurrentDictionary<Tuple<ConfigScope, string>, PolicyBase>();

private static readonly ConcurrentDictionary<ConfigScope, ConcurrentDictionary<string, PolicyBase>> s_cachedPoliciesFromRegistry =
new ConcurrentDictionary<ConfigScope, ConcurrentDictionary<string, PolicyBase>>();

private static readonly Func<ConfigScope, ConcurrentDictionary<string, PolicyBase>> s_subCacheCreationDelegate =
key => new ConcurrentDictionary<string, PolicyBase>(StringComparer.OrdinalIgnoreCase);

/// <summary>
/// The implementation of fetching a specific kind of policy setting from the given configuration scope.
Expand Down Expand Up @@ -915,6 +919,8 @@ internal static bool IsValidPSEditionValue(string editionValue)
private static T GetPolicySettingFromGPO<T>(ConfigScope[] preferenceOrder) where T : PolicyBase, new()
{
PolicyBase policy = null;
string policyName = typeof(T).Name;

foreach (ConfigScope scope in preferenceOrder)
{
if (InternalTestHooks.BypassGroupPolicyCaching)
Expand All @@ -923,13 +929,10 @@ internal static bool IsValidPSEditionValue(string editionValue)
}
else
{
var key = Tuple.Create(scope, typeof(T).Name);
if (!s_cachedPoliciesFromRegistry.TryGetValue(key, out policy))
var subordinateCache = s_cachedPoliciesFromRegistry.GetOrAdd(scope, s_subCacheCreationDelegate);
if (!subordinateCache.TryGetValue(policyName, out policy))
{
lock (s_cachedPoliciesFromRegistry)
{
policy = s_cachedPoliciesFromRegistry.GetOrAdd(key, tuple => GetPolicySettingFromGPOImpl<T>(tuple.Item1));
}
policy = subordinateCache.GetOrAdd(policyName, key => GetPolicySettingFromGPOImpl<T>(scope));
}
}

Expand Down
20 changes: 3 additions & 17 deletions src/System.Management.Automation/engine/hostifaces/ConnectionBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -971,30 +971,16 @@ protected void StopPipelines()
}
}

internal bool RunActionIfNoRunningPipelinesWithThreadCheck(Action action)
internal bool CanRunActionInCurrentPipeline()
{
bool ranit = false;
bool shouldRunAction = false;
lock (_pipelineListLock)
{
// If we have no running pipeline, or if the currently running pipeline is
// the same as the current thread, then execute the action.

var pipelineRunning = _currentlyRunningPipeline as PipelineBase;
if (pipelineRunning == null ||
Thread.CurrentThread.Equals(pipelineRunning.NestedPipelineExecutionThread))
{
shouldRunAction = true;
}
return pipelineRunning == null ||
Thread.CurrentThread == pipelineRunning.NestedPipelineExecutionThread;
}

if (shouldRunAction)
{
action();
ranit = true;
}

return ranit;
}

/// <summary>
Expand Down
11 changes: 8 additions & 3 deletions src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@
using System.Globalization;
using System.IO;
using System.Management.Automation.Configuration;
using System.Management.Automation.Internal;
using System.Management.Automation.Runspaces;
using System.Security;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

using Microsoft.PowerShell.Commands;

namespace System.Management.Automation.Host
{
/// <summary>
Expand Down Expand Up @@ -927,7 +926,10 @@ internal void TranscribeError(ExecutionContext context, InvocationInfo invocatio
/// </summary>
internal static TranscriptionOption GetSystemTranscriptOption(TranscriptionOption currentTranscript)
{
var transcription = Utils.GetPolicySetting<Transcription>(Utils.SystemWideThenCurrentUserConfig);
var transcription = InternalTestHooks.BypassGroupPolicyCaching
? Utils.GetPolicySetting<Transcription>(Utils.SystemWideThenCurrentUserConfig)
: s_transcriptionSettingCache.Value;

if (transcription != null)
{
// If we have an existing system transcript for this process, use that.
Expand All @@ -948,6 +950,9 @@ internal static TranscriptionOption GetSystemTranscriptOption(TranscriptionOptio

internal static TranscriptionOption systemTranscript = null;
private static object s_systemTranscriptLock = new Object();
private static Lazy<Transcription> s_transcriptionSettingCache = new Lazy<Transcription>(
() => Utils.GetPolicySetting<Transcription>(Utils.SystemWideThenCurrentUserConfig),
isThreadSafe: true);

private static TranscriptionOption GetTranscriptOptionFromSettings(Transcription transcriptConfig, TranscriptionOption currentTranscript)
{
Expand Down
10 changes: 8 additions & 2 deletions src/System.Management.Automation/engine/lang/scriptblock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -975,7 +975,8 @@ internal void InvokeWithPipe(
try
{
var runspace = (RunspaceBase)context.CurrentRunspace;
shouldGenerateEvent = !runspace.RunActionIfNoRunningPipelinesWithThreadCheck(() =>
if (runspace.CanRunActionInCurrentPipeline())
{
InvokeWithPipeImpl(
useLocalScope,
functionsToDefine,
Expand All @@ -986,7 +987,12 @@ internal void InvokeWithPipe(
scriptThis,
outputPipe,
invocationInfo,
args));
args);
}
else
{
shouldGenerateEvent = true;
}
}
finally
{
Expand Down
115 changes: 69 additions & 46 deletions src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ private IParameterMetadataProvider DelayParseScriptText()
internal Guid Id { get; private set; }

internal bool HasLogged { get; set; }
internal bool SkipLogging { get; set; }
internal bool IsFilter { get; private set; }

internal bool IsProductCode
Expand Down Expand Up @@ -824,6 +825,13 @@ internal bool HasLogged
set { _scriptBlockData.HasLogged = value; }
}

internal bool SkipLogging
{
get { return _scriptBlockData.SkipLogging; }

set { _scriptBlockData.SkipLogging = value; }
}

internal Assembly AssemblyDefiningPSTypes { set; get; }

internal HelpInfo GetHelpInfo(ExecutionContext context, CommandInfo commandInfo, bool dontSearchOnRemoteComputer,
Expand Down Expand Up @@ -1111,7 +1119,7 @@ internal void InvokeWithPipeImpl(ScriptBlockClauseToInvoke clauseToInvoke,
{
if (context.EngineSessionState.CurrentScope.LocalsTuple == null)
{
// If the locals tuple is, that means either:
// If the locals tuple is null, that means either:
// * we're invoking a script block for a module
// * something unexpected
context.EngineSessionState.CurrentScope.LocalsTuple = locals;
Expand Down Expand Up @@ -1361,53 +1369,61 @@ internal bool Compile(bool optimized)

internal static void LogScriptBlockCreation(ScriptBlock scriptBlock, bool force)
{
if (scriptBlock.HasLogged && !InternalTestHooks.ForceScriptBlockLogging)
{
// Fast exit if the script block is already logged and we are not force re-logging in tests.
return;
}

ScriptBlockLogging logSetting = GetScriptBlockLoggingSetting();
if (force || logSetting?.EnableScriptBlockLogging == true)
{
if (!scriptBlock.HasLogged || InternalTestHooks.ForceScriptBlockLogging)
// If script block logging is explicitly disabled, or it's from a trusted
// file or internal, skip logging.
if (logSetting?.EnableScriptBlockLogging == false ||
scriptBlock.ScriptBlockData.IsProductCode)
{
// If script block logging is explicitly disabled, or it's from a trusted
// file or internal, skip logging.
if (logSetting?.EnableScriptBlockLogging == false ||
scriptBlock.ScriptBlockData.IsProductCode)
{
return;
}
scriptBlock.SkipLogging = true;
return;
}

string scriptBlockText = scriptBlock.Ast.Extent.Text;
bool written = false;
string scriptBlockText = scriptBlock.Ast.Extent.Text;
bool written = false;

// Maximum size of ETW events is 64kb. Split a message if it is larger than 20k (Unicode) characters.
if (scriptBlockText.Length < 20000)
{
written = WriteScriptBlockToLog(scriptBlock, 0, 1, scriptBlock.Ast.Extent.Text);
}
else
// Maximum size of ETW events is 64kb. Split a message if it is larger than 20k (Unicode) characters.
if (scriptBlockText.Length < 20000)
{
written = WriteScriptBlockToLog(scriptBlock, 0, 1, scriptBlock.Ast.Extent.Text);
}
else
{
// But split the segments into random sizes (10k + between 0 and 10kb extra)
// so that attackers can't creatively force their scripts to span well-known
// segments (making simple rules less reliable).
int segmentSize = 10000 + (new Random()).Next(10000);
int segments = (int)Math.Floor((double)(scriptBlockText.Length / segmentSize)) + 1;
int currentLocation = 0;
int currentSegmentSize = 0;

for (int segment = 0; segment < segments; segment++)
{
// But split the segments into random sizes (10k + between 0 and 10kb extra)
// so that attackers can't creatively force their scripts to span well-known
// segments (making simple rules less reliable).
int segmentSize = 10000 + (new Random()).Next(10000);
int segments = (int)Math.Floor((double)(scriptBlockText.Length / segmentSize)) + 1;
int currentLocation = 0;
int currentSegmentSize = 0;

for (int segment = 0; segment < segments; segment++)
{
currentLocation = segment * segmentSize;
currentSegmentSize = Math.Min(segmentSize, scriptBlockText.Length - currentLocation);
currentLocation = segment * segmentSize;
currentSegmentSize = Math.Min(segmentSize, scriptBlockText.Length - currentLocation);

string textToLog = scriptBlockText.Substring(currentLocation, currentSegmentSize);
written = WriteScriptBlockToLog(scriptBlock, segment, segments, textToLog);
}
string textToLog = scriptBlockText.Substring(currentLocation, currentSegmentSize);
written = WriteScriptBlockToLog(scriptBlock, segment, segments, textToLog);
}
}

if (written)
{
scriptBlock.HasLogged = true;
}
if (written)
{
scriptBlock.HasLogged = true;
}
}
else
{
scriptBlock.SkipLogging = true;
}
}

private static bool WriteScriptBlockToLog(ScriptBlock scriptBlock, int segment, int segments, string textToLog)
Expand Down Expand Up @@ -1582,6 +1598,9 @@ private static bool GetAndValidateEncryptionRecipients(ScriptBlock scriptBlock,
private static string s_lastSeenCertificate = string.Empty;
private static bool s_hasProcessedCertificate = false;
private static CmsMessageRecipient[] s_encryptionRecipients = null;
private static Lazy<ScriptBlockLogging> s_sbLoggingSettingCache = new Lazy<ScriptBlockLogging>(
() => Utils.GetPolicySetting<ScriptBlockLogging>(Utils.SystemWideThenCurrentUserConfig),
isThreadSafe: true);

// Reset any static caches if the certificate has changed
private static void ResetCertificateCacheIfNeeded(string certificate)
Expand All @@ -1596,7 +1615,12 @@ private static void ResetCertificateCacheIfNeeded(string certificate)

private static ScriptBlockLogging GetScriptBlockLoggingSetting()
{
return Utils.GetPolicySetting<ScriptBlockLogging>(Utils.SystemWideThenCurrentUserConfig);
if (InternalTestHooks.BypassGroupPolicyCaching)
{
return Utils.GetPolicySetting<ScriptBlockLogging>(Utils.SystemWideThenCurrentUserConfig);
}

return s_sbLoggingSettingCache.Value;
}

// Quick check for script blocks that may have suspicious content. If this
Expand Down Expand Up @@ -1919,17 +1943,16 @@ char ToLower(char c)

internal static void LogScriptBlockStart(ScriptBlock scriptBlock, Guid runspaceId)
{
// When invoking, log the creation of the script block if it has suspicious
// content
bool forceLogCreation = false;
if (scriptBlock._scriptBlockData.HasSuspiciousContent)
// Fast exit if the script block can skip logging.
if (!scriptBlock.SkipLogging)
{
forceLogCreation = true;
}
// When invoking, log the creation of the script block if it has suspicious content
bool forceLogCreation = scriptBlock._scriptBlockData.HasSuspiciousContent;

// We delay logging the creation util the 'Start' so that we can be sure we've
// properly analyzed the script block's security.
LogScriptBlockCreation(scriptBlock, forceLogCreation);
// We delay logging the creation until the 'Start' so that we can be sure we've
// properly analyzed the script block's security.
LogScriptBlockCreation(scriptBlock, forceLogCreation);
}

if (GetScriptBlockLoggingSetting()?.EnableScriptBlockInvocationLogging == true)
{
Expand Down
2 changes: 1 addition & 1 deletion stylecop.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
},
"namingRules" : {
"allowCommonHungarianPrefixes" : true,
"allowedHungarianPrefixes" : [ "n", "r", "l", "i", "io", "is", "fs", "lp", "dw", "h", "rs", "ps", "op" ]
"allowedHungarianPrefixes" : [ "n", "r", "l", "i", "io", "is", "fs", "lp", "dw", "h", "rs", "ps", "op", "sb" ]
},
"maintainabilityRules" : {
"topLevelTypes" : [
Expand Down