Parallel-marker code refactoring (replace GC_markers with GC_markers_m1)

* include/private/gc_priv.h (GC_markers): Replace with GC_markers_m1
(which is GC_markers minus one); update the comment.
* mark.c (GC_markers): Likewise.
* mark.c (GC_help_marker): Replace GC_markers with GC_markers_m1.
* pthread_support.c (GC_is_mach_marker, start_mark_threads,
GC_segment_is_thread_stack, GC_fork_child_proc, GC_thr_init): Likewise.
* win32_threads.c (GC_get_next_stack, start_mark_threads,
GC_wait_marker, GC_notify_all_marker, GC_thr_init): Likewise.

diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h
index fed7f2b2374c8715e050d1af678075baa2387c8e..e687a66ba222d7e29483460631168eacede2a14f 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -2203,9 +2203,9 @@ GC_INNER ptr_t GC_store_debug_info(ptr_t p, word sz, const char *str,
   /* than the main garbage collector lock; standard pthreads-based      */
   /* implementations should be sufficient.                              */
 
-  GC_EXTERN int GC_markers;   /* Number of mark threads we would like   */
-                              /* to have.  Includes the initiating      */
-                              /* thread.  Defined in mark.c.            */
+  GC_EXTERN int GC_markers_m1; /* Number of mark threads we would like  */
+                               /* to have excluding the initiating      */
+                               /* thread.  Defined in mark.c.           */
 
   /* The mark lock and condition variable.  If the GC lock is also      */
   /* acquired, the GC lock must be acquired first.  The mark lock is    */

diff --git a/mark.c b/mark.c
index f419736483863be3bca2e51686f5aa650946c322..c8a63ec7de38a6748d9a453f7910fcb7a73fe8d1 100644 (file)
--- a/mark.c
+++ b/mark.c
@@ -1017,7 +1017,7 @@ STATIC void GC_do_local_mark(mse *local_mark_stack, mse *local_top)
 
 #define ENTRIES_TO_GET 5
 
-GC_INNER int GC_markers = 2;    /* Normally changed by thread-library-  */
+GC_INNER int GC_markers_m1 = 1; /* Normally changed by thread-library-  */
                                 /* -specific code.                      */
 
 /* Mark using the local mark stack until the global mark stack is empty */
@@ -1179,7 +1179,7 @@ GC_INNER void GC_help_marker(word my_mark_no)
       GC_wait_marker();
     }
     my_id = GC_helper_count;
-    if (GC_mark_no != my_mark_no || my_id >= (unsigned)GC_markers) {
+    if (GC_mark_no != my_mark_no || my_id > (unsigned)GC_markers_m1) {
       /* Second test is useful only if original threads can also        */
       /* act as helpers.  Under Linux they can't.                       */
       GC_release_mark_lock();

diff --git a/pthread_support.c b/pthread_support.c
index 99c0123946a56401676d115a53e177de603a6b31..02e243c5079a2104ce1d1905f213fd0499c22bcd 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -343,7 +343,7 @@ static ptr_t marker_sp[MAX_MARKERS - 1] = {0};
   GC_INNER GC_bool GC_is_mach_marker(thread_act_t thread)
   {
     int i;
-    for (i = 0; i < GC_markers - 1; i++) {
+    for (i = 0; i < GC_markers_m1; i++) {
       if (marker_mach_threads[i] == thread)
         return TRUE;
     }
@@ -420,19 +420,19 @@ static void start_mark_threads(void)
         }
       }
 #   endif /* HPUX || GC_DGUX386_THREADS */
-    for (i = 0; i < GC_markers - 1; ++i) {
+    for (i = 0; i < GC_markers_m1; ++i) {
       if (0 != REAL_FUNC(pthread_create)(GC_mark_threads + i, &attr,
                               GC_mark_thread, (void *)(word)i)) {
         WARN("Marker thread creation failed, errno = %" WARN_PRIdPTR "\n",
              errno);
         /* Don't try to create other marker threads.    */
-        GC_markers = i + 1;
+        GC_markers_m1 = i;
         if (i == 0) GC_parallel = FALSE;
         break;
       }
     }
     if (GC_print_stats) {
-      GC_log_printf("Started %d mark helper threads\n", GC_markers - 1);
+      GC_log_printf("Started %d mark helper threads\n", GC_markers_m1);
     }
     pthread_attr_destroy(&attr);
 }
@@ -686,7 +686,7 @@ STATIC void GC_remove_all_threads_but_me(void)
 
     GC_ASSERT(I_HOLD_LOCK());
 #   ifdef PARALLEL_MARK
-      for (i = 0; i < GC_markers - 1; ++i) {
+      for (i = 0; i < GC_markers_m1; ++i) {
         if (marker_sp[i] > lo && marker_sp[i] < hi) return TRUE;
 #       ifdef IA64
           if (marker_bsp[i] > lo && marker_bsp[i] < hi) return TRUE;
@@ -720,7 +720,7 @@ STATIC void GC_remove_all_threads_but_me(void)
 
     GC_ASSERT(I_HOLD_LOCK());
 #   ifdef PARALLEL_MARK
-      for (i = 0; i < GC_markers - 1; ++i) {
+      for (i = 0; i < GC_markers_m1; ++i) {
         if (marker_sp[i] > result && marker_sp[i] < bound)
           result = marker_sp[i];
       }
@@ -901,7 +901,7 @@ STATIC void GC_fork_child_proc(void)
 #   ifdef PARALLEL_MARK
       /* Turn off parallel marking in the child, since we are probably  */
       /* just going to exec, and we would have to restart mark threads. */
-        GC_markers = 1;
+        GC_markers_m1 = 0;
         GC_parallel = FALSE;
 #   endif /* PARALLEL_MARK */
     RESTORE_CANCEL(fork_cancel_state);
@@ -1036,27 +1036,27 @@ GC_INNER void GC_thr_init(void)
     WARN("GC_get_nprocs() returned %" WARN_PRIdPTR "\n", GC_nprocs);
     GC_nprocs = 2; /* assume dual-core */
 #   ifdef PARALLEL_MARK
-      GC_markers = 1;
+      GC_markers_m1 = 0; /* but use only one marker */
 #   endif
   } else {
 #  ifdef PARALLEL_MARK
      {
        char * markers_string = GETENV("GC_MARKERS");
        if (markers_string != NULL) {
-         GC_markers = atoi(markers_string);
-         if (GC_markers > MAX_MARKERS) {
+         GC_markers_m1 = atoi(markers_string) - 1;
+         if (GC_markers_m1 >= MAX_MARKERS) {
            WARN("Limiting number of mark threads\n", 0);
-           GC_markers = MAX_MARKERS;
+           GC_markers_m1 = MAX_MARKERS - 1;
          }
        } else {
-         GC_markers = GC_nprocs;
+         GC_markers_m1 = GC_nprocs - 1;
 #        ifdef GC_MIN_MARKERS
            /* This is primarily for targets without getenv().   */
-           if (GC_markers < GC_MIN_MARKERS)
-             GC_markers = GC_MIN_MARKERS;
+           if (GC_markers_m1 < GC_MIN_MARKERS - 1)
+             GC_markers_m1 = GC_MIN_MARKERS - 1;
 #        endif
-         if (GC_markers >= MAX_MARKERS)
-           GC_markers = MAX_MARKERS; /* silently limit GC_markers value */
+         if (GC_markers_m1 >= MAX_MARKERS)
+           GC_markers_m1 = MAX_MARKERS - 1; /* silently limit the value */
        }
      }
 #  endif
@@ -1065,9 +1065,9 @@ GC_INNER void GC_thr_init(void)
     if (GC_print_stats) {
       GC_log_printf(
                 "Number of processors = %d, number of marker threads = %d\n",
-                GC_nprocs, GC_markers);
+                GC_nprocs, GC_markers_m1 + 1);
     }
-    if (GC_markers <= 1) {
+    if (GC_markers_m1 <= 0) {
       GC_parallel = FALSE;
       if (GC_print_stats) {
         GC_log_printf("Single marker thread, turning off parallel marking\n");

diff --git a/win32_threads.c b/win32_threads.c
index 257f04912f592dc7cf9f6e8c1d1d3b18387cc579..5a642cb181d3a8154f1402df48b7c400edd17dc0 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -1464,7 +1464,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       }
     }
 #   ifdef PARALLEL_MARK
-      for (i = 0; i < GC_markers - 1; ++i) {
+      for (i = 0; i < GC_markers_m1; ++i) {
         ptr_t s = marker_sp[i];
 #       ifdef IA64
           /* FIXME: not implemented */
@@ -1574,7 +1574,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
 #   endif
 
     /* start_mark_threads() is the same as in pthread_support.c except for: */
-    /* - GC_markers value is adjusted already;                              */
+    /* - GC_markers_m1 value is adjusted already;                              */
     /* - thread stack is assumed to be large enough; and                    */
     /* - statistics about the number of marker threads is printed outside.  */
     static void start_mark_threads(void)
@@ -1588,13 +1588,13 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       if (0 != pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED))
         ABORT("pthread_attr_setdetachstate failed");
 
-      for (i = 0; i < GC_markers - 1; ++i) {
+      for (i = 0; i < GC_markers_m1; ++i) {
         marker_last_stack_min[i] = ADDR_LIMIT;
         if (0 != pthread_create(&new_thread, &attr,
                                 GC_mark_thread, (void *)(word)i)) {
           WARN("Marker thread creation failed.\n", 0);
           /* Don't try to create other marker threads.    */
-          GC_markers = i + 1;
+          GC_markers_m1 = i;
           if (i == 0) GC_parallel = FALSE;
           break;
         }
@@ -1737,7 +1737,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
 #     ifdef DONT_USE_SIGNALANDWAIT
         /* Initialize GC_marker_cv[] and GC_marker_Id[] fully before    */
         /* starting the first helper thread.                            */
-        for (i = 0; i < GC_markers - 1; ++i) {
+        for (i = 0; i < GC_markers_m1; ++i) {
           GC_marker_Id[i] = GetCurrentThreadId();
           if ((GC_marker_cv[i] = CreateEvent(NULL /* attrs */,
                                         TRUE /* isManualReset */,
@@ -1747,7 +1747,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
         }
 #     endif
 
-      for (i = 0; i < GC_markers - 1; ++i) {
+      for (i = 0; i < GC_markers_m1; ++i) {
         marker_last_stack_min[i] = ADDR_LIMIT;
 #       ifdef MSWINCE
           /* There is no _beginthreadex() in WinCE. */
@@ -1778,14 +1778,14 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
 #       endif
       }
 
-      /* Adjust GC_markers (and free unused resources) in case of failure. */
+      /* Adjust GC_markers_m1 (and free unused resources) if failed.    */
 #     ifdef DONT_USE_SIGNALANDWAIT
-        while (GC_markers > i + 1) {
-          GC_markers--;
-          CloseHandle(GC_marker_cv[GC_markers - 1]);
+        while (GC_markers_m1 > i) {
+          GC_markers_m1--;
+          CloseHandle(GC_marker_cv[GC_markers_m1]);
         }
 #     else
-        GC_markers = i + 1;
+        GC_markers_m1 = i;
 #     endif
       if (i == 0) {
         GC_parallel = FALSE;
@@ -1904,7 +1904,8 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       {
         HANDLE event = mark_cv;
         DWORD thread_id = GetCurrentThreadId();
-        int i = GC_markers - 1;
+        int i = GC_markers_m1;
+
         while (i-- > 0) {
           if (GC_marker_Id[i] == thread_id) {
             event = GC_marker_cv[i];
@@ -1923,7 +1924,8 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       GC_INNER void GC_notify_all_marker(void)
       {
         DWORD thread_id = GetCurrentThreadId();
-        int i = GC_markers - 1;
+        int i = GC_markers_m1;
+
         while (i-- > 0) {
           /* Notify every marker ignoring self (for efficiency).  */
           if (SetEvent(GC_marker_Id[i] != thread_id ? GC_marker_cv[i] :
@@ -2267,20 +2269,20 @@ GC_INNER void GC_thr_init(void)
   GC_ASSERT(sb_result == GC_SUCCESS);
 
 # if defined(PARALLEL_MARK)
-    /* Set GC_markers. */
+    /* Set GC_markers_m1. */
     {
       char * markers_string = GETENV("GC_MARKERS");
       if (markers_string != NULL) {
-        GC_markers = atoi(markers_string);
-        if (GC_markers > MAX_MARKERS) {
+        GC_markers_m1 = atoi(markers_string) - 1;
+        if (GC_markers_m1 >= MAX_MARKERS) {
           WARN("Limiting number of mark threads\n", 0);
-          GC_markers = MAX_MARKERS;
+          GC_markers_m1 = MAX_MARKERS - 1;
         }
       } else {
 #       ifdef MSWINCE
           /* There is no GetProcessAffinityMask() in WinCE.     */
           /* GC_sysinfo is already initialized.                 */
-          GC_markers = (int)GC_sysinfo.dwNumberOfProcessors;
+          GC_markers_m1 = (int)GC_sysinfo.dwNumberOfProcessors - 1;
 #       else
 #         ifdef _WIN64
             DWORD_PTR procMask = 0;
@@ -2297,15 +2299,15 @@ GC_INNER void GC_thr_init(void)
               ncpu++;
             } while ((procMask &= procMask - 1) != 0);
           }
-          GC_markers = ncpu;
+          GC_markers_m1 = ncpu - 1;
 #       endif
 #       ifdef GC_MIN_MARKERS
           /* This is primarily for testing on systems without getenv(). */
-          if (GC_markers < GC_MIN_MARKERS)
-            GC_markers = GC_MIN_MARKERS;
+          if (GC_markers_m1 < GC_MIN_MARKERS - 1)
+            GC_markers_m1 = GC_MIN_MARKERS - 1;
 #       endif
-        if (GC_markers >= MAX_MARKERS)
-          GC_markers = MAX_MARKERS; /* silently limit GC_markers value  */
+        if (GC_markers_m1 >= MAX_MARKERS)
+          GC_markers_m1 = MAX_MARKERS - 1; /* silently limit the value */
       }
     }
 
@@ -2316,7 +2318,7 @@ GC_INNER void GC_thr_init(void)
         HMODULE hK32;
         /* SignalObjectAndWait() API call works only under NT.          */
 #     endif
-      if (GC_win32_dll_threads || GC_markers <= 1
+      if (GC_win32_dll_threads || GC_markers_m1 <= 0
 #         if !defined(GC_PTHREADS_PARAMARK) && !defined(MSWINCE) \
                 && !defined(DONT_USE_SIGNALANDWAIT)
             || GC_wnt == FALSE
@@ -2327,7 +2329,7 @@ GC_INNER void GC_thr_init(void)
          ) {
         /* Disable parallel marking. */
         GC_parallel = FALSE;
-        GC_markers = 1;
+        GC_markers_m1 = 0;
       } else {
 #       ifndef GC_PTHREADS_PARAMARK
           /* Initialize Win32 event objects for parallel marking.       */
@@ -2357,7 +2359,7 @@ GC_INNER void GC_thr_init(void)
     /* If we are using a parallel marker, actually start helper threads. */
     if (GC_parallel) start_mark_threads();
     if (GC_print_stats) {
-      GC_log_printf("Started %d mark helper threads\n", GC_markers - 1);
+      GC_log_printf("Started %d mark helper threads\n", GC_markers_m1);
     }
 # endif
 }