This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[glibc] Fix Linux sysconf(_SC_NPROCESSORS_[CONF|ONLN]) performance problem


It turns out that apparently libdb (and possibly others) call
sysconf(_SC_NPROCESSORS_ONLN) (aka get_nprocs()) a *lot*.

The claimed reason is that it wants to "optimize" its locking behavior
wrt the number of CPU's in the system, but it's pretty clear that
libdb was assuming that the sysconf() would just return a constant
value or just otherwise be very low-overhead, because what actually
happens on Linux is not an optimization, but instead glibc spending
endless amounts of time in the kernel just generating /proc files
because it tries to parse /proc/stat.

So people who actually know about this just use a LD_PRELOAD thing to
override the slow glibc implementation, while normal people just get
bad performance because they never realize that the system is spending
a quarter of the CPU cycles just generating and parsing /proc/stat
files (that "quarter of the CPU cycles" is a rough estimate from a
real profile of an exim benchmark).

Rather than forcing people to use LD_PRELOAD and take over sysconf(),
or letting normal people down, shouldn't we just make the glibc
implementation better instead?

This patch does that in two different ways:

 - cache the value in a static variable, so that if/when there are
multiple calls, we don't spend time recalculating the number of CPU's
in the system unnecessarily.

 - while the /proc/stat parsing approach remains as a fallback, use
sched_getaffinity() to much more quickly and simply get the number of
processors that this process is allowed to use. That is not only much
simpler, but is actually also potentially more accurate wrt the number
of processors actually available to the process.

I'd argue that the /proc/stat parsing might as well be deleted
entirely, but in trying to keep the patch minimal I kept it as a
fallback.

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

This goes out to a random selection of top committers for the last six
months according to the glibc git tree and to libc-alpha. If there are
better people/lists out there, let me know.

Comments? The caching might be considered technically "wrong", but the
value it caches is badly defined anyway, and the current glibc code
will not even necessarily return consistent values for
_SC_NPROCESSORS_CONF vs _SC_NPROCESSORS_ONLN, so whatever.

If you just want the sched_getaffinity() approach, I can send a
castrated patch instead.

                Linus
 sysdeps/unix/sysv/linux/getsysstats.c |   42 +++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/getsysstats.c b/sysdeps/unix/sysv/linux/getsysstats.c
index af454b650de9..3a36be37e340 100644
--- a/sysdeps/unix/sysv/linux/getsysstats.c
+++ b/sysdeps/unix/sysv/linux/getsysstats.c
@@ -18,6 +18,7 @@
    Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
    02111-1307 USA.  */
 
+#define _GNU_SOURCE 1
 #include <alloca.h>
 #include <assert.h>
 #include <ctype.h>
@@ -36,6 +37,8 @@
 #include <atomic.h>
 #include <not-cancel.h>
 
+#include <sched.h>
+
 
 /* How we can determine the number of available processors depends on
    the configuration.  There is currently (as of version 2.0.21) no
@@ -125,8 +128,8 @@ next_line (int fd, char *const buffer, char **cp, char **re,
 }
 
 
-int
-__get_nprocs ()
+static int
+__get_nprocs_from_proc (void)
 {
   /* XXX Here will come a test for the new system call.  */
 
@@ -171,13 +174,34 @@ __get_nprocs ()
 
   return result;
 }
+
+static int
+__get_nprocs_internal (void)
+{
+  cpu_set_t set;
+
+  if (sched_getaffinity(0, sizeof(cpu_set_t), &set) >= 0)
+    return CPU_COUNT(&set);
+
+  return __get_nprocs_from_proc();
+}
+
+int
+__get_nprocs ()
+{
+  static int nprocs = -1;
+
+  if (nprocs < 0)
+    nprocs = __get_nprocs_internal();
+  return nprocs;
+}
 weak_alias (__get_nprocs, get_nprocs)
 
 
 /* On some architectures it is possible to distinguish between configured
    and active cpus.  */
-int
-__get_nprocs_conf ()
+static int
+__get_nprocs_conf_internal (void)
 {
   /* XXX Here will come a test for the new system call.  */
 
@@ -224,6 +248,16 @@ __get_nprocs_conf ()
 
   return result;
 }
+
+int
+__get_nprocs_conf ()
+{
+  static long nprocs = -1;
+
+  if (nprocs < 0)
+    nprocs = __get_nprocs_conf_internal();
+  return nprocs;
+}
 weak_alias (__get_nprocs_conf, get_nprocs_conf)
 
 /* General function to get information about memory status from proc

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]