Skip to content

Commit f6c220c

Browse files
authored
Revert the PR "Make ForEach-Object faster for its commonly used scenarios" (PowerShell#10485)
It turns out this optimization brings in a breaking change: `$MyInvocation` is different comparing to before the optimization change. I tried to fix the breaking change, but couldn't without introducing more hacky code. Given that, that PR should be reverted.
1 parent 5069c7d commit f6c220c

File tree

2 files changed

+11
-110
lines changed

2 files changed

+11
-110
lines changed

src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs

Lines changed: 5 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ private static CommandProcessorBase AddCommand(PipelineProcessor pipe,
6868

6969
object command;
7070
IScriptExtent commandExtent;
71-
var cpiCommand = commandElements[commandIndex++];
71+
var cpiCommand = commandElements[commandIndex];
7272
if (cpiCommand.ParameterNameSpecified)
7373
{
7474
command = cpiCommand.ParameterText;
@@ -156,16 +156,12 @@ private static CommandProcessorBase AddCommand(PipelineProcessor pipe,
156156
}
157157
}
158158

159-
// If possible, rewrite the 'ForEach-Object' command into a filter-like script block in the pipeline.
160-
// e.g. 1..2 | ForEach-Object { $_ + 1 } => 1..2 | . { process { $_ + 1 } }
161-
if (!TryRewriteForEachObjectCommand(context, commandSessionState, commandElements, ref commandProcessor, ref commandIndex))
162-
{
163-
InternalCommand cmd = commandProcessor.Command;
164-
commandProcessor.UseLocalScope = !dotSource && (cmd is ScriptCommand || cmd is PSScriptCmdlet);
165-
}
159+
InternalCommand cmd = commandProcessor.Command;
160+
commandProcessor.UseLocalScope = !dotSource &&
161+
(cmd is ScriptCommand || cmd is PSScriptCmdlet);
166162

167163
bool isNativeCommand = commandProcessor is NativeCommandProcessor;
168-
for (int i = commandIndex; i < commandElements.Length; ++i)
164+
for (int i = commandIndex + 1; i < commandElements.Length; ++i)
169165
{
170166
var cpi = commandElements[i];
171167

@@ -315,107 +311,6 @@ private static CommandProcessorBase AddCommand(PipelineProcessor pipe,
315311
return commandProcessor;
316312
}
317313

318-
private static ConditionalWeakTable<ScriptBlockAst, ScriptBlock> s_astRewriteCache = new ConditionalWeakTable<ScriptBlockAst, ScriptBlock>();
319-
private static ConditionalWeakTable<ScriptBlockAst, ScriptBlock>.CreateValueCallback s_astRewriteCallback =
320-
sbAst =>
321-
{
322-
ScriptBlockAst newScriptBlockAst = new ScriptBlockAst(
323-
sbAst.Extent,
324-
paramBlock: null,
325-
beginBlock: null,
326-
processBlock: (NamedBlockAst)sbAst.EndBlock.Copy(),
327-
endBlock: null,
328-
dynamicParamBlock: null);
329-
newScriptBlockAst.PostParseChecksPerformed = sbAst.PostParseChecksPerformed;
330-
sbAst.Parent?.SetParent(newScriptBlockAst);
331-
332-
return new ScriptBlock(newScriptBlockAst, isFilter: false);
333-
};
334-
335-
private static bool TryRewriteForEachObjectCommand(
336-
ExecutionContext context,
337-
SessionStateInternal commandSessionState,
338-
CommandParameterInternal[] commandElements,
339-
ref CommandProcessorBase commandProcessor,
340-
ref int commandIndex)
341-
{
342-
const string ForEachObject_ProcessParam = "Process";
343-
344-
// Skip optimization in the following cases
345-
// 1. the debugger is enabled -- so a breakpoint set on the command 'ForEach-Object' works properly.
346-
// 2. the 'ConstrainedLanguageMode' has been used for the current runspace -- the language mode transition is tricky,
347-
// and it's better to use the same old code path for safety.
348-
if (context._debuggingMode > 0 || context.HasRunspaceEverUsedConstrainedLanguageMode)
349-
{
350-
return false;
351-
}
352-
353-
var cmdlet = commandProcessor.CommandInfo as CmdletInfo;
354-
if (cmdlet == null || cmdlet.ImplementingType != typeof(ForEachObjectCommand))
355-
{
356-
return false;
357-
}
358-
359-
int indexAdvanceOffset = 0;
360-
int cmdElementsLength = commandElements.Length;
361-
ScriptBlock processScriptBlock = null;
362-
363-
if (commandIndex == cmdElementsLength - 1)
364-
{
365-
// Target ForEach-Object syntax:
366-
// * `... | ForEach-Object { ... } | ...`
367-
// * `... | ForEach-Object -process:{ ... } | ...`
368-
var currentElement = commandElements[commandIndex];
369-
if (currentElement.ArgumentSpecified && !currentElement.ArgumentSplatted &&
370-
(!currentElement.ParameterAndArgumentSpecified || ForEachObject_ProcessParam.Equals(currentElement.ParameterName, StringComparison.OrdinalIgnoreCase)))
371-
{
372-
processScriptBlock = currentElement.ArgumentValue as ScriptBlock;
373-
indexAdvanceOffset = 1;
374-
}
375-
}
376-
else if (commandIndex == cmdElementsLength - 2)
377-
{
378-
// Target ForEach-Object syntax:
379-
// * `... | ForEach-Object -Process { ... } | ...`
380-
var currentElement = commandElements[commandIndex];
381-
var nextElement = commandElements[commandIndex + 1];
382-
383-
if (currentElement.ParameterNameSpecified && !currentElement.ArgumentSpecified &&
384-
ForEachObject_ProcessParam.Equals(currentElement.ParameterName, StringComparison.OrdinalIgnoreCase) &&
385-
nextElement.ArgumentSpecified && !nextElement.ArgumentSplatted && !nextElement.ParameterNameSpecified)
386-
{
387-
processScriptBlock = nextElement.ArgumentValue as ScriptBlock;
388-
indexAdvanceOffset = 2;
389-
}
390-
}
391-
392-
if (processScriptBlock != null && processScriptBlock.Ast is ScriptBlockAst sbAst)
393-
{
394-
if (!sbAst.IsConfiguration && sbAst.ParamBlock == null && sbAst.BeginBlock == null &&
395-
sbAst.ProcessBlock == null && sbAst.DynamicParamBlock == null && sbAst.EndBlock != null &&
396-
sbAst.EndBlock.Unnamed)
397-
{
398-
ScriptBlock sbRewritten = s_astRewriteCache.GetValue(sbAst, s_astRewriteCallback);
399-
ScriptBlock sbToUse = sbRewritten.Clone();
400-
sbToUse.SessionStateInternal = processScriptBlock.SessionStateInternal;
401-
sbToUse.LanguageMode = processScriptBlock.LanguageMode;
402-
403-
// We always clone the script block, so that the cached value doesn't hold on to any session state.
404-
// Foreach-Object invokes the script block in the caller's scope, so do not use new scope.
405-
commandProcessor = CommandDiscovery.CreateCommandProcessorForScript(
406-
sbToUse,
407-
context,
408-
useNewScope: false,
409-
commandSessionState);
410-
411-
commandIndex += indexAdvanceOffset;
412-
return true;
413-
}
414-
}
415-
416-
return false;
417-
}
418-
419314
internal static IEnumerable<CommandParameterInternal> Splat(object splattedValue, Ast splatAst)
420315
{
421316
splattedValue = PSObject.Base(splattedValue);

test/powershell/Modules/Microsoft.PowerShell.Core/ForEach-Object.Tests.ps1

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,10 @@ Describe "ForEach-Object" -Tags "CI" {
4141
$sbToUse = GetScriptBlock
4242
1 | ForEach-Object $sbToUse | Should -BeExactly "ForEachObjectTest-Zoo"
4343
}
44+
45+
It "ForEach-Object scriptblock should get the 'InvocationInfo' from the caller scope" {
46+
$file = New-Item TestDrive:\test.ps1 -ItemType File -Force
47+
Set-Content -Path $file -Value '1 | ForEach-Object { $MyInvocation.MyCommand.Name }'
48+
TestDrive:\test.ps1 | Should -BeExactly "test.ps1"
49+
}
4450
}

0 commit comments

Comments
 (0)