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

[yln]

Summary

We can now use Python3. Let's use os.cpu_count() to cleanup this
helper.

[yln]

Does the following mean that Python3 is now okay to use for lit?
https://lists.llvm.org/pipermail/llvm-dev/2020-December/147372.html
https://reviews.llvm.org/D93097

[serge-sans-paille]

In D94734#2499865, @yln wrote:

Does the following mean that Python3 is now okay to use for lit?
https://lists.llvm.org/pipermail/llvm-dev/2020-December/147372.html
https://reviews.llvm.org/D93097

I'd say yes.

[thakis]

Didn't look at the diff, but there's already code in the tree that requires python3 (eg subprocess.DEVNULL in compiler-rt/test/lit.common.cfg.py). So that part is fine.

[jdenny]

llvm/utils/lit/lit/util.py

118–128

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.

[yln]

Address Joel's comments.

[yln]

llvm/utils/lit/lit/util.py

118–128

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)

[jdenny]

LGTM

[keith]

Looks like this broken a build http://green.lab.llvm.org/green/job/ZorgTests/49794/console

Not sure if that build should be updated to use python3, or this should be reverted

[yln]

Reverting until we can fix the bot (since it's low priority).

llvm/utils/lit/lit/cl_arguments.py

parser.add_argument('--version',
             action='version',
             version='%(prog)s ' + lit.__version__)
 
     parser.add_argument("-j", "--threads", "--workers",
             dest="workers",
             metavar="N",
             help="Number of workers used for testing",
             type=_positive_int,
-            default=lit.util.detectCPUs())
+            default=lit.util.usable_core_count())
     parser.add_argument("--config-prefix",
             dest="configPrefix",
             metavar="NAME",
             help="Prefix for 'lit' config files")
     parser.add_argument("-D", "--param",
             dest="user_params",
             metavar="NAME=VAL",
             help="Add 'NAME' = 'VAL' to the user defined parameters",

llvm/utils/lit/lit/run.py

def _update_test(self, local_test, remote_test):
         local_test.requires = remote_test.requires
         local_test.result = remote_test.result
 
     # TODO(yln): interferes with progress bar
     # Some tests use threads internally, and at least on Linux each of these
     # threads counts toward the current process limit. Try to raise the (soft)
     # process limit so that tests don't fail due to resource exhaustion.
     def _increase_process_limit(self):
-        ncpus = lit.util.detectCPUs()
+        ncpus = lit.util.usable_core_count()
         desired_limit = self.workers * ncpus * 2 # the 2 is a safety factor
 
         # Importing the resource module will likely fail on Windows.
         try:
             import resource
             NPROC = resource.RLIMIT_NPROC
 
             soft_limit, hard_limit = resource.getrlimit(NPROC)

llvm/utils/lit/lit/util.py

def to_unicode(s):
     """
     if isinstance(s, bytes):
         # In Python2, this branch is taken for both 'str' and 'bytes'.
         # In Python3, this branch is taken only for 'bytes'.
         return s.decode('utf-8')
     return s
 
 
-# TODO(yln): multiprocessing.cpu_count()
-# TODO(python3): len(os.sched_getaffinity(0)) and os.cpu_count()
-def detectCPUs():
-    """Detects the number of CPUs on a system.
-
-    Cribbed from pp.
+def usable_core_count():
+    """Return the number of cores the current process can use, if supported.
+    Otherwise, return the total number of cores (like `os.cpu_count()`).
+    Default to 1 if undetermined.
 
     """
-    # 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
+    try:
+        n = len(os.sched_getaffinity(0))
+    except AttributeError:
+        n = os.cpu_count() or 1
+
+    # On Windows, with more than 32 processes, process creation often fails with
+    if platform.system() == 'Windows':
+        return min(n, 32)
+
+    return n
 
 
 def mkdir(path):
     try:
         if platform.system() == 'Windows':
             from ctypes import windll
             from ctypes import GetLastError, WinError