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

[Campbell Barton]

On Sun, Oct 10, 2021 at 8:38 PM Arthur Miller <arthur.miller@live.com> wrote:
>
> 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.

Agree - in many cases the overhead of launching a process isn't likely
to be an issue,
although some corner cases could end up calling this more often than
expected, causing slowdowns.

In general I've found this kind of code (while it doesn't read so
nicely with all the ifdef's), doesn't cause much maintenance overhead
since it's just querying a value. Once this is working, it only needs
to be touched if API's are ever deprecated (for example).
Also, it seems it's not so trivial to get this info from the command
line on all systems, hpux docs [0] for e.g. suggest calling:
/usr/sbin/ioscan -kf | grep processor | wc -l
... while this can be replicated in elisp, the C version seems more concise.

[0]: https://www.ibm.com/support/pages/determining-how-many-cpus-you-have-hp-ux




--
- Campbell