Skip to content
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.

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

Windows Setup doc; offer 10x perf improvement #58259

Merged
merged 5 commits into from
Jul 30, 2019

Conversation

petemounce
Copy link
Contributor

SUMMARY

Add a section to the Windows Setup documentation about how to get a 10x performance improvement over cleanly installed Windows.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

lib/ansible/plugins/connection/ssh.py
lib/ansible/plugins/shell/powershell.py

ADDITIONAL INFORMATION

We found that when we did this, tasks that were taking ~6s dropped to low hundreds-of-milliseconds.

One playbook (over winrm) went from 25m35s to 8m9s. SSH connection benefits similarly.

Thought you might like to offer it to users, and this seemed like an appropriate place.

All credit to @ca-johnson for the diligent profiling work he did!

@ansibot
Copy link
Contributor

ansibot commented Jun 24, 2019

@petemounce This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Jun 24, 2019

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. performance support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community labels Jun 24, 2019
Copy link
Member

@nitzmahone nitzmahone left a comment

Choose a reason for hiding this comment

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

Hrm, I think this is a good thing to have in our docs, but I'm thinking we might need a "troubleshooting / performance" page, rather than plopping it in the middle of the (already scarily large) "initial setup" page... This is only a problem if your install/imaging process has somehow invalidated the NI cache- Powershell is always ngen'd "out of the box" on 2012R2+ in my testing...

@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jun 25, 2019
@petemounce
Copy link
Contributor Author

@nitzmahone happy to do that - please tell me where I should place the new documentation file and how to name it, and I'll slice and dice?

I've observed this optimisation to be necessary with out-of-the-box GCE Windows 2016 images and AWS AMIs (the latter in circa 2015, though; might have moved on, I haven't needed to know in a little while).

@jborean93
Copy link
Contributor

@acozine or @samccann do you have a suggestion for @petemounce to place performance tips and tricks for Windows?

@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Jul 4, 2019
@acozine
Copy link
Contributor

acozine commented Jul 9, 2019

@jborean93 yep!
@petemounce could you create a new file called docs/docsite/rst/user_guide/windows_performance.rst, put this content (and any additions) there, and add it to the TOC for the Windows guide below the windows_dsc entry on the main Windows page? Ping me with questions, here or in #ansible-docs on freenode IRC.

@acozine
Copy link
Contributor

acozine commented Jul 9, 2019

@petemounce see also https://docs.ansible.com/ansible/latest/dev_guide/style_guide/index.html for some pointers/help on creating rst pages.

Peter Mounce added 2 commits July 18, 2019 13:11
We found that when we did this, tasks that were taking ~6s dropped to low hundreds-of-milliseconds.

One playbook (over winrm) went from 25m35s to 8m9s. SSH connection benefits similarly.

Thought you might like to offer it to users, and this seemed like an appropriate place.

All credit to Carl for the diligent profiling work he did!
@petemounce
Copy link
Contributor Author

@acozine @jborean93 - sure; how's this?

@ansibot
Copy link
Contributor

ansibot commented Jul 18, 2019

The test ansible-test sanity --test docs-build [explain] failed with 1 error:

docs/docsite/rst/index.rst:55:0: literal-block-lex-error: Could not lex literal_block as "powershell". Highlighting skipped.

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Jul 18, 2019

@ansibot ansibot removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Jul 18, 2019
@ansibot ansibot added module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. labels Jul 18, 2019
Copy link
Contributor

@jborean93 jborean93 left a comment

Choose a reason for hiding this comment

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

Just a few minor points. I will let @acozine and @samccann comment on the actual text. From a dev perspective this is all good to me.

docs/docsite/rst/user_guide/windows_performance.rst Outdated Show resolved Hide resolved
docs/docsite/rst/user_guide/windows_performance.rst Outdated Show resolved Hide resolved
docs/docsite/rst/user_guide/windows_performance.rst Outdated Show resolved Hide resolved
@acozine
Copy link
Contributor

acozine commented Jul 22, 2019

@petemounce are you still working on this material?

@petemounce
Copy link
Contributor Author

Yes

@jhawkesworth
Copy link
Contributor

Nice, lets have this.
Didn't speed anything up on my vms but good to have documented way to get ngen cache up to date.
It might be nice to present the Optimize-PowershellAssemblies function as a win_shell task but wouldn't want to hold up merging this for that.

@samccann samccann merged commit 24d8e82 into ansible:devel Jul 30, 2019
@samccann
Copy link
Contributor

Thanks @petemounce for the addition to Ansible documentation.

@petemounce
Copy link
Contributor Author

Thanks for review and merge!

@ansible ansible locked and limited conversation to collaborators Aug 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 docs This issue/PR relates to or includes documentation. docsite This issue/PR relates to the documentation website. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. performance support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants