[issue17914] add os.cpu_count()
Mark Summerfield added the comment: 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.) -- nosy: +mark ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
STINNER Victor added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Mark Summerfield added the comment: Since this is closed I've created a new issue as requested: http://bugs.python.org/issue23037 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Roundup Robot added the comment: New changeset 6a0437adafbd by Charles-François Natali in branch 'default': Issue #17914: Use os.cpu_count() instead of multiprocessing.cpu_count() where http://hg.python.org/cpython/rev/6a0437adafbd -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Changes by Charles-François Natali cf.nat...@gmail.com: -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Changes by Charles-François Natali cf.nat...@gmail.com: -- stage: needs patch - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Roundup Robot added the comment: New changeset 5e0c56557390 by Charles-Francois Natali in branch 'default': Issue #17914: Add os.cpu_count(). Patch by Yogesh Chaudhari, based on an http://hg.python.org/cpython/rev/5e0c56557390 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment:
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.
--
Added file: http://bugs.python.org/file30319/use_cpu_count.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17914
___diff -r 5e0c56557390 Doc/library/multiprocessing.rst
--- a/Doc/library/multiprocessing.rst Mon May 20 14:40:46 2013 +0200
+++ b/Doc/library/multiprocessing.rst Mon May 20 14:52:18 2013 +0200
@@ -1646,9 +1646,9 @@
callbacks and has a parallel map implementation.
*processes* is the number of worker processes to use. If *processes* is
- ``None`` then the number returned by :func:`cpu_count` is used. If
- *initializer* is not ``None`` then each worker process will call
- ``initializer(*initargs)`` when it starts.
+ ``None`` then the number returned by :func:`os.cpu_count` is used, with a
+ fallback value of 1. If *initializer* is not ``None`` then each worker
+ process will call ``initializer(*initargs)`` when it starts.
.. versionadded:: 3.2
*maxtasksperchild* is the number of tasks a worker process can complete
diff -r 5e0c56557390 Lib/concurrent/futures/process.py
--- a/Lib/concurrent/futures/process.py Mon May 20 14:40:46 2013 +0200
+++ b/Lib/concurrent/futures/process.py Mon May 20 14:52:18 2013 +0200
@@ -331,7 +331,7 @@
_check_system_limits()
if max_workers is None:
-self._max_workers = multiprocessing.cpu_count()
+self._max_workers = os.cpu_count() or 1
else:
self._max_workers = max_workers
diff -r 5e0c56557390 Lib/multiprocessing/pool.py
--- a/Lib/multiprocessing/pool.py Mon May 20 14:40:46 2013 +0200
+++ b/Lib/multiprocessing/pool.py Mon May 20 14:52:18 2013 +0200
@@ -17,10 +17,11 @@
import queue
import itertools
import collections
+import os
import time
import traceback
-from multiprocessing import Process, cpu_count, TimeoutError
+from multiprocessing import Process, TimeoutError
from multiprocessing.util import Finalize, debug
#
@@ -147,10 +148,7 @@
self._initargs = initargs
if processes is None:
-try:
-processes = cpu_count()
-except NotImplementedError:
-processes = 1
+processes = os.cpu_count() or 1
if processes 1:
raise ValueError(Number of processes must be at least 1)
diff -r 5e0c56557390 Lib/test/regrtest.py
--- a/Lib/test/regrtest.py Mon May 20 14:40:46 2013 +0200
+++ b/Lib/test/regrtest.py Mon May 20 14:52:18 2013 +0200
@@ -508,12 +508,9 @@
elif o in ('-j', '--multiprocess'):
use_mp = int(a)
if use_mp = 0:
-try:
-import multiprocessing
-# Use all cores + extras for tests that like to sleep
-use_mp = 2 + multiprocessing.cpu_count()
-except (ImportError, NotImplementedError):
-use_mp = 3
+# Use all cores + extras for tests that like to sleep
+ncpu = os.cpu_count() or 1
+use_mp = 2 + ncpu
if use_mp == 1:
use_mp = None
elif o == '--header':
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
STINNER Victor added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Roundup Robot added the comment: New changeset a85ac58e9eaf by Charles-Francois Natali in branch 'default': Issue #17914: Remove OS-X special-case, and use the correct int type. http://hg.python.org/cpython/rev/a85ac58e9eaf -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Roundup Robot added the comment: New changeset f9d815522cdb by Charles-Francois Natali in branch 'default': Issue #17914: We can now inline _bsd_cpu_count(). http://hg.python.org/cpython/rev/f9d815522cdb -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: * 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). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Minor modifications based on review comments. 1. Change mib array size to 2, 2. return value set to 0 consistently (in C code), and 3. removed IRIX #defines -- Added file: http://bugs.python.org/file30282/issue17914-6.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Typo fix -- Added file: http://bugs.python.org/file30284/issue17914-7.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Giampaolo Rodola' added the comment: +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=svn30f3c67322f99ab30ed87205245dc8394f89f0acr=c970f35bc9640ac32eb9f09de8c230e7f86a2466#44 BSD / OSX: https://code.google.com/p/psutil/source/browse/psutil/_psutil_bsd.c?spec=svn30f3c67322f99ab30ed87205245dc8394f89f0acr=9b6e780ea6b598a785670c2626c7557f9fef9238#486 Windows: https://code.google.com/p/psutil/source/browse/psutil/_psutil_mswindows.c?spec=svn30f3c67322f99ab30ed87205245dc8394f89f0acr=4d5b0de27024e9d3cd6a3573a493290498afa9c2#426 SunOS: https://code.google.com/p/psutil/source/browse/psutil/_pssunos.py?spec=svnff76a4e33da359162c28f8c7478f9e6c6dff347bname=sunosr=d53e11edfbe18d22f4e08168f72b1952cfaef373#27 -- nosy: +giampaolo.rodola ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: @Stinner: 1. While I agree with your idea of what you have done in test_os, (particularly, for determining if platform is supported or not) there seems to be no reason(AFAIK) to have a shutil for cpu_count. I agree with neologox there. 2. Also I am not comfortable with the idea of having multiple 'implementations' of cpu_count. There should be one-- and preferably only one --obvious way to do it. 3. The idea of returning 1 by default does not seem to serve a useful purpose. It should be left to the end-user to decide what needs to be done based on error/actual_value received from system. (+1 to Antoine and nedbat) For eg, a. Let's say someone works on scheduling and power managment modules. It is important to know that the platform does not support providing cpu_count() instead of giving 1. This will ensure that they don't go about erroneously setting wrong options for scheduler and/or overclocking the CPU too much(or too little). b. On the other hand if another user just wants to use a cpu_count number from a his application to determine the number of threads to spawn he can set th = cpu_count() or 1 (on a side note: *usually* for programs that are non IO intensive and require no/little synchronization it is best to spawn cpu_count() number of threads) These are just 2 examples to demonstrate that it must be the end-user who decides what to do with the proper_value or reasonable_error_value given by cpu_count() 4. +1 to Antoine on last comment ;-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Modified patch based on further comments and review. 1. Removed *everything* from os.py 2. removed typecasting for ncpu where not required. 3. removed redundant comments -- Added file: http://bugs.python.org/file30246/issue17914-5.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment:
Just for giggles, here's the glibc default implementation on non Linux
platforms:
http://sourceware.org/git/?p=glibc.git;a=blob;f=misc/getsysstats.c;hb=HEAD
int
__get_nprocs ()
{
/* We don't know how to determine the number. Simply return always 1. */
return 1;
}
And on Linux, 1 is returned as a fallback when you don't have the
right /sys or /proc entry:
http://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/getsysstats.c
(The enum discussion enlighted me, endless discussions are so fun!)
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17914
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Antoine Pitrou added the comment: And on Linux, 1 is returned as a fallback when you don't have the right /sys or /proc entry: http://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/getsysstats.c (The enum discussion enlighted me, endless discussions are so fun!) Do you want cpu_count() to return an enum? os.cpu_count() CPUCount.Four: 4 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Ned Batchelder added the comment: Python's goal is not to emulate the suboptimal parts of other languages. We have dynamic typing, and so can return None from the same function that returns 1. And we have compact expressions like `cpu_count() or 1`, so we don't have to make unfortunate compromises. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment:
Python's goal is not to emulate the suboptimal parts of other languages.
Well, I'm sure they could have returned -1 or 0, which are valid C
long distinct from any valid integer representing a number of CPUs. If
the libc guys (and many other APIs out there ), chose to return 1 as
default, there's a reason.
Furthermore, you're missing the point: since the underlying libraries
os.cpu_count() rely on return 1 when they can't determine the number
of CPUs, why complicate the API by pretending to return None in that
case, since you can't detect it in the first place?
We have dynamic typing, and so can return None from the same function that
returns 1. And we have compact expressions like `cpu_count() or 1`, so we
don't have to make unfortunate compromises.
That's not because it's feasible that it's a good idea.
Dynamic typing is different from no typing: the return type of a
function is part of its API. You can't return None when you're
supposed to return an int. If I go to a coffee machine to get some
chocolate and there's no chocolate left, I don't want it to return me
a coffee instead.
What's looks more natural
if os.cpu_count() is None
or
if os.cpu_count() = 1
Even in dynamic typing, it's always a good thing to be consistent in
parameter and return value type.
Why?
For example, PEP 362 formalizes function signatures.
With a os.cpu_count() returning a number (or eventually raising an
exception), the signature is:
def cpu_count() - int
[...]
What does it become if you can return None instead?
For example, there's some discussion to use such signatures or other
DSL to automatically generate the glue code needed to parse arguments
and return values from C extension modules (PEP 436 and 437).
Basically, you just implement:
/*
** [annotation]
** @return int
*/
long cpu_count_impl(void)
{
long result = 1;
#ifdef _SC_NPROCESSORS_CONF
long = sysconf(_SC_NPROCESSORS_ONL);
[...]
#fi
return result;
}
And the DSL processor takes care of the rest.
What does this become if your return object isn't typed?
Really, typing is of paramount importance, even in a dynamically typed language.
And pretending to return a distinct value is pretty much useless,
since the underlying platform will always return a default value of 1.
Plus `cpu_count() or 1` is really ugly.
Now I hope I made my point, but honestly at this point I don't care
anymore, since in practice it should never return None. Documenting
the None return value is just noise in the API...
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue17914
___
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Antoine Pitrou added the comment: Python's goal is not to emulate the suboptimal parts of other languages. Well, I'm sure they could have returned -1 or 0, which are valid C long distinct from any valid integer representing a number of CPUs. If the libc guys (and many other APIs out there ), chose to return 1 as default, there's a reason. Well, they can be wrong sometimes, too :-) Furthermore, you're missing the point: since the underlying libraries os.cpu_count() rely on return 1 when they can't determine the number of CPUs, why complicate the API by pretending to return None in that case, since you can't detect it in the first place? The patch doesn't seem to rely on the glibc, so we are fine here. Or do the other libs work likewise? And the DSL processor takes care of the rest. What does this become if your return object isn't typed? It's typed, just the type is int or None. I'm sure some statically-typed languages are able to express this (OCaml? Haskell?). Anyway, I don't mind whether it's None or 0 or -42. But let's not hide the information. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Based on the last 3 messages by Ned, Charles and Antoine, I keep thinking that arguments made by Charles are very valid ones and that it would be better to return 1. I say this (partly from the 'type' argument, but), mainly, *if* its known that the underlying libraries are returning 1 on failure. *If* that is the case I see no reason try to return None (which, btw, will never happen if none of the calls return non-positive values ever). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: Well, they can be wrong sometimes, too :-) Indeed, as can I ;-) The patch doesn't seem to rely on the glibc, so we are fine here. Or do the other libs work likewise? sysconf(_SC_NPROCESSORS_CONF) is implemented with the above function in the glibc. For other Unix systems apparently they use the sysctl() syscall, and I don't *think* it can fail to return the number of CPUs. So the only platforms where it could in theory fail are things like Cygwin, etc. And the DSL processor takes care of the rest. What does this become if your return object isn't typed? It's typed, just the type is int or None. I'm sure some statically-typed languages are able to express this (OCaml? Haskell?). I recently started learning Haskell. You have Either and Maybe that could fall into that category. Anyway, I don't mind whether it's None or 0 or -42. But let's not hide the information. I liked your suggestion of making it an enum: os.cpu_count() CPUCount.UnknownCount: 42 Nah, None is fine to me! -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Changes by Stefan Drees ste...@drees.name: -- nosy: +dilettant ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: @STINNER: I don't understand. Where exactly should the patch handle this? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Modified patch based on review by neologix -- Added file: http://bugs.python.org/file30236/issue17914-4.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
STINNER Victor added the comment: Here is a new patch (cpu_count.patch) with a different approach: * add a os.cpu_count() which returns the raw value (may be zero or negative if the OS returns a dummy value) and raise an OSError on error * os.cpu_count() is not available on all platforms * shutil.cpu_count() is the high level API using 1 as a fallback. The fallback value (which is 1 by default) is configurable, ex: shutil.cpu_count(fallback=None) returns None on os.cpu_count() error. * multiprocessing.cpu_count() simply reuses shutil.cpu_count() So os.cpu_count() as a well defined behaviour, and shutil.cpu_count() is the convinient API. My patch is based on issue17914-4.patch and so also on Trent Nelson's code. I only tested my patch on Linux. It must be tested on other platforms. If nobody tests the patch on HPUX, it would be safer to remove HPUX support. It looks like Trent's code was not tested, I don't think that his code works on platforms other than Windows. test_os will fail if os.cpu_count() fails. The test should be fixed to handle failures, but I prefer to start with a failing test to check if the error case occurs on a buildbot. -- Added file: http://bugs.python.org/file30241/cpu_count.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
STINNER Victor added the comment: The return type of the C function sysconf() is long, but Python uses int: I opened the issue #17964 to track this bug. (os.cpu_count() uses sysconf()). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: Here is a new patch (cpu_count.patch) with a different approach: * add a os.cpu_count() which returns the raw value (may be zero or negative if the OS returns a dummy value) and raise an OSError on error * os.cpu_count() is not available on all platforms * shutil.cpu_count() is the high level API using 1 as a fallback. The fallback value (which is 1 by default) is configurable, ex: shutil.cpu_count(fallback=None) returns None on os.cpu_count() error. * multiprocessing.cpu_count() simply reuses shutil.cpu_count() Do we really need cpu_count() in three places with different semantics? Also, it doesn't belong to shutil(), which stands for shell utilities IIRC. IMO just one version in Modules/posixmodule.c is enough (with multiprocessing's version as an alias), there's no need to over-engineer this. It can return None, that's fine with me now at this point. I only tested my patch on Linux. It must be tested on other platforms. If nobody tests the patch on HPUX, it would be safer to remove HPUX support. Well, HP-UX isn't officially supported, so that's reasonable. test_os will fail if os.cpu_count() fails. The test should be fixed to handle failures, but I prefer to start with a failing test to check if the error case occurs on a buildbot. That's reasonable. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Antoine Pitrou added the comment: * add a os.cpu_count() which returns the raw value (may be zero or negative if the OS returns a dummy value) and raise an OSError on error * os.cpu_count() is not available on all platforms * shutil.cpu_count() is the high level API using 1 as a fallback. The fallback value (which is 1 by default) is configurable, ex: shutil.cpu_count(fallback=None) returns None on os.cpu_count() error. -1. This is simply too complicated for a simple API. Just let os.cpu_count() return None. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Based on the conversation and the particular inputs to the thread form neologix and ezio, I would like to submit this patch. It probably needs modification(s) as I am not sure what to do with the implementation that is already present in multiprocessing. This patch simply calls the os.cpu_count() from multiprocessing now and behaves as it would have previously. The test cases are also added to test_os similar to ones from multiprocessing. -- components: +2to3 (2.x to 3.x conversion tool) keywords: +patch nosy: +Yogesh.Chaudhari Added file: http://bugs.python.org/file30217/issue17914.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: Based on the conversation and the particular inputs to the thread form neologix and ezio, I would like to submit this patch. It probably needs modification(s) as I am not sure what to do with the implementation that is already present in multiprocessing. This patch simply calls the os.cpu_count() from multiprocessing now and behaves as it would have previously. Thanks, but it would be better to reuse Trent's C implementation instead of multiprocessing's: http://hg.python.org/sandbox/trent/file/dd1c2fd3aa31/Modules/posixmodule.c#l10213 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Ned Batchelder added the comment: A few small points: Use `num is None` instead of `num == None`. Use `isinstance(cpus, int)` rather than `type(cpus) is int`. And this I think will throw an exception in Python 3: `cpus = 1 or cpus == None`, because you can't compare None to 1. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Serhiy Storchaka added the comment: I think the idiom `os.cpu_count() or 1` should be mentioned in the documentation an officially recommended. Otherwise people will produce a non-portable code which works on their developer's computers but not on exotic platforms. I have added some other comments on Rietveld. -- nosy: +serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Antoine Pitrou added the comment: I agree with Charles-François. An approach using C library functions is far superior to launching external commands. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: I think the idiom `os.cpu_count() or 1` should be mentioned in the documentation an officially recommended. Otherwise people will produce a non-portable code which works on their developer's computers but not on exotic platforms. And I maintain it's an ugly idiom ;-) Since the user can't do anything except falling back to 1, os.cpu_count() should always return a positive number (1 by default). That's AFAICT what all other platforms (Java, Ruby, etc) do, because it makes sense. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Antoine Pitrou added the comment: And I maintain it's an ugly idiom ;-) Since the user can't do anything except falling back to 1, os.cpu_count() should always return a positive number (1 by default). The user can also raise an error. For example, if I'm writing a benchmark to measure per-core scaling performance, I would like to bail out if I can't calculate the number of cores (rather than report incorrect results). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Serhiy Storchaka added the comment: Since the user can't do anything except falling back to 1, os.cpu_count() should always return a positive number (1 by default). In general I agree with you. Actually the os module should contains two functions: cpu_count() which fallbacks to 1 and is_cpu_counting_supported() for rare need. But this looks even more ugly and I choose single function even if in most cases I need use strange idiom. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Appreciate everyone's feedback. I have modified the patch based on further messages in the thread. @Neologix modified posixmodule according to one in Trent's branch and used that for cpu_count(). Kindly suggest improvements/changes if any. @Ned: Thanks for the suggestions. I have applied them wherever applicable. However regarding And this I think will throw an exception in Python 3: `cpus = 1 or cpus == None`, because you can't compare None to 1. It does not throw any exceptions as of now. -- Added file: http://bugs.python.org/file30220/issue17914-1.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Serhiy Storchaka added the comment: Yogesh, didn't you notice comments on Rietveld? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: @Serhiy Sorry, I missed your comments in the thread. I have made 2 changes and ignored the cpu_count() returning 0, because it returns -1 on failure, else give the number of CPUs. Also the test_os, checks for 0 return if that was to e the case. -- Added file: http://bugs.python.org/file30223/issue17914-2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Serhiy Storchaka added the comment: Now we have three cpu_count() functions: multiprocessing.cpu_count() raises an exception on failure, posix.cpu_count() returns -1, and os.cpu_count() returns None. It will be easy to get rid of Python wrapper in the os module and return None directly from C code. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Returning None from C code sounds reasonable to me. Anyone else wants to pitch in with suggestions for/against this? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: Returning None from C code sounds reasonable to me. Anyone else wants to pitch in with suggestions for/against this? Go for it ;-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: Modified patch to return None from C code -- Added file: http://bugs.python.org/file30226/issue17914-3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Ned Batchelder added the comment: @Yogesh: if cpus is None, then this will raise an exception in Python 3: `cpus = 1 or cpus == None` Perhaps you don't have enough test cases yet. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Yogesh Chaudhari added the comment: @Ned: if cpus is None, then this will raise an exception in Python 3: `cpus = 1 or cpus == None` I understand that cpus = INTEGER will raise an exception and have already modified the condition to remove that kind of check. I was merely stating that equality checks do not raise exception. eg: cpus = None cpus == 1 False cpus == None True Thanks for pointing me out in the right direction to remove those invalid checks and showing the use of proper alternatives at other places in the patch -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
STINNER Victor added the comment: Not being able to decide for the default value is not a problem: just an optional default argument, which is 1 by default (most convinient value), return default on error. os.get_terminal_size() has a similar API for example. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: 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 #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)? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
STINNER Victor added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: 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 ;-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Ezio Melotti added the comment: 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. -- nosy: +ezio.melotti ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
R. David Murray added the comment: As for why to not return 1, I can imagine code that checks cpu_count, and only if it returns the don't know result would it invoke some more expensive method of determining the CPU count on platforms that cpu_count doesn't support. Since the os module is the home for close to the metal (well, OS) functions, I agree that it does not make sense to throw away the information that cpu_count can't actually determine the CPU count. Contrawise, I could see the multiprocessing version returning 1, since it is a higher level API and os.cpu_count would be available for those wanting the don't know info. -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
New submission from Charles-François Natali: multiprocessing.cpu_count() implementation should be made available in the os module, where it belongs. -- keywords: easy messages: 188506 nosy: neologix priority: normal severity: normal stage: needs patch status: open type: enhancement versions: Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Ned Batchelder added the comment: 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. -- nosy: +nedbat ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Kushal Das added the comment: 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). -- nosy: +kushaldas ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Charles-François Natali added the comment: 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. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Ned Batchelder added the comment: Seriously, return zero, and I can use it as: cpu_count = os.cpu_count() or 1 Why throw away information? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
Antoine Pitrou added the comment: Returning 0 or None sounds better to me than 1 or -1. (I have a preference for None) -- nosy: +pitrou ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
STINNER Victor added the comment: 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 ;-) -- nosy: +haypo ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue17914] add os.cpu_count()
STINNER Victor added the comment: See also #17444, Trent Nelson wrote an implementation of os.cpu_count(). -- nosy: +trent ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue17914 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
