[PATCH] D94734: [lit] Use os.cpu_count() to cleanup TODO

[Julian Lettner via Phabricator via llvm-commits]

yln marked an inline comment as done.
yln added inline comments.


================
Comment at: llvm/utils/lit/lit/util.py:117
     """
-    # Linux, Unix and MacOS:
-    if hasattr(os, 'sysconf'):
-        if 'SC_NPROCESSORS_ONLN' in os.sysconf_names:
-            # Linux & Unix:
-            ncpus = os.sysconf('SC_NPROCESSORS_ONLN')
-            if isinstance(ncpus, int) and ncpus > 0:
-                return ncpus
-        else:  # OSX:
-            return int(subprocess.check_output(['sysctl', '-n', 'hw.ncpu'],
-                                               stderr=subprocess.STDOUT))
-    # Windows:
-    if 'NUMBER_OF_PROCESSORS' in os.environ:
-        ncpus = int(os.environ['NUMBER_OF_PROCESSORS'])
-        if ncpus > 0:
-            # With more than 32 processes, process creation often fails with
-            # "Too many open files".  FIXME: Check if there's a better fix.
-            return min(ncpus, 32)
-    return 1  # Default
+    n = os.cpu_count() or 1
+
----------------
jdenny wrote:
> I'm comparing the following documentation:
> 
> * <https://www.gnu.org/software/libc/manual/html_node/Processor-Resources.html>
> * <https://docs.python.org/3/library/os.html#os.cpu_count>
> 
> It seems like `SC_NPROCESSORS_ONLN` is closer to `len(os.sched_getaffinity(0))` than `os.cpu_count()`.  I'm not familiar with these routines, so I might be misunderstanding.
That's a good point.  What we are really interested in is the number of usable cores.  Changed function to the following:
`len(os.sched_getaffinity(0))` get number of usable cores on platforms that support this query
`os.cpu_count()` get total number of cores (with a fallback to 1)




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94734/new/

https://reviews.llvm.org/D94734