Re: [PATCH] support for accessing CPU/core count (processor-count)

[Arthur Miller]

Omar Polo <op@omarpolo.com> writes:

> Arthur Miller <arthur.miller@live.com> writes:
>
>> Omar Polo <op@omarpolo.com> writes:
>>
>>> Campbell Barton <ideasman42@gmail.com> writes:
>>>
>>>> Hi, this patch adds support for accessing the number of CPU's / cores
>>>> on a system, matching CPython's multiprocessing.cpu_count() [0].
>>>>
>>>> I've only tested this for Linux, this includes code that should work
>>>> on other platforms, although that would need to be double-checked of
>>>> course.
>>>> For reference I checked CPython / Blender & Stack-overflow [1]
>>>
>>> I can confirm it works on OpenBSD too, but needs tweaking.  Some
>>> comments inline and attaching an updated patch
>>>
>>>> Accessing this information can be useful to automatically detect the
>>>> number of jobs to run.
>>>>
>>>>
>>>> [0]:
>>>> https://docs.python.org/3/library/multiprocessing.html#multiprocessing.cpu_count
>>>> [1]: https://stackoverflow.com/a/3006416/432509
>>>>
>>>> commit 7be53f10f3df3c3183cc97d6bbadb78ebb61e8d2
>>>> Author: Campbell Barton <ideasman42@gmail.com>
>>>> Date:   Sun Oct 10 10:16:47 2021 +1100
>>>> 
>>>>     Support accessing the number of CPU's.
>>>> 
>>>>     Add (processor-count) for accessing the number of cores/CPU's.
>>>> 
>>>> diff --git a/src/emacs.c b/src/emacs.c
>>>> index 866e43fda9..26e2f6b1f2 100644
>>>> --- a/src/emacs.c
>>>> +++ b/src/emacs.c
>>>> @@ -3156,6 +3156,38 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, 
>>>> Sdaemon_initialized, 0, 0, 0,
>>>>    return Qt;
>>>>  }
>>>> 
>>>> +DEFUN ("processor-count", Fprocessor_count, Sprocessor_count, 0, 0, 0,
>>>> +       doc: /* Return the number of CPUs in the system.
>>>> +
>>>> +The value will always be above zero, 1 for unsupported systems.  */)
>>>> +  (void)
>>>> +{
>>>> +  int nproc = -1;
>>>> +#ifdef WINDOWSNT
>>>> +  SYSTEM_INFO info;
>>>> +  GetSystemInfo(&info);
>>>> +  nproc = (int)info.dwNumberOfProcessors;
>>>> +#elif defined (__APPLE__) || \
>>>> +      defined (__OpenBSD__) || \
>>>> +      defined (__FreeBSD__) || \
>>>> +      defined (__NetBSD__) || \
>>>> +      defined (__DragonFly__)
>>>> +  int mib[2];
>>>> +  size_t len;
>>>> +
>>>> +  mib[0] = CTL_HW;
>>>
>>> the #include <sys/sysctl.h> is missing.
>>>
>>>> +  mib[1] = HW_NCPU;
>>>
>>> at least on OpenBSD this should be HW_NCPUONLINE.
>>>
>>> OpenBSD disables hyperthreading by default so HW_NCPU is (almost) always
>>> misleading.  For example, on my machine
>>>
>>>     % uname -a
>>>     OpenBSD venera 7.0 GENERIC.MP#221 amd64
>>>     % sysctl hw.ncpu
>>>     hw.ncpu=8
>>>     % sysctl hw.ncpuonline
>>>     hw.ncpuonline=4
>>>
>>> and this has been the case for a while already (I mean, a couple of
>>> years if not more.)
>>>
>>> I don't have access to other BSDs other than OpenBSD, but judging from
>>> the manpages online.
>>>
>>>  - NetBSD has hw.ncpuonline
>>>  - DragonFly and FreeBSD have only hw.ncpu
>>>  - apple I don't know how to check.  man.apple.com doesn't seem to
>>>    exists ^^"
>>>
>>>> +  len = sizeof(nproc);
>>>> +  sysctl(mib, 2, &nproc, &len, nullptr, 0);
>>>                                   ^^^^^^^
>>> shouldn't this be NULL?
>>>
>>>> +#elif defined (__hpux)
>>>> +  nproc = mpctl(MPC_GETNUMSPUS, NULL, NULL);
>>>> +#elif defined (_SC_NPROCESSORS_ONLN)
>>>> +  nproc = (int)sysconf(_SC_NPROCESSORS_ONLN);
>>>> +#endif
>>>> +
>>>> +  return make_fixnum (MAX(nproc, 1));
>>>
>>> emacs.c:3188:23: warning: implicit declaration of function 'MAX' is invalid 
>>> in C99 [-Wimplicit-function-declaration]
>>>
>>> I'm attaching an updated patch.  Note that I'm not sure if including
>>> sys/types.h and sys/sysctl.h breaks the build on some OS.
>>>
>>> if someone knows how to reduce the number of #ifdefs I'll be glad :)
>>>
>>>> +}
>>>> +
>>>
>>> Cheers,
>>>
>>> Omar Polo
>>>
>>> From 5fe26a73abc60eae7dd2528e3854f2f379bcb4c7 Mon Sep 17 00:00:00 2001
>>> From: Campbell Barton <ideasman42@gmail.com>
>>> Date: Sun, 10 Oct 2021 08:21:01 +0000
>>> Subject: [PATCH] add function to access the number of CPU
>>>
>>> * emacs.c (Fprocessor_count): add processor-count function to access
>>>   the number of CPU
>>> ---
>>>  src/emacs.c | 35 +++++++++++++++++++++++++++++++++++
>>>  1 file changed, 35 insertions(+)
>>>
>>> diff --git a/src/emacs.c b/src/emacs.c
>>> index 866e43fda9..5bd10a7586 100644
>>> --- a/src/emacs.c
>>> +++ b/src/emacs.c
>>> @@ -25,6 +25,8 @@ along with GNU Emacs.  If not, see 
>>> <https://www.gnu.org/licenses/>.  */
>>>  #include <fcntl.h>
>>>  #include <stdlib.h>
>>>  
>>> +#include <sys/types.h>
>>> +#include <sys/sysctl.h>
>>>  #include <sys/file.h>
>>>  #include <sys/stat.h>
>>>  #include <unistd.h>
>>> @@ -3156,6 +3158,38 @@ from the parent process and its tty file 
>>> descriptors.  */)
>>>    return Qt;
>>>  }
>>>  
>>> +DEFUN ("processor-count", Fprocessor_count, Sprocessor_count, 0, 0, 0,
>>> +       doc: /* Return the number of CPUs in the system.
>>> +
>>> +The value will always be above zero, 1 for unsupported systems.  */)
>>> +  (void)
>>> +{
>>> +  int nproc = -1;
>>> +#ifdef WINDOWSNT
>>> +  SYSTEM_INFO info;
>>> +  GetSystemInfo(&info);
>>> +  nproc = (int)info.dwNumberOfProcessors;
>>> +#elif defined (HW_NCPU) || defined (HW_NCPUONLINE)
>>> +  int mib[2];
>>> +  size_t len;
>>> +
>>> +  mib[0] = CTL_HW;
>>> +#ifdef HW_NCPUONLINE
>>> +  mib[1] = HW_NCPUONLINE;
>>> +#else
>>> +  mib[1] = HW_NCPU;
>>> +#endif
>>> +  len = sizeof(nproc);
>>> +  sysctl(mib, 2, &nproc, &len, NULL, 0);
>>> +#elif defined (__hpux)
>>> +  nproc = mpctl(MPC_GETNUMSPUS, NULL, NULL);
>>> +#elif defined (_SC_NPROCESSORS_ONLN)
>>> +  nproc = (int)sysconf(_SC_NPROCESSORS_ONLN);
>>> +#endif
>>> +
>>> +  return make_fixnum (nproc > 1 ? nproc : 1);
>>> +}
>>> +
>>>  void
>>>  syms_of_emacs (void)
>>>  {
>>> @@ -3176,6 +3210,7 @@ syms_of_emacs (void)
>>>    defsubr (&Sinvocation_directory);
>>>    defsubr (&Sdaemonp);
>>>    defsubr (&Sdaemon_initialized);
>>> +  defsubr (&Sprocessor_count);
>>>  
>>>    DEFVAR_LISP ("command-line-args", Vcommand_line_args,
>>>            doc: /* Args passed by shell to Emacs, as a list of strings.
>>
>> What is wrong on just reading this from /cat/cpuinfo on GNU/Linux?
>>
>> Windows has "wmic cpu get NumberOfCores,NumberOfLogicalProcessors" which 
>> works
>> on cmd prompt, which means executable from elisp as a process. I dont know 
>> for
>> apple and *bsds.
>>
>> It is just text that can be parsed directly with elisp, so it is easier to
>> maintain does not need to be compiled etc. I don't see cpu queries as a
>> performance crucial query. Just a thought.
>
> You're right
>
>       syscstl -n hw.ncpuonline
>
> would do it.  I guess that a rationale for doing this in C would be to
> make it easier on the elisp side to use it.  We can try to support every
> OS here instead of leaving the burden on the elisp package authors.
>
> (the same thing applies to proced.  You could implement it by parsing ps
> output I guess, but there are elisp API that calls into C for that.)
>
> But I don't know, I don't really have strong opinions.  I've read a bit
> of code that didn't look right and tried to improve it :)

By the way; I just realize also that native compiler does this, and there is
also a note about exporting already written c function to lisp:

#begin_src emacs-lisp
(declare-function w32-get-nproc "w32.c")
(defvar comp-num-cpus nil)
(defun comp-effective-async-max-jobs ()
  "Compute the effective number of async jobs."
  (if (zerop native-comp-async-jobs-number)
      (or comp-num-cpus
          (setf comp-num-cpus
                ;; FIXME: we already have a function to determine
                ;; the number of processors, see get_native_system_info in 
w32.c.
                ;; The result needs to be exported to Lisp.
                (max 1 (/ (cond ((eq 'windows-nt system-type)
                                 (w32-get-nproc))
                                ((executable-find "nproc")
                                 (string-to-number
                                  (shell-command-to-string "nproc")))
                                ((eq 'berkeley-unix system-type)
                                 (string-to-number
                                  (shell-command-to-string "sysctl -n 
hw.ncpu")))
                                (t 1))
                          2))))
    native-comp-async-jobs-number))
#end_src

Maybe you can just extract the cpu number part from Andrea's function and rename
into some public API independent of native comp?