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

Add explicit 'logical' choice to --numprocesses flag #646

Merged
merged 2 commits into from Jun 8, 2021

Conversation

kroeschl
Copy link
Contributor

With the change to physical CPU count in #560, test sets that are run with psutil installed and that are not CPU-bound (in my case, with lots of network activity) take roughly twice as long to execute on hyperthreaded CPUs. Add a new choice logical for flag --numprocesses for use cases like this.

This implementation is a bit weird because of the multiple fallback paths in pytest_xdist_auto_num_workers() which all return the logical CPU count already, but I don't see a better option.

Testing:

  • Ran tox against all but python3.5 on Linux:
$ python3.9 -m tox -p
...
✔ OK linting in 5.201 seconds
✔ OK py38-psutil in 7.751 seconds
✔ OK py39-pytestlatest in 39.017 seconds
✔ OK py38-pytestlatest in 39.792 seconds
✔ OK py38-pytestmain in 42.884 seconds
✔ OK py36-pytestlatest in 48.754 seconds
__________________________ summary __________________________ 
  linting: commands succeeded
  py36-pytestlatest: commands succeeded
  py38-pytestlatest: commands succeeded
  py39-pytestlatest: commands succeeded
  py38-pytestmain: commands succeeded
  py38-psutil: commands succeeded
  congratulations :)
  • Ran a set of tests against this change and saw 12 workers when run with --numprocesses logical on my six-core hyperthreaded CPU.

@kroeschl kroeschl marked this pull request as ready for review March 25, 2021 18:49
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the PR!

Sorry a lot for taking so long, we seem to have missed the notifications on this one.

Please take a look at my comment. 👍

Also, please rebase on the latest master.

changelog/646.feature.rst Outdated Show resolved Hide resolved
This can significantly speed up runs when testing is not CPU-bound.
src/xdist/plugin.py Outdated Show resolved Hide resolved
src/xdist/plugin.py Outdated Show resolved Hide resolved
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks a lot @kroeschl!

@ferndot
Copy link

ferndot commented Jun 8, 2021

@nicoddemus is this going to be released soon? I am attempting to use this for a project.

@RonnyPfannschmidt RonnyPfannschmidt merged commit 7f07d50 into pytest-dev:master Jun 8, 2021
@RonnyPfannschmidt
Copy link
Member

missed this one, thanks for the poke, next release will have it

@ferndot
Copy link

ferndot commented Jun 11, 2021

Thanks @RonnyPfannschmidt! Do you know when that next release will be?

@RonnyPfannschmidt
Copy link
Member

@joshua-s the 2 items i have to complete is full support for git archives (including non-tag archives for git >=2.32) and enhancemens of the errors when using bad pip/setuptools for modern setuptools_scm
ill create a milestone

@RonnyPfannschmidt
Copy link
Member

hmm, #tfw when you respond for the wrong project, definitively time for a weekend

@nicoddemus should we do one early next week

@nicoddemus
Copy link
Member

@nicoddemus should we do one early next week

Sounds good to me. Would you like to do it, or want me to do it?

gcf-merge-on-green bot pushed a commit to GoogleCloudPlatform/python-docs-samples that referenced this pull request Aug 3, 2021
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pytest-xdist](https://togithub.com/pytest-dev/pytest-xdist) | `==2.2.1` -> `==2.3.0` | [![age](https://badges.renovateapi.com/packages/pypi/pytest-xdist/2.3.0/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/pypi/pytest-xdist/2.3.0/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/pypi/pytest-xdist/2.3.0/compatibility-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/pypi/pytest-xdist/2.3.0/confidence-slim/2.2.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>pytest-dev/pytest-xdist</summary>

### [`v2.3.0`](https://togithub.com/pytest-dev/pytest-xdist/blob/master/CHANGELOG.rst#pytest-xdist-230-2021-06-16)

[Compare Source](https://togithub.com/pytest-dev/pytest-xdist/compare/v2.2.1...v2.3.0)

\===============================

## Deprecations and Removals

-   `#&#8203;654 <https://github.com/pytest-dev/pytest-xdist/issues/654>`\_: Python 3.5 is no longer supported.

## Features

-   `#&#8203;646 <https://github.com/pytest-dev/pytest-xdist/issues/646>`\_: Add `--numprocesses=logical` flag, which automatically uses the number of logical CPUs available, instead of physical CPUs with `auto`.

    This is very useful for test suites which are not CPU-bound.

-   `#&#8203;650 <https://github.com/pytest-dev/pytest-xdist/issues/650>`\_: Added new `pytest_handlecrashitem` hook to allow handling and rescheduling crashed items.

## Bug Fixes

-   `#&#8203;421 <https://github.com/pytest-dev/pytest-xdist/issues/421>`\_: Copy the parent process sys.path into local workers, to work around execnet's python -c adding the current directory to sys.path.

-   `#&#8203;638 <https://github.com/pytest-dev/pytest-xdist/issues/638>`\_: Fix issue caused by changing the branch name of the pytest repository.

## Trivial Changes

-   `#&#8203;592 <https://github.com/pytest-dev/pytest-xdist/issues/592>`\_: Replace master with controller where ever possible.

-   `#&#8203;643 <https://github.com/pytest-dev/pytest-xdist/issues/643>`\_: Use 'main' to refer to pytest default branch in tox env names.

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Renovate will not automatically rebase this PR, because other commits have been found.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/GoogleCloudPlatform/python-docs-samples).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants