8159073: : Error handling incomplete when creating GC threads lazily
authorjmasa
Wed, 08 Jun 2016 14:11:51 -0700
changeset 40096 246c62cd9180
parent 40094 b6bc61803957
child 40097 7f2107220cce
8159073: : Error handling incomplete when creating GC threads lazily Reviewed-by: drwhite, tschatzl, sangheki
hotspot/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp
hotspot/src/share/vm/gc/cms/parNewGeneration.cpp
hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp
hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp
hotspot/src/share/vm/gc/parallel/gcTaskManager.cpp
hotspot/src/share/vm/gc/parallel/gcTaskManager.hpp
hotspot/src/share/vm/gc/parallel/psScavenge.cpp
hotspot/src/share/vm/gc/shared/workerManager.hpp
hotspot/src/share/vm/gc/shared/workgroup.cpp
hotspot/src/share/vm/gc/shared/workgroup.hpp
hotspot/src/share/vm/runtime/globals.hpp
hotspot/test/gc/stress/TestGCOld.java
--- a/hotspot/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp	Wed Jun 08 14:11:51 2016 -0700
@@ -2888,7 +2888,10 @@
 
       CMSParInitialMarkTask tsk(this, &srs, n_workers);
       initialize_sequential_subtasks_for_young_gen_rescan(n_workers);
-      if (n_workers > 1) {
+      // If the total workers is greater than 1, then multiple workers
+      // may be used at some time and the initialization has been set
+      // such that the single threaded path cannot be used.
+      if (workers->total_workers() > 1) {
         workers->run_task(&tsk);
       } else {
         tsk.work(0);
@@ -3507,7 +3510,7 @@
   uint num_workers = AdaptiveSizePolicy::calc_active_conc_workers(conc_workers()->total_workers(),
                                                                   conc_workers()->active_workers(),
                                                                   Threads::number_of_non_daemon_threads());
-  conc_workers()->set_active_workers(num_workers);
+  num_workers = conc_workers()->update_active_workers(num_workers);
 
   CompactibleFreeListSpace* cms_space  = _cmsGen->cmsSpace();
 
--- a/hotspot/src/share/vm/gc/cms/parNewGeneration.cpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/cms/parNewGeneration.cpp	Wed Jun 08 14:11:51 2016 -0700
@@ -898,7 +898,7 @@
        AdaptiveSizePolicy::calc_active_workers(workers->total_workers(),
                                                workers->active_workers(),
                                                Threads::number_of_non_daemon_threads());
-  workers->set_active_workers(active_workers);
+  active_workers = workers->update_active_workers(active_workers);
   _old_gen = gch->old_gen();
 
   // If the next generation is too full to accommodate worst-case promotion
@@ -952,7 +952,9 @@
     // separate thread causes wide variance in run times.  We can't help this
     // in the multi-threaded case, but we special-case n=1 here to get
     // repeatable measurements of the 1-thread overhead of the parallel code.
-    if (active_workers > 1) {
+    // Might multiple workers ever be used?  If yes, initialization
+    // has been done such that the single threaded path should not be used.
+    if (workers->total_workers() > 1) {
       workers->run_task(&tsk);
     } else {
       tsk.work(0);
--- a/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/g1/g1CollectedHeap.cpp	Wed Jun 08 14:11:51 2016 -0700
@@ -1331,7 +1331,7 @@
         AdaptiveSizePolicy::calc_active_workers(workers()->total_workers(),
                                                 workers()->active_workers(),
                                                 Threads::number_of_non_daemon_threads());
-      workers()->set_active_workers(n_workers);
+      workers()->update_active_workers(n_workers);
 
       ParRebuildRSTask rebuild_rs_task(this);
       workers()->run_task(&rebuild_rs_task);
@@ -3067,7 +3067,7 @@
     uint active_workers = AdaptiveSizePolicy::calc_active_workers(workers()->total_workers(),
                                                                   workers()->active_workers(),
                                                                   Threads::number_of_non_daemon_threads());
-    workers()->set_active_workers(active_workers);
+    workers()->update_active_workers(active_workers);
 
     TraceCollectorStats tcs(g1mm()->incremental_collection_counters());
     TraceMemoryManagerStats tms(false /* fullGC */, gc_cause());
--- a/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Wed Jun 08 14:11:51 2016 -0700
@@ -1031,11 +1031,14 @@
   uint active_workers = MAX2(1U, parallel_marking_threads());
   assert(active_workers > 0, "Should have been set");
 
+  // Setting active workers is not guaranteed since fewer
+  // worker threads may currently exist and more may not be
+  // available.
+  active_workers = _parallel_workers->update_active_workers(active_workers);
   // Parallel task terminator is set in "set_concurrency_and_phase()"
   set_concurrency_and_phase(active_workers, true /* concurrent */);
 
   G1CMConcurrentMarkingTask markingTask(this, cmThread());
-  _parallel_workers->set_active_workers(active_workers);
   _parallel_workers->run_task(&markingTask);
   print_stats();
 }
--- a/hotspot/src/share/vm/gc/parallel/gcTaskManager.cpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/parallel/gcTaskManager.cpp	Wed Jun 08 14:11:51 2016 -0700
@@ -537,7 +537,7 @@
         created_workers() - active_workers() - idle_workers();
       if (more_inactive_workers < 0) {
         int reduced_active_workers = active_workers() + more_inactive_workers;
-        set_active_workers(reduced_active_workers);
+        update_active_workers(reduced_active_workers);
         more_inactive_workers = 0;
       }
       log_trace(gc, task)("JT: %d  workers %d  active  %d  idle %d  more %d",
--- a/hotspot/src/share/vm/gc/parallel/gcTaskManager.hpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/parallel/gcTaskManager.hpp	Wed Jun 08 14:11:51 2016 -0700
@@ -457,11 +457,12 @@
   uint workers() const {
     return _workers;
   }
-  void set_active_workers(uint v) {
+  uint update_active_workers(uint v) {
     assert(v <= _workers, "Trying to set more workers active than there are");
     _active_workers = MIN2(v, _workers);
     assert(v != 0, "Trying to set active workers to 0");
     _active_workers = MAX2(1U, _active_workers);
+    return _active_workers;
   }
   // Sets the number of threads that will be used in a collection
   void set_active_gang();
--- a/hotspot/src/share/vm/gc/parallel/psScavenge.cpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/parallel/psScavenge.cpp	Wed Jun 08 14:11:51 2016 -0700
@@ -391,11 +391,15 @@
       ParallelTaskTerminator terminator(
         active_workers,
                   (TaskQueueSetSuper*) promotion_manager->stack_array_depth());
-      if (active_workers > 1) {
-        for (uint j = 0; j < active_workers; j++) {
-          q->enqueue(new StealTask(&terminator));
+        // If active_workers can exceed 1, add a StrealTask.
+        // PSPromotionManager::drain_stacks_depth() does not fully drain its
+        // stacks and expects a StealTask to complete the draining if
+        // ParallelGCThreads is > 1.
+        if (gc_task_manager()->workers() > 1) {
+          for (uint j = 0; j < active_workers; j++) {
+            q->enqueue(new StealTask(&terminator));
+          }
         }
-      }
 
       gc_task_manager()->execute_and_wait(q);
     }
--- a/hotspot/src/share/vm/gc/shared/workerManager.hpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/shared/workerManager.hpp	Wed Jun 08 14:11:51 2016 -0700
@@ -55,18 +55,29 @@
     uint start = created_workers;
     uint end = MIN2(active_workers, total_workers);
     for (uint worker_id = start; worker_id < end; worker_id += 1) {
-      WorkerThread* new_worker = holder->install_worker(worker_id);
-      assert(new_worker != NULL, "Failed to allocate GangWorker");
+      WorkerThread* new_worker = NULL;
+      if (initializing || !InjectGCWorkerCreationFailure) {
+        new_worker = holder->install_worker(worker_id);
+      }
       if (new_worker == NULL || !os::create_thread(new_worker, worker_type)) {
+        log_trace(gc, task)("WorkerManager::add_workers() : "
+                            "creation failed due to failed allocation of native %s",
+                            new_worker == NULL ?  "memory" : "thread");
+        if (new_worker != NULL) {
+           delete new_worker;
+        }
         if (initializing) {
-          vm_exit_out_of_memory(0, OOM_MALLOC_ERROR,
-                  "Cannot create worker GC thread. Out of system resources.");
+          vm_exit_out_of_memory(0, OOM_MALLOC_ERROR, "Cannot create worker GC thread. Out of system resources.");
         }
+        break;
       }
       created_workers++;
       os::start_thread(new_worker);
     }
 
+    log_trace(gc, task)("WorkerManager::add_workers() : "
+                        "created_workers: %u", created_workers);
+
     return created_workers;
   }
 
--- a/hotspot/src/share/vm/gc/shared/workgroup.cpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/shared/workgroup.cpp	Wed Jun 08 14:11:51 2016 -0700
@@ -274,8 +274,10 @@
             "Trying to execute task %s with %u workers which is more than the amount of total workers %u.",
             task->name(), num_workers, total_workers());
   guarantee(num_workers > 0, "Trying to execute task %s with zero workers", task->name());
-  add_workers(num_workers, false);
+  uint old_num_workers = _active_workers;
+  update_active_workers(num_workers);
   _dispatcher->coordinator_execute_on_workers(task, num_workers);
+  update_active_workers(old_num_workers);
 }
 
 AbstractGangWorker::AbstractGangWorker(AbstractWorkGang* gang, uint id) {
--- a/hotspot/src/share/vm/gc/shared/workgroup.hpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/gc/shared/workgroup.hpp	Wed Jun 08 14:11:51 2016 -0700
@@ -156,15 +156,14 @@
     return _active_workers;
   }
 
-  void set_active_workers(uint v) {
+  uint update_active_workers(uint v) {
     assert(v <= _total_workers,
            "Trying to set more workers active than there are");
     _active_workers = MIN2(v, _total_workers);
     add_workers(false /* exit_on_failure */);
     assert(v != 0, "Trying to set active workers to 0");
-    assert(UseDynamicNumberOfGCThreads || _active_workers == _total_workers,
-           "Unless dynamic should use total workers");
     log_info(gc, task)("GC Workers: using %d out of %d", _active_workers, _total_workers);
+    return _active_workers;
   }
 
   // Add GC workers as needed.
--- a/hotspot/src/share/vm/runtime/globals.hpp	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/src/share/vm/runtime/globals.hpp	Wed Jun 08 14:11:51 2016 -0700
@@ -1438,6 +1438,10 @@
           "Dynamically choose the number of parallel threads "              \
           "parallel gc will use")                                           \
                                                                             \
+  diagnostic(bool, InjectGCWorkerCreationFailure, false,                    \
+             "Inject thread creation failures for "                         \
+             "UseDynamicNumberOfGCThreads")                                 \
+                                                                            \
   diagnostic(bool, ForceDynamicNumberOfGCThreads, false,                    \
           "Force dynamic selection of the number of "                       \
           "parallel threads parallel gc will use to aid debugging")         \
--- a/hotspot/test/gc/stress/TestGCOld.java	Fri Jul 29 08:17:43 2016 +0000
+++ b/hotspot/test/gc/stress/TestGCOld.java	Wed Jun 08 14:11:51 2016 -0700
@@ -1,5 +1,5 @@
 /*
-* Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved.
 * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
 *
 * This code is free software; you can redistribute it and/or modify it
@@ -33,6 +33,7 @@
  * @run main/othervm -Xmx384M -XX:+UseConcMarkSweepGC TestGCOld 50 1 20 10 10000
  * @run main/othervm -Xmx384M -XX:+UseG1GC TestGCOld 50 1 20 10 10000
  * @run main/othervm -Xms64m -Xmx128m -XX:+UseG1GC -XX:+UseDynamicNumberOfGCThreads -Xlog:gc,gc+task=trace TestGCOld 50 5 20 1 5000
+ * @run main/othervm -Xms64m -Xmx128m -XX:+UseG1GC -XX:+UseDynamicNumberOfGCThreads  -XX:+UnlockDiagnosticVMOptions -XX:+InjectGCWorkerCreationFailure -Xlog:gc,gc+task=trace TestGCOld 50 5 20 1 5000
  */
 
 import java.text.*;