add os.cpu_count()

@pitrou

BPO	17914
Nosy	@pitrou, @vstinner, @giampaolo, @nedbat, @mark-summerfield, @tpn, @ezio-melotti, @bitdancer, @serhiy-storchaka, @kushaldas, @sdrees
Files	issue17914.patch: Patch for adding os.cpu_count() issue17914-1.patch: Modified patch using C implementation of cpu_count() issue17914-2.patch issue17914-3.patch: Modified patch to return None from C code issue17914-4.patch: Patch modified to use proper macro, remove os.py and skiptest buildbots cpu_count.patch issue17914-5.patch: removed os.py, tyepcasting and redundant comments from previous patch issue17914-6.patch: Change array size to 2, return value to 0(in C code), and removed IRIX issue17914-7.patch use_cpu_count.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-06-28.18:21:53.122>
created_at = <Date 2013-05-06.11:06:47.771>
labels = ['easy', 'type-feature', 'expert-2to3']
title = 'add os.cpu_count()'
updated_at = <Date 2014-12-12.12:36:49.737>
user = 'https://bugs.python.org/neologix'

bugs.python.org fields:

activity = <Date 2014-12-12.12:36:49.737>
actor = 'mark'
assignee = 'none'
closed = True
closed_date = <Date 2013-06-28.18:21:53.122>
closer = 'neologix'
components = ['2to3 (2.x to 3.x conversion tool)']
creation = <Date 2013-05-06.11:06:47.771>
creator = 'neologix'
dependencies = []
files = ['30217', '30220', '30223', '30226', '30236', '30241', '30246', '30282', '30284', '30319']
hgrepos = []
issue_num = 17914
keywords = ['patch', 'easy']
message_count = 60.0
messages = ['188506', '188507', '188509', '188514', '188515', '188516', '188522', '188605', '188606', '188624', '188626', '188627', '188628', '188644', '188905', '188906', '188907', '188909', '188910', '188911', '188912', '188918', '188934', '188944', '188949', '188951', '188953', '188957', '188961', '188963', '188967', '188968', '189053', '189058', '189076', '189079', '189098', '189099', '189104', '189112', '189169', '189170', '189173', '189179', '189180', '189184', '189199', '189360', '189372', '189381', '189654', '189656', '189658', '189670', '189672', '189673', '192005', '232524', '232529', '232541']
nosy_count = 14.0
nosy_names = ['pitrou', 'vstinner', 'giampaolo.rodola', 'nedbat', 'mark', 'trent', 'ezio.melotti', 'r.david.murray', 'neologix', 'python-dev', 'serhiy.storchaka', 'kushal.das', 'dilettant', 'Yogesh.Chaudhari']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17914'
versions = ['Python 3.4']

multiprocessing.cpu_count() implementation should be made available in the os module, where it belongs.

Note that I think it might be interesting to return 1 if the actual value cannot be determined, since that's a sensible default, and likely what the caller will do anyway. This contrasts with the current multiprocessing's implementation which raised NotImplementedError.

If you can't determine the number of CPUs, return a clear "can't determine" value, such as 0 or -1. Returning 1 will hide information, and it's an easy default for the caller to apply if they want to.

I am interested to submit a patch on this. Should I move the implementation to os module and made the multiprocessing one as an alias ? or keep it in both places ?

I prefer the idea of returning -1 instead of the current way of raising NotImplementedError in case we can not determine the number of CPU(s).

I am interested to submit a patch on this. Should I move the implementation to os module and made the multiprocessing one as an alias ? or keep it in both places ?

Yes, you should move it, add a corresponding documentation to
Doc/modules/os.rst (you can probably reuse the multiprocessing doc),
and add a test in Lib/test/test_os.py (you can also probably reuse the
multiprocessing test).

I prefer the idea of returning -1 instead of the current way of raising NotImplementedError in case we can not determine the number of CPU(s).

Seriously, I don't see what this brings. Since the user can't do
anything except using 1 instead, why not do this in the library?
I've searched a bit, and other platforms (.e.g Java, Ruby) don't raise
an exception, and always return a positive value.

Seriously, return zero, and I can use it as: cpu_count = os.cpu_count() or 1

Why throw away information?

Returning 0 or None sounds better to me than 1 or -1.
(I have a preference for None)

I also vote +1 for returning None when the information is unknown.

Just write "os.cpu_count() or 1" if you need 1 when the count is unknown ;-)

See also bpo-17444, Trent Nelson wrote an implementation of os.cpu_count().

I also vote +1 for returning None when the information is unknown.

I still don't like it.
If a function returns a number of CPU, it should either return an
integer >= 1, or raise an exception.
None is *not* an integer.

And returning an exception is IMO useles, since the user can't do
anything with anyway, other than fallback to 1.

Just write "os.cpu_count() or 1" if you need 1 when the count is unknown ;-)

os.cpu_count() or 1 is an ugly idiom.

See also bpo-17444, Trent Nelson wrote an implementation of os.cpu_count().

I don't see exactly what this C implementation brings over the one in
multiprocessing (which is written in Python)?

I don't see exactly what this C implementation brings over the one
in multiprocessing (which is written in Python)?

multiprocessing.cpu_count() creates a subprocess on BSD and Darwin to get the number of CPU. Calling sysctl() or sysctlnametomib() should be faster and use less memory.

On Windows, GetSystemInfo() is called instead of reading an environment variable. I suppose that this function is more reliable.

Trent's os.cpu_count() returns -1 if the count cannot be read, multiprocessing.cpu_count() raises NotImplementedError.

Fair enough, I guess we should use it then.

We just have to agree on the value to return when the number of CPUs
can't be determined ;-)

Returning None sounds reasonable to me.
Raising an exception pretty much means that the function should always be called in a try/except (unless you are sure that the code is running on an OS that knows the number of CPUs). Returning -1 is not very Pythonic, and between 0 and None I prefer the latter, since it's IMHO a clearer indication that the value couldn't be determined.

Minor modifications based on review comments.

Change mib array size to 2,
return value set to 0 consistently (in C code), and
removed IRIX #defines

Typo fix

+1 for returning None.
I haven't looked into patches but if needed feel free to borrow some code from psutil:

Linux:
https://code.google.com/p/psutil/source/browse/psutil/_pslinux.py?spec=svn30f3c67322f99ab30ed87205245dc8394f89f0ac&r=c970f35bc9640ac32eb9f09de8c230e7f86a2466#44

BSD / OSX:
https://code.google.com/p/psutil/source/browse/psutil/_psutil_bsd.c?spec=svn30f3c67322f99ab30ed87205245dc8394f89f0ac&r=9b6e780ea6b598a785670c2626c7557f9fef9238#486

Windows:
https://code.google.com/p/psutil/source/browse/psutil/_psutil_mswindows.c?spec=svn30f3c67322f99ab30ed87205245dc8394f89f0ac&r=4d5b0de27024e9d3cd6a3573a493290498afa9c2#426

SunOS:
https://code.google.com/p/psutil/source/browse/psutil/_pssunos.py?spec=svnff76a4e33da359162c28f8c7478f9e6c6dff347b&name=sunos&r=d53e11edfbe18d22f4e08168f72b1952cfaef373#27

New changeset 5e0c56557390 by Charles-Francois Natali in branch 'default':
Issue bpo-17914: Add os.cpu_count(). Patch by Yogesh Chaudhari, based on an
http://hg.python.org/cpython/rev/5e0c56557390

Alright, committed.
Yogesh, thanks for the patch!

I'm attaching a patch to replace several occurrences of
multiprocessing.cpu_count() by os.cpu_count() in the stdlib/test
suite.

In my patch cpu_count.patch, I changed posix_cpu_count():

rewrite Mac OS X implementation: code in 5e0c56557390 looks wrong. It gets a MIB but then don't use it when calling _bsd_cpu_count(). But I didn't check my patch nor the commit version on Mac OS X.
use "int ncpu;" instead of "long ncpu;" when calling mpctl() and sysctl(). For mpctl(), ncpu is used to store the result, so a wide type is ok. But for sysctl(), we pass a pointer. What happens if sysctl() expects int whereas we pass a pointer to a long? We announce sizeof(int)!?
inline _bsd_cpu_count()

Sorry for this late review.

New changeset a85ac58e9eaf by Charles-Francois Natali in branch 'default':
Issue bpo-17914: Remove OS-X special-case, and use the correct int type.
http://hg.python.org/cpython/rev/a85ac58e9eaf

New changeset f9d815522cdb by Charles-Francois Natali in branch 'default':
Issue bpo-17914: We can now inline _bsd_cpu_count().
http://hg.python.org/cpython/rev/f9d815522cdb

rewrite Mac OS X implementation: code in 5e0c56557390 looks wrong. It
gets a MIB but then don't use it when calling _bsd_cpu_count(). But I didn't
check my patch nor the commit version on Mac OS X.

Indeed.
I just removed the OS-X special case altogether.
Apparently, the standard sysctl is supposed to work os OS-X.
We'll see what happens.

use "int ncpu;" instead of "long ncpu;" when calling mpctl() and
sysctl(). For mpctl(), ncpu is used to store the result, so a wide type is
ok. But for sysctl(), we pass a pointer. What happens if sysctl() expects
int whereas we pass a pointer to a long? We announce sizeof(int)!?

Ouch. This was overlooked when the type was changed to long.
I fixed this.

inline _bsd_cpu_count()

Done in a subsequent commit (especially since the OS-X special case
has been removed).

New changeset 6a0437adafbd by Charles-François Natali in branch 'default':
Issue bpo-17914: Use os.cpu_count() instead of multiprocessing.cpu_count() where
http://hg.python.org/cpython/rev/6a0437adafbd

In message
http://bugs.python.org/issue17914#msg188626
Victor Stenner says

"On Windows, GetSystemInfo() is called instead of reading an environment variable. I suppose that this function is more reliable."

From my reading, and based on feedback from one of my customers, I believe he is correct and that GetSystemInfo() ought to be used on Windows. (It is available in pywin32 win32api.)

From my reading, and based on feedback from one of my customers, I believe he is correct and that GetSystemInfo() ought to be used on Windows. (It is available in pywin32 win32api.)

Please open a new issue to suggest this enhancement, this issue is closed.

Since this is closed I've created a new issue as requested:
http://bugs.python.org/issue23037

Navigation Menu

Repository navigation

add os.cpu_count() #62114

35 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

add os.cpu_count() #62114

Description

Activity

neologix commented on May 6, 2013

neologix commented on May 6, 2013

nedbat commented on May 6, 2013

kushaldas commented on May 6, 2013

neologix commented on May 6, 2013

nedbat commented on May 6, 2013

pitrou commented on May 6, 2013

vstinner commented on May 6, 2013

vstinner commented on May 6, 2013

neologix commented on May 7, 2013

vstinner commented on May 7, 2013

neologix commented on May 7, 2013

ezio-melotti commented on May 7, 2013

35 remaining items

YogeshChaudhari commented on May 16, 2013

YogeshChaudhari commented on May 16, 2013

giampaolo commented on May 16, 2013

python-dev commented on May 20, 2013

neologix commented on May 20, 2013

vstinner commented on May 20, 2013

python-dev commented on May 20, 2013

python-dev commented on May 20, 2013

neologix commented on May 20, 2013

python-dev commented on Jun 28, 2013

mark-summerfield commented on Dec 12, 2014

vstinner commented on Dec 12, 2014

mark-summerfield commented on Dec 12, 2014

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions