changeset 7367:729d098d4ff4

8054491: Remove wrong assert and refactor code in G1CollectorPolicy::record_concurrent_mark_end Reviewed-by: tschatzl, brutisso
author mlarsson
date Mon, 03 Nov 2014 11:29:00 +0100
parents 4aeb83b5657f
children 11003925fa03
files src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp
diffstat 2 files changed, 18 insertions(+), 26 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp	Fri Oct 31 09:10:51 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/g1CollectorPolicy.cpp	Mon Nov 03 11:29:00 2014 +0100
@@ -1585,34 +1585,22 @@
   }
 };
 
+uint G1CollectorPolicy::calculate_parallel_work_chunk_size(uint n_workers, uint n_regions) {
+  assert(n_workers > 0, "Active gc workers should be greater than 0");
+  const uint overpartition_factor = 4;
+  const uint min_chunk_size = MAX2(n_regions / n_workers, 1U);
+  return MAX2(n_regions / (n_workers * overpartition_factor), min_chunk_size);
+}
+
 void
-G1CollectorPolicy::record_concurrent_mark_cleanup_end(int no_of_gc_threads) {
+G1CollectorPolicy::record_concurrent_mark_cleanup_end(uint n_workers) {
   _collectionSetChooser->clear();
 
-  uint region_num = _g1->num_regions();
-  const uint OverpartitionFactor = 4;
-  uint WorkUnit;
-  // The use of MinChunkSize = 8 in the original code
-  // causes some assertion failures when the total number of
-  // region is less than 8.  The code here tries to fix that.
-  // Should the original code also be fixed?
-  if (no_of_gc_threads > 0) {
-    const uint MinWorkUnit = MAX2(region_num / no_of_gc_threads, 1U);
-    WorkUnit = MAX2(region_num / (no_of_gc_threads * OverpartitionFactor),
-                    MinWorkUnit);
-  } else {
-    assert(no_of_gc_threads > 0,
-      "The active gc workers should be greater than 0");
-    // In a product build do something reasonable to avoid a crash.
-    const uint MinWorkUnit = MAX2(region_num / (uint) ParallelGCThreads, 1U);
-    WorkUnit =
-      MAX2(region_num / (uint) (ParallelGCThreads * OverpartitionFactor),
-           MinWorkUnit);
-  }
-  _collectionSetChooser->prepare_for_par_region_addition(_g1->num_regions(),
-                                                         WorkUnit);
-  ParKnownGarbageTask parKnownGarbageTask(_collectionSetChooser, WorkUnit, (uint) no_of_gc_threads);
-  _g1->workers()->run_task(&parKnownGarbageTask);
+  uint n_regions = _g1->num_regions();
+  uint chunk_size = calculate_parallel_work_chunk_size(n_workers, n_regions);
+  _collectionSetChooser->prepare_for_par_region_addition(n_regions, chunk_size);
+  ParKnownGarbageTask par_known_garbage_task(_collectionSetChooser, chunk_size, n_workers);
+  _g1->workers()->run_task(&par_known_garbage_task);
 
   _collectionSetChooser->sort_regions();
 
--- a/src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp	Fri Oct 31 09:10:51 2014 +0100
+++ b/src/share/vm/gc_implementation/g1/g1CollectorPolicy.hpp	Mon Nov 03 11:29:00 2014 +0100
@@ -612,6 +612,10 @@
                                           uint desired_min_length,
                                           uint desired_max_length);
 
+  // Calculate and return chunk size (in number of regions) for parallel
+  // concurrent mark cleanup.
+  uint calculate_parallel_work_chunk_size(uint n_workers, uint n_regions);
+
   // Check whether a given young length (young_length) fits into the
   // given target pause time and whether the prediction for the amount
   // of objects to be copied for the given length will fit into the
@@ -687,7 +691,7 @@
 
   // Record start, end, and completion of cleanup.
   void record_concurrent_mark_cleanup_start();
-  void record_concurrent_mark_cleanup_end(int no_of_gc_threads);
+  void record_concurrent_mark_cleanup_end(uint n_workers);
   void record_concurrent_mark_cleanup_completed();
 
   // Records the information about the heap size for reporting in