bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Two problems with result of num_processors (NPROC_CURRENT_OVERRIDABL


From: Bruno Haible
Subject: Re: Two problems with result of num_processors (NPROC_CURRENT_OVERRIDABLE)
Date: Sun, 26 Feb 2017 18:05:04 +0100
User-agent: KMail/5.1.3 (Linux/4.4.0-64-generic; KDE/5.18.0; x86_64; ; )

Hi Pádraig,

The function 'num_processors' is now a bit large, and in particular contains
the multiplied complexity of
  - the number of systems which each require a different approach,
  - the query parameter which in one MUST consider omp_env_limit and in the
    other case MAY but NEED NOT consider omp_env_limit.

To make this code more maintainable, I would propose to split it into
  - a function which contains only the complexity of the various systems,
  - a function which contains only the complexity of OMP handling.

Like this:


2017-02-26  Bruno Haible  <address@hidden>

        nproc: Refactor large function.
        * lib/nproc.c (num_processors_ignoring_omp): New function, extracted
        from num_processors.
        (num_processors): In this function, only deal with OMP.

diff --git a/lib/nproc.c b/lib/nproc.c
index db3ca9b..7880e62 100644
--- a/lib/nproc.c
+++ b/lib/nproc.c
@@ -199,65 +199,12 @@ num_processors_via_affinity_mask (void)
   return 0;
 }
 
-/* Parse OMP environment variables without dependence on OMP.
-   Return 0 for invalid values.  */
-static unsigned long int
-parse_omp_threads (char const* threads)
-{
-  unsigned long int ret = 0;
-
-  if (threads == NULL)
-    return ret;
-
-  /* The OpenMP spec says that the value assigned to the environment variables
-     "may have leading and trailing white space".  */
-  while (*threads != '\0' && c_isspace (*threads))
-    threads++;
 
-  /* Convert it from positive decimal to 'unsigned long'.  */
-  if (c_isdigit (*threads))
-    {
-      char *endptr = NULL;
-      unsigned long int value = strtoul (threads, &endptr, 10);
-
-      if (endptr != NULL)
-        {
-          while (*endptr != '\0' && c_isspace (*endptr))
-            endptr++;
-          if (*endptr == '\0')
-            return value;
-          /* Also accept the first value in a nesting level,
-             since we can't determine the nesting level from env vars.  */
-          else if (*endptr == ',')
-            return value;
-        }
-    }
-
-  return ret;
-}
-
-unsigned long int
-num_processors (enum nproc_query query)
+/* Return the total number of processors.  Here QUERY must be one of
+   NPROC_ALL, NPROC_CURRENT.  The result is guaranteed to be at least 1.  */
+static unsigned long int
+num_processors_ignoring_omp (enum nproc_query query)
 {
-  unsigned long int omp_env_limit = ULONG_MAX;
-
-  if (query == NPROC_CURRENT_OVERRIDABLE)
-    {
-      unsigned long int omp_env_threads;
-      /* Honor the OpenMP environment variables, recognized also by all
-         programs that are based on OpenMP.  */
-      omp_env_threads = parse_omp_threads (getenv ("OMP_NUM_THREADS"));
-      omp_env_limit = parse_omp_threads (getenv ("OMP_THREAD_LIMIT"));
-      if (! omp_env_limit)
-        omp_env_limit = ULONG_MAX;
-
-      if (omp_env_threads)
-        return MIN (omp_env_threads, omp_env_limit);
-
-      query = NPROC_CURRENT;
-    }
-  /* Here query is one of NPROC_ALL, NPROC_CURRENT.  */
-
   /* On systems with a modern affinity mask system call, we have
          sysconf (_SC_NPROCESSORS_CONF)
             >= sysconf (_SC_NPROCESSORS_ONLN)
@@ -279,7 +226,7 @@ num_processors (enum nproc_query query)
         unsigned long nprocs = num_processors_via_affinity_mask ();
 
         if (nprocs > 0)
-          return MIN (nprocs, omp_env_limit);
+          return nprocs;
       }
 
 #if defined _SC_NPROCESSORS_ONLN
@@ -287,7 +234,7 @@ num_processors (enum nproc_query query)
            Cygwin, Haiku.  */
         long int nprocs = sysconf (_SC_NPROCESSORS_ONLN);
         if (nprocs > 0)
-          return MIN (nprocs, omp_env_limit);
+          return nprocs;
       }
 #endif
     }
@@ -330,7 +277,7 @@ num_processors (enum nproc_query query)
         if (query == NPROC_CURRENT)
           {
             if (psd.psd_proc_cnt > 0)
-              return MIN (psd.psd_proc_cnt, omp_env_limit);
+              return psd.psd_proc_cnt;
           }
         else
           {
@@ -351,7 +298,7 @@ num_processors (enum nproc_query query)
              ? MP_NAPROCS
              : MP_NPROCS);
     if (nprocs > 0)
-      return MIN (nprocs, omp_env_limit);
+      return nprocs;
   }
 #endif
 
@@ -367,7 +314,7 @@ num_processors (enum nproc_query query)
     if (sysctl (mib, ARRAY_SIZE (mib), &nprocs, &len, NULL, 0) == 0
         && len == sizeof (nprocs)
         && 0 < nprocs)
-      return MIN (nprocs, omp_env_limit);
+      return nprocs;
   }
 #endif
 
@@ -376,9 +323,73 @@ num_processors (enum nproc_query query)
     SYSTEM_INFO system_info;
     GetSystemInfo (&system_info);
     if (0 < system_info.dwNumberOfProcessors)
-      return MIN (system_info.dwNumberOfProcessors, omp_env_limit);
+      return system_info.dwNumberOfProcessors;
   }
 #endif
 
   return 1;
 }
+
+/* Parse OMP environment variables without dependence on OMP.
+   Return 0 for invalid values.  */
+static unsigned long int
+parse_omp_threads (char const* threads)
+{
+  unsigned long int ret = 0;
+
+  if (threads == NULL)
+    return ret;
+
+  /* The OpenMP spec says that the value assigned to the environment variables
+     "may have leading and trailing white space".  */
+  while (*threads != '\0' && c_isspace (*threads))
+    threads++;
+
+  /* Convert it from positive decimal to 'unsigned long'.  */
+  if (c_isdigit (*threads))
+    {
+      char *endptr = NULL;
+      unsigned long int value = strtoul (threads, &endptr, 10);
+
+      if (endptr != NULL)
+        {
+          while (*endptr != '\0' && c_isspace (*endptr))
+            endptr++;
+          if (*endptr == '\0')
+            return value;
+          /* Also accept the first value in a nesting level,
+             since we can't determine the nesting level from env vars.  */
+          else if (*endptr == ',')
+            return value;
+        }
+    }
+
+  return ret;
+}
+
+unsigned long int
+num_processors (enum nproc_query query)
+{
+  unsigned long int omp_env_limit = ULONG_MAX;
+
+  if (query == NPROC_CURRENT_OVERRIDABLE)
+    {
+      unsigned long int omp_env_threads;
+      /* Honor the OpenMP environment variables, recognized also by all
+         programs that are based on OpenMP.  */
+      omp_env_threads = parse_omp_threads (getenv ("OMP_NUM_THREADS"));
+      omp_env_limit = parse_omp_threads (getenv ("OMP_THREAD_LIMIT"));
+      if (! omp_env_limit)
+        omp_env_limit = ULONG_MAX;
+
+      if (omp_env_threads)
+        return MIN (omp_env_threads, omp_env_limit);
+
+      query = NPROC_CURRENT;
+    }
+  /* Here query is one of NPROC_ALL, NPROC_CURRENT.  */
+  {
+    unsigned long nprocs = num_processors_ignoring_omp (query);
+    return MIN (nprocs, omp_env_limit);
+  }
+}




reply via email to

[Prev in Thread] Current Thread [Next in Thread]