FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fast pipeline binding for ForEach-Object by lzybkr · Pull Request #10153 · PowerShell/PowerShell · GitHub

Fast pipeline binding for ForEach-Object - #10153

Closed
Jason Shirk (lzybkr) wants to merge 1 commit into
PowerShell:masterfrom
lzybkr:foreach
Closed

Fast pipeline binding for ForEach-Object#10153
Jason Shirk (lzybkr) wants to merge 1 commit into
PowerShell:masterfrom
lzybkr:foreach

Conversation

Copy link
Copy Markdown
Contributor

PR Summary

Proof of concept change to speed up ForEach-Object. I don't plan to do further work, but someone else can hopefully use the idea.

This undoubtedly misses something important, but it does make ForEach-Object faster. (Note I include the loop for comparison, the cmdlet is still an order of magnitude slower.)

Script Before After
1..100kb | % { } 335ms 180ms
foreach ($i in 1..100kb) { } 10ms 10ms
dir -re PowerShell | % Name 230ms 220ms

PR Context

People often use trivial pipelines with ForEach-Object to micro-benchmark PowerShell and conclude that it is slow, or they should use the foreach statement instead of the cmdlet.

I've long had the idea that we can have dynamic sites like we have for property access or method invocation, for but instead for parameter binding.

This PR is the embodiment of that idea for the single use case of binding InputObject for the ForEach-Object command.

PR Checklist

Copy link
Copy Markdown
Collaborator

When using & { process { $_ }} instead of Foreach-Object/Where-Object, there is a huge performance gain, apparently caused by using a simple function instead of an advanced function. Parameter binding is no issue for these cmdlets in the vast majority of use cases as users typically only use $_ in their payload scriptblocks, and thus internally switching to a simple function w/o cmdletbinding might improve the speed without having to reinvent the entire cmdlet.

Copy link
Copy Markdown

I have an open issue #9941 for Where-Object. Dongbo Wang (@daxian-dbw) is taking care of it. Maybe those two can be worked together? Seems similar to me?

Copy link
Copy Markdown
Collaborator

I guess there is 2 places for perf consideration:

  • Bindings. If source collection is strong typed collection we could bind only once. For common case source collection may contains free-typed objects and we have to do binding for every object.
  • Compilation. Perhaps we could compile to something more faster. Ex., for Where-Object a filter function is more faster than scriptblock, maybe a delegate is better. Although maybe this is again binding.

mi-hol commented Mar 22, 2020

Copy link
Copy Markdown

Jason Shirk (@lzybkr) Dongbo Wang (@daxian-dbw) could the suggestion from Dr. Tobias Weltner (@TobiasPSP) be considered for 7.1? In his blog he shows many alternative approaches to speed up pwsh that should be unnecessary due to optimizations in the core.

Copy link
Copy Markdown
Member

mi-hol Please check out the issue #10982. There is tons of discussion there.

Ilya (iSazonov) added the CL-Performance Indicates that a PR should be marked as a performance improvement in the Change Log label May 18, 2020
ghost added the Review - Needed The PR is being reviewed label May 27, 2020

Copy link
Copy Markdown

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jun 15, 2020
ghost added the Stale label Jun 30, 2020

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 15 days. It will be closed if no further activity occurs within 10 days of this comment.

ghost closed this Jul 10, 2020

God-damnit-all commented Jan 1, 2022
edited
Loading

Copy link
Copy Markdown
Contributor

Why did this PR get marked as 'Waiting on Author' when there wasn't any maintainer feedback that I can see?

EDIT: Oops, nevermind. Didn't notice daxian-dbw was part of the PowerShell team.

ghost removed the Stale label Jan 1, 2022
This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Performance Indicates that a PR should be marked as a performance improvement in the Change Log Review - Needed The PR is being reviewed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL