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.

Sign up for GitHub

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

Jump to bottom

Use psutil.cpu_count(logical=False) in auto_detect_cpus() #560

Merged
merged 4 commits into from Aug 3, 2020
Merged

Use psutil.cpu_count(logical=False) in auto_detect_cpus() #560

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
77e5fdf
Use psutil.cpu_count(logical=False) in auto_detect_cpus()
utapyngo Jul 23, 2020
1c85f18
Add changelog
utapyngo Jul 23, 2020
3312dcd
Apply black
utapyngo Jul 23, 2020
e528a42
Support OpenBSD and NetBSD
utapyngo Jul 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/553.bugfix.rst
View file
Open in desktop
@@ -0,0 +1 @@
When using ``-n auto``, count the number of physical CPU cores instead of logical ones.
8 changes: 7 additions & 1 deletion setup.py
View file
Open in desktop
@@ -1,6 +1,12 @@
from setuptools import setup, find_packages

install_requires = ["execnet>=1.1", "pytest>=4.4.0", "pytest-forked", "six"]
install_requires = [
"execnet>=1.1",
"psutil>=3.0.0",
"pytest>=4.4.0",
"pytest-forked",
"six",
]


with open("README.rst") as f:
Expand Down
19 changes: 2 additions & 17 deletions src/xdist/plugin.py
View file
Open in desktop
@@ -1,28 +1,13 @@
import os
import uuid

import psutil
import py
import pytest


def auto_detect_cpus():
try:
from os import sched_getaffinity
except ImportError:
if os.environ.get("TRAVIS") == "true":
# workaround https://bitbucket.org/pypy/pypy/issues/2375
return 2
try:
from os import cpu_count
except ImportError:
from multiprocessing import cpu_count
else:

def cpu_count():
return len(sched_getaffinity(0))

try:
n = cpu_count()
n = psutil.cpu_count(logical=False)
except NotImplementedError:
Copy link
Member

Choose a reason for hiding this comment

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

is this something that psutil will raise?

Return the number of logical CPUs in the system (same as os.cpu_count in Python 3.4) or None if undetermined. logical cores means the number of physical cores multiplied by the number of threads that can run on each core (this is known as Hyper Threading). If logical is False return the number of physical cores only (Hyper Thread CPUs are excluded) or None if undetermined. On OpenBSD and NetBSD psutil.cpu_count(logical=False) always return None. Example on a system having 2 physical hyper-thread CPU cores:

if it never raises, we can use return psutil.cpu_count(logical=False) or 1

Copy link
Member

Choose a reason for hiding this comment

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

I'm also concerned about:

On OpenBSD and NetBSD psutil.cpu_count(logical=False) always return None.

should we fallback to the old code if we get a None?

Copy link
Member

Choose a reason for hiding this comment

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

should we fallback to the old code if we get a None?

Not sure, we don't officially support OpenBSD and NetBSD... your suggestion above of using return psutil.cpu_count(logical=False) or 1 would at least prevent a crash, and I would rather keep the code cleaner. But if others disagree then I think your suggestion is on point.

Copy link
Member

Choose a reason for hiding this comment

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

Does this obviate the Travis workaround?

Copy link
Member

Choose a reason for hiding this comment

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

Good question. 👍

Copy link
Contributor Author

@utapyngo utapyngo Jul 28, 2020 •
edited

Choose a reason for hiding this comment

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

I tested this locally on pypy35:

image

return 1
return n if n else 1
Expand Down
19 changes: 7 additions & 12 deletions testing/test_plugin.py
View file
Open in desktop
Expand Up @@ -35,17 +35,10 @@ def test_dist_options(testdir):


def test_auto_detect_cpus(testdir, monkeypatch):
import os
import psutil
from xdist.plugin import pytest_cmdline_main as check_options

if hasattr(os, "sched_getaffinity"):
monkeypatch.setattr(os, "sched_getaffinity", lambda _pid: set(range(99)))
elif hasattr(os, "cpu_count"):
monkeypatch.setattr(os, "cpu_count", lambda: 99)
else:
import multiprocessing

monkeypatch.setattr(multiprocessing, "cpu_count", lambda: 99)
monkeypatch.setattr(psutil, "cpu_count", lambda logical=True: 99)

config = testdir.parseconfigure("-n2")
assert config.getoption("numprocesses") == 2
Expand All @@ -58,10 +51,12 @@ def test_auto_detect_cpus(testdir, monkeypatch):
assert config.getoption("usepdb")
assert config.getoption("numprocesses") == 0

monkeypatch.delattr(os, "sched_getaffinity", raising=False)
monkeypatch.setenv("TRAVIS", "true")
def cpu_count_not_implemented(logical=True):
raise NotImplementedError

monkeypatch.setattr(psutil, "cpu_count", cpu_count_not_implemented)
config = testdir.parseconfigure("-nauto")
assert config.getoption("numprocesses") == 2
assert config.getoption("numprocesses") == 1


def test_boxed_with_collect_only(testdir):
Expand Down