-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Patch cpu count physical #1727
Patch cpu count physical #1727
Conversation
I track the commit history of method 1, it's not a perfect solution to delete it simply.
Maybe we should combine the two. |
That would solve the issue on systems with more than one socket, but not in systems with Hyper Threading, as But you are right in that the first method solved an existing problem. I believe that |
…r more than one processor
8f5e7ee
to
122174a
Compare
With the new changes, Without Hyper Threading:
With Hyper Threading:
|
I see very similar problem on 2 socket system in which I have a VM: In [2]: psutil.cpu_count()
Out[2]: 64
In [3]: psutil.cpu_count(logical=False)
Out[3]: 1 where I see:
So I see |
And
|
Thanks @jandrovins for this PR! I've just tested it out on our dual-socket CPU E5-2660 v3 machine on Linux. It looks like the number of cores is now correctly reported, taking hyperthreading into account. Though these CPUs support hyperthreading ( $ uname -a
Linux node14 3.11.10-25-default #1 SMP Wed Dec 17 17:57:03 UTC 2014 (8210f77) x86_64 x86_64 x86_64 GNU/Linux
$ cat /proc/cpuinfo | grep processor | tail -n 1
processor : 19 Old behaviour: >>> import psutil
>>> psutil.cpu_count() # correct, by accident? i.e. guessing that HT is turned on, and so doubling the core count for single socket
20
>>> psutil.cpu_count(logical=False)
10
>>> psutil._pslinux.cpu_count_physical()
10 With this PR, everything seems correct: >>> import psutil
>>> psutil.cpu_count()
20
>>> psutil.cpu_count(logical=False)
20
>>> psutil._pslinux.cpu_count_physical()
20
|
I have also tested the new method 1 on dual socket machines both with and without HT. It returns correct values under CentOS 6 and 7. |
This looks to be ready to merge. Am I missing a reason it hasn't been? |
@jandrovins will this patch also fix issue 1808 that I reported yesterday? |
Could you try it @ewilts? I've just tested on a dual-socket Xeon Gold 6244 (8 cores, 16 threads per socket) machine, and the current psutil release gives 12 (?!) and 32 for Installing from this patch |
… physical cores
[cpu][linux] Fix #849 implement giampaolo/psutil#1727 and test Counts against lscpu
If you wanna know whether the processor running your code is utilizing hyperthreading and to then act accordingly, you can define your own function.
|
This doesn't work for multi-socket CPUs (as discussed above). If you have hyperthreading disabled and 2 sockets (a common configuration) then you get different answers, even with HT disabled. |
There are not a lot of people in these days who have more than 1 physical CPU on their motherboard. That was once a thing, but now it isn't anymore (since the multicore CPUs came into market). And even if one has a multisocket CPU, one must have Windows 10 Pro (the Home edition supports only single-socket CPUs). I know that we want to detect every possible scenario, but if one has Windows 10 Home, then you can be sure that only 1 physical CPU is being used (albeit one has 2 or more physical CPUs). I am googling around for a way to detect whether a multisocket CPU is utilized by the system. Haven't found anything useful yet. |
For home computers, you're probably right. For data centers, this is absolutely false.
Or Linux, which is where I tripped over this bug and where most of this thread has been focused. |
This is precisely one of the issues that this PR solves. |
When is the new version of |
@all-involved: I trust this PR fixes the problem which unfortunately I cannot reproduce or investigate due to lack of time. Before merging though, I would like to make sure that also "method 2" is not broken: Lines 630 to 651 in ba0c0ab
"method 2" is used if /sys/devices/system/cpu/cpu[0-9]*/topology/thread_siblings_list files are not available. What happens if "method 1" is removed/commented-out? Does "method 2" give the expected results? Can somebody try?
|
tested on CentOS 6 and 7, both method 1 and 2 independently return the correct number of physical cores across two sockets. |
Let me get this straight. So does the Also, the name of the function Do you find any value in this? |
Using cat /proc/cpuinfo and lscpu as standins, it looks like physical_id is physical socket, core id is specific core number, and cpu cores the number of physical (non-hyperthreaded) cores per physical socket. bash-4.2# lscpu |grep -E 'CPU|Thread|Core|Socket' |
It's supposed to return physical CPU cores. And yes, it's a bit misleading. To summarize:
The number of CPU sockets is currently not implemented. We can debate whether to add it but:
|
I agree this functionality is missing in psutil, and would be interested in having it.
|
Note that
|
Merged. I also committed bfae1fc which takes into account that |
This has always been cause of confusion, e.g. see: #1727 (comment) Removed the reference to "physical" from dostrings, functions and test. I still left it in the doc though, as it's more explanatory. Signed-off-by: Giampaolo Rodola <[email protected]>
Solves #1620
Method 1 of
cpu_count_physical()
fails on several scenarios. I tested in a node with two sockets:Each of these values is added to a
set
, so this function would end up returning half the physical cores. And in my PC with 2 physical cores with Hyper Threadding:This function returned twice the physical cores.