From f6c220cdd90c2120ef87d0683a1168e95f0a3076 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 4 Sep 2019 22:12:30 -0700 Subject: [PATCH] Revert the PR "Make `ForEach-Object` faster for its commonly used scenarios" (#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. --- .../engine/runtime/Operations/MiscOps.cs | 115 +----------------- .../ForEach-Object.Tests.ps1 | 6 + 2 files changed, 11 insertions(+), 110 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs index 780a970854..db135416d4 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/MiscOps.cs @@ -68,7 +68,7 @@ namespace System.Management.Automation object command; IScriptExtent commandExtent; - var cpiCommand = commandElements[commandIndex++]; + var cpiCommand = commandElements[commandIndex]; if (cpiCommand.ParameterNameSpecified) { command = cpiCommand.ParameterText; @@ -156,16 +156,12 @@ namespace System.Management.Automation } } - // If possible, rewrite the 'ForEach-Object' command into a filter-like script block in the pipeline. - // e.g. 1..2 | ForEach-Object { $_ + 1 } => 1..2 | . { process { $_ + 1 } } - if (!TryRewriteForEachObjectCommand(context, commandSessionState, commandElements, ref commandProcessor, ref commandIndex)) - { - InternalCommand cmd = commandProcessor.Command; - commandProcessor.UseLocalScope = !dotSource && (cmd is ScriptCommand || cmd is PSScriptCmdlet); - } + InternalCommand cmd = commandProcessor.Command; + commandProcessor.UseLocalScope = !dotSource && + (cmd is ScriptCommand || cmd is PSScriptCmdlet); bool isNativeCommand = commandProcessor is NativeCommandProcessor; - for (int i = commandIndex; i < commandElements.Length; ++i) + for (int i = commandIndex + 1; i < commandElements.Length; ++i) { var cpi = commandElements[i]; @@ -315,107 +311,6 @@ namespace System.Management.Automation return commandProcessor; } - private static ConditionalWeakTable s_astRewriteCache = new ConditionalWeakTable(); - private static ConditionalWeakTable.CreateValueCallback s_astRewriteCallback = - sbAst => - { - ScriptBlockAst newScriptBlockAst = new ScriptBlockAst( - sbAst.Extent, - paramBlock: null, - beginBlock: null, - processBlock: (NamedBlockAst)sbAst.EndBlock.Copy(), - endBlock: null, - dynamicParamBlock: null); - newScriptBlockAst.PostParseChecksPerformed = sbAst.PostParseChecksPerformed; - sbAst.Parent?.SetParent(newScriptBlockAst); - - return new ScriptBlock(newScriptBlockAst, isFilter: false); - }; - - private static bool TryRewriteForEachObjectCommand( - ExecutionContext context, - SessionStateInternal commandSessionState, - CommandParameterInternal[] commandElements, - ref CommandProcessorBase commandProcessor, - ref int commandIndex) - { - const string ForEachObject_ProcessParam = "Process"; - - // Skip optimization in the following cases - // 1. the debugger is enabled -- so a breakpoint set on the command 'ForEach-Object' works properly. - // 2. the 'ConstrainedLanguageMode' has been used for the current runspace -- the language mode transition is tricky, - // and it's better to use the same old code path for safety. - if (context._debuggingMode > 0 || context.HasRunspaceEverUsedConstrainedLanguageMode) - { - return false; - } - - var cmdlet = commandProcessor.CommandInfo as CmdletInfo; - if (cmdlet == null || cmdlet.ImplementingType != typeof(ForEachObjectCommand)) - { - return false; - } - - int indexAdvanceOffset = 0; - int cmdElementsLength = commandElements.Length; - ScriptBlock processScriptBlock = null; - - if (commandIndex == cmdElementsLength - 1) - { - // Target ForEach-Object syntax: - // * `... | ForEach-Object { ... } | ...` - // * `... | ForEach-Object -process:{ ... } | ...` - var currentElement = commandElements[commandIndex]; - if (currentElement.ArgumentSpecified && !currentElement.ArgumentSplatted && - (!currentElement.ParameterAndArgumentSpecified || ForEachObject_ProcessParam.Equals(currentElement.ParameterName, StringComparison.OrdinalIgnoreCase))) - { - processScriptBlock = currentElement.ArgumentValue as ScriptBlock; - indexAdvanceOffset = 1; - } - } - else if (commandIndex == cmdElementsLength - 2) - { - // Target ForEach-Object syntax: - // * `... | ForEach-Object -Process { ... } | ...` - var currentElement = commandElements[commandIndex]; - var nextElement = commandElements[commandIndex + 1]; - - if (currentElement.ParameterNameSpecified && !currentElement.ArgumentSpecified && - ForEachObject_ProcessParam.Equals(currentElement.ParameterName, StringComparison.OrdinalIgnoreCase) && - nextElement.ArgumentSpecified && !nextElement.ArgumentSplatted && !nextElement.ParameterNameSpecified) - { - processScriptBlock = nextElement.ArgumentValue as ScriptBlock; - indexAdvanceOffset = 2; - } - } - - if (processScriptBlock != null && processScriptBlock.Ast is ScriptBlockAst sbAst) - { - if (!sbAst.IsConfiguration && sbAst.ParamBlock == null && sbAst.BeginBlock == null && - sbAst.ProcessBlock == null && sbAst.DynamicParamBlock == null && sbAst.EndBlock != null && - sbAst.EndBlock.Unnamed) - { - ScriptBlock sbRewritten = s_astRewriteCache.GetValue(sbAst, s_astRewriteCallback); - ScriptBlock sbToUse = sbRewritten.Clone(); - sbToUse.SessionStateInternal = processScriptBlock.SessionStateInternal; - sbToUse.LanguageMode = processScriptBlock.LanguageMode; - - // We always clone the script block, so that the cached value doesn't hold on to any session state. - // Foreach-Object invokes the script block in the caller's scope, so do not use new scope. - commandProcessor = CommandDiscovery.CreateCommandProcessorForScript( - sbToUse, - context, - useNewScope: false, - commandSessionState); - - commandIndex += indexAdvanceOffset; - return true; - } - } - - return false; - } - internal static IEnumerable Splat(object splattedValue, Ast splatAst) { splattedValue = PSObject.Base(splattedValue); diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ForEach-Object.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ForEach-Object.Tests.ps1 index 2e8ea686a4..d3d5ea97ca 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/ForEach-Object.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ForEach-Object.Tests.ps1 @@ -41,4 +41,10 @@ Describe "ForEach-Object" -Tags "CI" { $sbToUse = GetScriptBlock 1 | ForEach-Object $sbToUse | Should -BeExactly "ForEachObjectTest-Zoo" } + + It "ForEach-Object scriptblock should get the 'InvocationInfo' from the caller scope" { + $file = New-Item TestDrive:\test.ps1 -ItemType File -Force + Set-Content -Path $file -Value '1 | ForEach-Object { $MyInvocation.MyCommand.Name }' + TestDrive:\test.ps1 | Should -BeExactly "test.ps1" + } }