GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Add tools for PowerShell perf analysis by lzybkr · Pull Request #7595 · PowerShell/PowerShell

Conversation

Jason Shirk (lzybkr) commented Aug 21, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

PR Summary

These scripts and additional files are useful in analyzing PowerShell performance on Windows with either perfview or wpr/wpa.

PR Checklist

PR has a meaningful title
- Use the present tense and imperative mood when describing your changes
Summarized changes
Change is not breaking
Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
This PR is ready to merge and is not Work in Progress.
- If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
User-facing changes
- Not Applicable
- OR
- User-facing Documentation needed
  - Issue filed - Issue link:
Testing - New and feature
- Not Applicable or can only be tested interactively
- OR
- Make sure you've added a new test if existing tests do not effectively test the code changed
  - Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

powercode, adityapatwardhan, and daxian-dbw reacted with hooray emoji

All reactions

🎉 3 reactions

Jason Shirk (lzybkr) requested review from Travis Plunk (TravisEz13), Aditya Patwardhan (adityapatwardhan), Andrew (anmenaga), Dan Travison (dantraMSFT) and Dongbo Wang (daxian-dbw) as code owners

August 21, 2018 23:15

Jason Shirk (lzybkr) mentioned this pull request

Aug 21, 2018

Write-Host 150ms+ delay after first command when executed from profile #7577

Closed

Staffan Gustafsson (powercode) commented Aug 21, 2018

Copy link

Copy Markdown

Collaborator

I'm so happy!!

Maybe add some info regarding using builds with symbols (cross gened) to get usable stacks?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Kyle Sabo (Robo210) reviewed

Aug 22, 2018

View reviewed changes

Comment thread

tools/performance/GC.wprp Outdated

Kyle Sabo (Robo210) Aug 22, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's... 1 GB of buffers? Seems a bit excessive.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. I forget where I copied that profile from and it looks like it's not actually being used so I'll just delete the file.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kyle Sabo (Robo210) reviewed

Aug 22, 2018

View reviewed changes

Comment thread

tools/performance/GC.wprp Outdated

Kyle Sabo (Robo210) Aug 22, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without a System collector this profile won't be usable by itself (it won't have process, thread, or loader events) - the user will always have to start at least one other session alongside this in order to make sense of the data in WPA.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I'm removing this profile.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kyle Sabo (Robo210) reviewed

Aug 22, 2018

View reviewed changes

Comment thread

tools/performance/README.md Outdated

Kyle Sabo (Robo210) Aug 22, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These wprp files aren't WPA regions files, they are WPR profiles.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I clearly wasn't thinking when I wrote that - I'm removing the files.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kyle Sabo (Robo210) reviewed

Aug 22, 2018

View reviewed changes

Comment thread

tools/performance/PowerShellPerf.wprp Outdated

Kyle Sabo (Robo210) Aug 22, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your other profiles capture a small subset of the events this one does, but use a much larger set of buffers.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the profile we care about. I don't recall seeing dropped events, so maybe this is fine?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Kyle Sabo (Robo210) reviewed

Aug 22, 2018

View reviewed changes

Comment thread

tools/performance/PowerShellPerf.wprp Outdated

Kyle Sabo (Robo210) Aug 22, 2018

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite what the outdated MSDN documentation says, the Name attribute has been optional since Windows 8.1, and should be considered deprecated.

There's probably no value targeting the WPR that ships with the Windows 8 ADK as opposed to the Windows 8.1 ADK. Newer versions of the ADK can be installed on older OS releases.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I appreciate the review. I agree, folks profiling PowerShell probably aren't running Win8 so I'll just remove the Name attribute.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Aditya Patwardhan (adityapatwardhan) requested changes

Aug 22, 2018

View reviewed changes

Comment thread

tools/performance/Invoke-PerfviewPS.ps1 Outdated

Aditya Patwardhan (adityapatwardhan) Aug 22, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be uncommented?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it's expensive and not normally needed, but good to know about so I left it. I'll add a comment to explain.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

tools/performance/Invoke-PerfviewPS.ps1 Outdated

Aditya Patwardhan (adityapatwardhan) Aug 22, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this a parameter? So we can launch a different pwsh.exe version than the one we are using.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

tools/performance/GC.xml Outdated

Aditya Patwardhan (adityapatwardhan) Aug 22, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add EOL

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

tools/performance/PowerShell.stacktags Outdated

Aditya Patwardhan (adityapatwardhan) Aug 22, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment why sometimes the module name has .ni while sometimes it doesn't

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, I'm not sure, and maybe not relevant to PowerShell Core. This file isn't really that refined at all, so the hope is folks will contribute and make it more useful,

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Jason Shirk (lzybkr) Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I remember now, comment added.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

tools/performance/PowerShellPerf.wpaProfile Outdated

Aditya Patwardhan (adityapatwardhan) Aug 22, 2018

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add EOL

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Add tools for PowerShell perf analysis

683cc79

Jason Shirk (lzybkr) commented Aug 22, 2018

Copy link

Copy Markdown

Contributor Author

Sorry about amending instead of adding, mostly just renaming and reformatting, but all feedback should be addressed now.

powercode reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Staffan Gustafsson (powercode) commented Aug 30, 2018

Copy link

Copy Markdown

Collaborator

Can't hold breath much longer...

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Andrew (anmenaga) commented Aug 30, 2018

Copy link

Copy Markdown

FYI. 18 spelling errors in tools/performance/README.md are causing Travis CI build failure.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Jason Shirk (lzybkr) commented Aug 30, 2018

Copy link

Copy Markdown

Contributor Author

I see 0 spelling errors and have no plans to make the tool happy, so a maintainer can fix or else close the PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Staffan Gustafsson (powercode) commented Aug 30, 2018

Copy link

Copy Markdown

Collaborator

The tool fails on stuff like etl, perfview etc. Only bogus errors.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) approved these changes

Aug 30, 2018

View reviewed changes


          Updated spellings file

5d2c5d2

Jason Shirk (lzybkr) commented Aug 30, 2018

Copy link

Copy Markdown

Contributor Author

Staffan Gustafsson (@powercode) - yes, I reviewed what it complained about, it also complained about analyze I think.

But my point - we have humans reviewing and didn't complain. The tool should not block PRs.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) commented Aug 30, 2018

Copy link

Copy Markdown

Member

Updated the spellings file. Once CI is complete, I will merge.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Aditya Patwardhan (adityapatwardhan) self-assigned this

Aug 30, 2018

Aditya Patwardhan (adityapatwardhan) merged commit 944d716 into PowerShell:master

Aug 30, 2018

Aditya Patwardhan (adityapatwardhan) commented Aug 30, 2018

Copy link

Copy Markdown

Member

Jason Shirk (@lzybkr) Thanks for your contribution!!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Jason Shirk (lzybkr) deleted the perf_tools branch

August 30, 2018 23:52

Travis Plunk (TravisEz13) pushed a commit that referenced this pull request

Aug 31, 2018


          Add tools for PowerShell perf analysis (#7595)

eae6d50

Travis Plunk (TravisEz13) added this to the v6.1.0 milestone

Aug 31, 2018

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Add tools for PowerShell perf analysis (PowerShell#7595)

d307dff

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request

Aug 6, 2026


          Add tools for PowerShell perf analysis (PowerShell#7595)

57ba402

Reviewers

Aditya Patwardhan (adityapatwardhan)

adityapatwardhan approved these changes

Andrew (anmenaga)

Awaiting requested review from anmenaga

Dan Travison (dantraMSFT)

Awaiting requested review from dantraMSFT

Dongbo Wang (daxian-dbw)

Awaiting requested review from daxian-dbw

Travis Plunk (TravisEz13)

Awaiting requested review from TravisEz13

+1 more reviewer

Kyle Sabo (Robo210)

Robo210 left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

Aditya Patwardhan (adityapatwardhan)

Labels

None yet

Projects

None yet

Milestone

v6.1.0

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

Jason Shirk (lzybkr) commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

Staffan Gustafsson (powercode) commented Aug 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jason Shirk (lzybkr) commented Aug 22, 2018

Uh oh!

Staffan Gustafsson (powercode) commented Aug 30, 2018

Uh oh!

Andrew (anmenaga) commented Aug 30, 2018

Uh oh!

Jason Shirk (lzybkr) commented Aug 30, 2018

Uh oh!

Staffan Gustafsson (powercode) commented Aug 30, 2018

Uh oh!

Jason Shirk (lzybkr) commented Aug 30, 2018

Uh oh!

Aditya Patwardhan (adityapatwardhan) commented Aug 30, 2018

Uh oh!

Aditya Patwardhan (adityapatwardhan) commented Aug 30, 2018

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Jason Shirk (lzybkr) commented Aug 21, 2018 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.