diff src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp @ 4711:adedfbbf0360

7120038: G1: ParallelGCThreads==0 is broken Summary: Running G1 with ParallelGCThreads==0 results in various crashes and asserts. Most of these are caused by unguarded references to the worker threads array or an incorrect number of active workers. Reviewed-by: jmasa, tonyp
author johnc
date Fri, 16 Dec 2011 11:40:00 -0800
parents 41406797186b
children 441e946dc1af
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Fri Dec 16 02:14:27 2011 -0500
+++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Fri Dec 16 11:40:00 2011 -0800
@@ -3787,8 +3787,9 @@
         double end_time_sec = os::elapsedTime();
         double pause_time_ms = (end_time_sec - start_time_sec) * MILLIUNITS;
         g1_policy()->record_pause_time_ms(pause_time_ms);
-        int active_gc_threads = workers()->active_workers();
-        g1_policy()->record_collection_pause_end(active_gc_threads);
+        int active_workers = (G1CollectedHeap::use_parallel_gc_threads() ?
+                                workers()->active_workers() : 1);
+        g1_policy()->record_collection_pause_end(active_workers);
 
         MemoryService::track_memory_usage();
 
@@ -5312,8 +5313,10 @@
   int active_workers = (G1CollectedHeap::use_parallel_gc_threads() ?
                         workers()->active_workers() : 1);
 
-  assert(active_workers == workers()->active_workers(),
-         "Need to reset active_workers");
+  assert(!G1CollectedHeap::use_parallel_gc_threads() ||
+           active_workers == workers()->active_workers(),
+           "Need to reset active_workers");
+
   set_par_threads(active_workers);
   G1ParPreserveCMReferentsTask keep_cm_referents(this, active_workers, _task_queues);
 
@@ -5451,13 +5454,13 @@
     assert(UseDynamicNumberOfGCThreads ||
            n_workers == workers()->total_workers(),
            "If not dynamic should be using all the  workers");
+    workers()->set_active_workers(n_workers);
     set_par_threads(n_workers);
   } else {
     assert(n_par_threads() == 0,
            "Should be the original non-parallel value");
     n_workers = 1;
   }
-  workers()->set_active_workers(n_workers);
 
   G1ParTask g1_par_task(this, _task_queues);
 
@@ -5479,6 +5482,7 @@
     workers()->run_task(&g1_par_task);
   } else {
     StrongRootsScope srs(this);
+    g1_par_task.set_for_termination(n_workers);
     g1_par_task.work(0);
   }
 
@@ -5727,8 +5731,8 @@
     // Iterate over the dirty cards region list.
     G1ParCleanupCTTask cleanup_task(ct_bs, this);
 
-    if (ParallelGCThreads > 0) {
-      set_par_threads(workers()->total_workers());
+    if (G1CollectedHeap::use_parallel_gc_threads()) {
+      set_par_threads();
       workers()->run_task(&cleanup_task);
       set_par_threads(0);
     } else {
@@ -6136,8 +6140,9 @@
 void G1CollectedHeap::set_par_threads() {
   // Don't change the number of workers.  Use the value previously set
   // in the workgroup.
+  assert(G1CollectedHeap::use_parallel_gc_threads(), "shouldn't be here otherwise");
   int n_workers = workers()->active_workers();
-    assert(UseDynamicNumberOfGCThreads ||
+  assert(UseDynamicNumberOfGCThreads ||
            n_workers == workers()->total_workers(),
       "Otherwise should be using the total number of workers");
   if (n_workers == 0) {