add os.cpu_count()

[storchaka_gmail.com]

http://bugs.python.org/review/17914/diff/8118/Lib/multiprocessing/__init__.py
File Lib/multiprocessing/__init__.py (right):

http://bugs.python.org/review/17914/diff/8118/Lib/multiprocessing/__init__.py...
Lib/multiprocessing/__init__.py:89: if num == None:
if num is None:

http://bugs.python.org/review/17914/diff/8118/Lib/os.py
File Lib/os.py (right):

http://bugs.python.org/review/17914/diff/8118/Lib/os.py#newcode1046
Lib/os.py:1046: return num
What if some method returns num = 0?

http://bugs.python.org/review/17914/diff/8118/Lib/test/test_os.py
File Lib/test/test_os.py (right):

http://bugs.python.org/review/17914/diff/8118/Lib/test/test_os.py#newcode2222
Lib/test/test_os.py:2222: self.assertTrue((type(cpus) is int) or (cpus is None))
These asserts produces low-informative report when failed.

if cpus is not None:
    self.assertIsInstance(cpus, int)
    self.assertGreater(cpus, 0)

[Charles-François Natali]

http://bugs.python.org/review/17914/diff/8127/Lib/os.py
File Lib/os.py (right):

http://bugs.python.org/review/17914/diff/8127/Lib/os.py#newcode1021
Lib/os.py:1021: def cpu_count():
This wrapper is useless, all functions defined in posix are automatically
imported.
This file doesn't warrant any change.

http://bugs.python.org/review/17914/diff/8127/Lib/test/test_os.py
File Lib/test/test_os.py (right):

http://bugs.python.org/review/17914/diff/8127/Lib/test/test_os.py#newcode2222
Lib/test/test_os.py:2222: if cpus is not None:
If cpus is None, it would be better to skip the test with something like:

self.skipTest("Could not determine the number of CPUs")

That way, we can find out if it's not supported on one of the buildbots for
example.

http://bugs.python.org/review/17914/diff/8127/Modules/posixmodule.c
File Modules/posixmodule.c (right):

http://bugs.python.org/review/17914/diff/8127/Modules/posixmodule.c#newcode10337
Modules/posixmodule.c:10337: return Py_None;
Py_None is a CPython object, not a C integer, you can't return it like that.
Also, you should use the Py_RETURN_NONE macro to properly incref it.

The code should probably be refactored to something like:

"""
long ncpus = 0;

#if defined(_SC_NPROCESSORS_ONLN)
    ncpus = sysconf(_SC_NPROCESSORS_ONLN)
#elif defined(MS_WINDOWS)
    ncpus = [...]
#elif defined __APPLE__
    ncpus = [...]
#fi

    if (ncpus >=1 )
        return PyLong_FromLong(ncpus)
    else
        Py_RETURN_NONE
}
"""

[Charles-François Natali]

http://bugs.python.org/review/17914/diff/8136/Lib/os.py
File Lib/os.py (right):

http://bugs.python.org/review/17914/diff/8136/Lib/os.py#newcode28
Lib/os.py:28: import posix
Really, this is useless, all the content of the posix module is automatically
imported (search for posix below).

This file shouldn't be modified *at all*.

http://bugs.python.org/review/17914/diff/8136/Modules/posixmodule.c
File Modules/posixmodule.c (right):

http://bugs.python.org/review/17914/diff/8136/Modules/posixmodule.c#newcode10334
Modules/posixmodule.c:10334: /* We are not returning Py_RETURN_NONE despite
error. This is because this
This comment is useless to anyone familiar with CPython code ;-)

http://bugs.python.org/review/17914/diff/8136/Modules/posixmodule.c#newcode10355
Modules/posixmodule.c:10355: ncpu =
PyLong_FromLong(sysinfo.dwNumberOfProcessors);
This can't work, you're once again assigning a Python object (PyLong_FromLong)
to an int.
Just use:

#ifdef MS_WINDOWS
[...]
ncpu = sysinfo.dwNumberOfProcessors;
#elif __hpux
ncpu = mpctl(MPC_GETNUMSPUS, NULL, NULL);
#elif defined(HAVE_SYSCONF) && defined(_SC_NPROCESSORS_ONLN)
ncpu = sysconf(_SC_NPROCESSORS_ONLN);
[...]
#fi

if (ncpu => 1)
    return PyLong_FromLong(ncpu);
else
    Py_RETURN_NONE
}

Basically, you store the value returned by the platform's library functions to a
C long, then you convert it to a Python integer, if it's a valid number (i.e.
>=1). Otherwise, you return the None Python object.

As is, your code can only compile on Linux.

[Charles-François Natali]

http://bugs.python.org/review/17914/diff/8146/Modules/posixmodule.c
File Modules/posixmodule.c (right):

http://bugs.python.org/review/17914/diff/8146/Modules/posixmodule.c#newcode10317
Modules/posixmodule.c:10317: or Py_None if this value cannot be established.");
Py_None is the internal object Python uses to represent None.
The docstring should use "None".

http://bugs.python.org/review/17914/diff/8146/Modules/posixmodule.c#newcode10329
Modules/posixmodule.c:10329: int mib[4];
The array size should probably be 2 here, since only the first two elements are
used (and 2 is passed to sysctl syscall).

http://bugs.python.org/review/17914/diff/8146/Modules/posixmodule.c#newcode10337
Modules/posixmodule.c:10337: return -1
nitpicking: here you return -1 as failure indicator, but you initialized ncpu to
0 above, and in posix_cpu_count you also initialize it to 0.
A consistent value of 0 or -1 would be more consistent.

http://bugs.python.org/review/17914/diff/8146/Modules/posixmodule.c#newcode10351
Modules/posixmodule.c:10351: #ifndef _SC_NPROCESSORS_ONLN
This part doesn't look correct.
IIUC the goal is to make _SC_NPROCESSORS_ONLN an alias for _SC_NPROC_ONLN on
Irix.
But it's in the __hpux branch, so it won't be defined for the branch:
#elif defined(HAVE_SYSCONF) && defined(_SC_NPROCESSORS_ONLN)

Since those platforms aren't supported anyway, I'd just remove the part from
#elif __hpux
to
#endif /* ! defined(_SC_NPROCESSORS_ONLN)
included

[Charles-François Natali]

Apart from this typo, looks good to me.

I'll commit it next week, unless someone beats me to it.

http://bugs.python.org/review/17914/diff/8169/Modules/posixmodule.c
File Modules/posixmodule.c (right):

http://bugs.python.org/review/17914/diff/8169/Modules/posixmodule.c#newcode10333
Modules/posixmodule.c:10333: err = sysctl(mib, 2, &ncpu, &len, NULL, 0)
You've lost the trailing ';'.

[HenryG]

http://bugs.python.org/review/17914/diff/8198/Lib/test/regrtest.py
File Lib/test/regrtest.py (right):

http://bugs.python.org/review/17914/diff/8198/Lib/test/regrtest.py#newcode513
Lib/test/regrtest.py:513: use_mp = 2 + ncpu
Why the intermediate variable?
  use_mp = 2 + (os.cpu_count() or 1)