Skip to content

add os.cpu_count() #62114

Closed
Closed
add os.cpu_count()#62114
Labels
easytopic-2to3type-featureA feature request or enhancement

Description

@neologix
neologix
mannequin
opened on May 6, 2013
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']

    Activity

    1. neologix commented on May 6, 2013

      @neologix
      neologixmannequin
      on May 6, 2013
      MannequinAuthor

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

    2. neologix
      added
      easy
      type-featureA feature request or enhancement
      on May 6, 2013

    3. neologix commented on May 6, 2013

      @neologix
      neologixmannequin
      on May 6, 2013
      MannequinAuthor

      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.

    4. nedbat commented on May 6, 2013

      @nedbat
      nedbat
      on May 6, 2013
      Member

      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.

    5. kushaldas commented on May 6, 2013

      @kushaldas
      kushaldas
      on May 6, 2013
      Member

      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).

    6. neologix commented on May 6, 2013

      @neologix
      neologixmannequin
      on May 6, 2013
      MannequinAuthor

      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.

    7. nedbat commented on May 6, 2013

      @nedbat
      nedbat
      on May 6, 2013
      Member

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

      Why throw away information?

    8. pitrou commented on May 6, 2013

      @pitrou
      pitrou
      on May 6, 2013
      Member

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

    9. vstinner commented on May 6, 2013

      @vstinner
      vstinner
      on May 6, 2013
      Member

      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 ;-)

    10. vstinner commented on May 6, 2013

      @vstinner
      vstinner
      on May 6, 2013
      Member

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

    11. neologix commented on May 7, 2013

      @neologix
      neologixmannequin
      on May 7, 2013
      MannequinAuthor

      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)?

    12. vstinner commented on May 7, 2013

      @vstinner
      vstinner
      on May 7, 2013
      Member

      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.

    13. neologix commented on May 7, 2013

      @neologix
      neologixmannequin
      on May 7, 2013
      MannequinAuthor

      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 ;-)

    14. ezio-melotti commented on May 7, 2013

      @ezio-melotti
      ezio-melotti
      on May 7, 2013
      Member

      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.

    15. 35 remaining items

    16. YogeshChaudhari commented on May 16, 2013

      @YogeshChaudhari
      YogeshChaudharimannequin
      on May 16, 2013
      Mannequin

      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

    17. YogeshChaudhari commented on May 16, 2013

      @YogeshChaudhari
      YogeshChaudharimannequin
      on May 16, 2013
      Mannequin

      Typo fix

    18. giampaolo commented on May 16, 2013

      @giampaolo
      giampaolo
      on May 16, 2013
      Contributor

      +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

    19. python-dev commented on May 20, 2013

      @python-dev
      python-devmannequin
      on May 20, 2013
      Mannequin

      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

    20. neologix commented on May 20, 2013

      @neologix
      neologixmannequin
      on May 20, 2013
      MannequinAuthor

      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.

    21. vstinner commented on May 20, 2013

      @vstinner
      vstinner
      on May 20, 2013
      Member

      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.

    22. python-dev commented on May 20, 2013

      @python-dev
      python-devmannequin
      on May 20, 2013
      Mannequin

      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

    23. python-dev commented on May 20, 2013

      @python-dev
      python-devmannequin
      on May 20, 2013
      Mannequin

      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

    24. neologix commented on May 20, 2013

      @neologix
      neologixmannequin
      on May 20, 2013
      MannequinAuthor
      • 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).

    25. python-dev commented on Jun 28, 2013

      @python-dev
      python-devmannequin
      on Jun 28, 2013
      Mannequin

      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

    26. neologix
      closed this as completedon Jun 28, 2013

    27. mark-summerfield commented on Dec 12, 2014

      @mark-summerfield
      mark-summerfieldmannequin
      on Dec 12, 2014
      Mannequin

      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.)

    28. vstinner commented on Dec 12, 2014

      @vstinner
      vstinner
      on Dec 12, 2014
      Member

      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.

    29. mark-summerfield commented on Dec 12, 2014

      @mark-summerfield
      mark-summerfieldmannequin
      on Dec 12, 2014
      Mannequin

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

    30. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      easytopic-2to3type-featureA feature request or enhancement

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions