8217330: Split G1CollectionSetChooser into collection set candidate container and the chooser algorithm
authortschatzl
Fri, 08 Feb 2019 12:55:20 +0100
changeset 53703 24341625d8f2
parent 53702 50a5d0353570
child 53704 ef72c85a0534
8217330: Split G1CollectionSetChooser into collection set candidate container and the chooser algorithm Reviewed-by: lkorinth, kbarrett
src/hotspot/share/gc/g1/collectionSetChooser.cpp
src/hotspot/share/gc/g1/collectionSetChooser.hpp
src/hotspot/share/gc/g1/g1CollectionSet.cpp
src/hotspot/share/gc/g1/g1CollectionSet.hpp
src/hotspot/share/gc/g1/g1CollectionSetCandidates.cpp
src/hotspot/share/gc/g1/g1CollectionSetCandidates.hpp
src/hotspot/share/gc/g1/g1Policy.cpp
src/hotspot/share/gc/g1/heapRegion.hpp
src/hotspot/share/memory/allocation.cpp
--- a/src/hotspot/share/gc/g1/collectionSetChooser.cpp	Fri Feb 08 12:55:20 2019 +0100
+++ b/src/hotspot/share/gc/g1/collectionSetChooser.cpp	Fri Feb 08 12:55:20 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2019, 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
@@ -25,20 +25,21 @@
 #include "precompiled.hpp"
 #include "gc/g1/collectionSetChooser.hpp"
 #include "gc/g1/g1CollectedHeap.inline.hpp"
+#include "gc/g1/g1CollectionSetCandidates.hpp"
 #include "gc/g1/heapRegionRemSet.hpp"
 #include "gc/shared/space.inline.hpp"
 #include "runtime/atomic.hpp"
+#include "utilities/quickSort.hpp"
 
-// Even though we don't use the GC efficiency in our heuristics as
-// much as we used to, we still order according to GC efficiency. This
-// will cause regions with a lot of live objects and large RSets to
-// end up at the end of the array. Given that we might skip collecting
-// the last few old regions, if after a few mixed GCs the remaining
-// have reclaimable bytes under a certain threshold, the hope is that
-// the ones we'll skip are ones with both large RSets and a lot of
-// live objects, not the ones with just a lot of live objects if we
+// Order regions according to GC efficiency. This will cause regions with a lot
+// of live objects and large remembered sets to end up at the end of the array.
+// Given that we might skip collecting the last few old regions, if after a few
+// mixed GCs the remaining have reclaimable bytes under a certain threshold, the
+// hope is that the ones we'll skip are ones with both large remembered sets and
+// a lot of live objects, not the ones with just a lot of live objects if we
 // ordered according to the amount of reclaimable bytes per region.
 static int order_regions(HeapRegion* hr1, HeapRegion* hr2) {
+  // Make sure that NULL entries are moved to the end.
   if (hr1 == NULL) {
     if (hr2 == NULL) {
       return 0;
@@ -51,6 +52,7 @@
 
   double gc_eff1 = hr1->gc_efficiency();
   double gc_eff2 = hr2->gc_efficiency();
+
   if (gc_eff1 > gc_eff2) {
     return -1;
   } if (gc_eff1 < gc_eff2) {
@@ -60,243 +62,209 @@
   }
 }
 
-static int order_regions(HeapRegion** hr1p, HeapRegion** hr2p) {
-  return order_regions(*hr1p, *hr2p);
-}
+// Determine collection set candidates: For all regions determine whether they
+// should be a collection set candidates, calculate their efficiency, sort and
+// return them as G1CollectionSetCandidates instance.
+// Threads calculate the GC efficiency of the regions they get to process, and
+// put them into some work area unsorted. At the end the array is sorted and
+// copied into the G1CollectionSetCandidates instance; the caller will be the new
+// owner of this object.
+class G1BuildCandidateRegionsTask : public AbstractGangTask {
+
+  // Work area for building the set of collection set candidates. Contains references
+  // to heap regions with their GC efficiencies calculated. To reduce contention
+  // on claiming array elements, worker threads claim parts of this array in chunks;
+  // Array elements may be NULL as threads might not get enough regions to fill
+  // up their chunks completely.
+  // Final sorting will remove them.
+  class G1BuildCandidateArray : public StackObj {
+
+    uint const _max_size;
+    uint const _chunk_size;
+
+    HeapRegion** _data;
+
+    uint volatile _cur_claim_idx;
+
+    // Calculates the maximum array size that will be used.
+    static uint required_array_size(uint num_regions, uint num_workers, uint chunk_size) {
+      uint const max_waste = num_workers * chunk_size;
+      // The array should be aligned with respect to chunk_size.
+      uint const aligned_num_regions = ((num_regions + chunk_size - 1) / chunk_size) * chunk_size;
 
-CollectionSetChooser::CollectionSetChooser() :
-  // The line below is the worst bit of C++ hackery I've ever written
-  // (Detlefs, 11/23).  You should think of it as equivalent to
-  // "_regions(100, true)": initialize the growable array and inform it
-  // that it should allocate its elem array(s) on the C heap.
-  //
-  // The first argument, however, is actually a comma expression
-  // (set_allocation_type(this, C_HEAP), 100). The purpose of the
-  // set_allocation_type() call is to replace the default allocation
-  // type for embedded objects STACK_OR_EMBEDDED with C_HEAP. It will
-  // allow to pass the assert in GenericGrowableArray() which checks
-  // that a growable array object must be on C heap if elements are.
-  //
-  // Note: containing object is allocated on C heap since it is CHeapObj.
-  //
-  _regions((ResourceObj::set_allocation_type((address) &_regions,
-                                             ResourceObj::C_HEAP),
-                  100), true /* C_Heap */),
-    _front(0), _end(0), _first_par_unreserved_idx(0),
-    _region_live_threshold_bytes(0), _remaining_reclaimable_bytes(0) {
-  _region_live_threshold_bytes = mixed_gc_live_threshold_bytes();
-}
+      return aligned_num_regions + max_waste;
+    }
+
+  public:
+    G1BuildCandidateArray(uint max_num_regions, uint num_workers, uint chunk_size) :
+      _max_size(required_array_size(max_num_regions, num_workers, chunk_size)),
+      _chunk_size(chunk_size),
+      _data(NEW_C_HEAP_ARRAY(HeapRegion*, _max_size, mtGC)),
+      _cur_claim_idx(0) {
+      for (uint i = 0; i < _max_size; i++) {
+        _data[i] = NULL;
+      }
+    }
+
+    ~G1BuildCandidateArray() {
+      FREE_C_HEAP_ARRAY(HeapRegion*, _data);
+    }
+
+    // Claim a new chunk, returning its bounds [from, to[.
+    void claim_chunk(uint& from, uint& to) {
+      uint result = Atomic::add(_chunk_size, &_cur_claim_idx);
+      assert(_max_size > result - 1,
+             "Array too small, is %u should be %u with chunk size %u.",
+             _max_size, result, _chunk_size);
+      from = result - _chunk_size;
+      to = result;
+    }
+
+    // Set element in array.
+    void set(uint idx, HeapRegion* hr) {
+      assert(idx < _max_size, "Index %u out of bounds %u", idx, _max_size);
+      assert(_data[idx] == NULL, "Value must not have been set.");
+      _data[idx] = hr;
+    }
 
-#ifndef PRODUCT
-void CollectionSetChooser::verify() {
-  guarantee(_end <= regions_length(), "_end: %u regions length: %u", _end, regions_length());
-  guarantee(_front <= _end, "_front: %u _end: %u", _front, _end);
-  uint index = 0;
-  size_t sum_of_reclaimable_bytes = 0;
-  while (index < _front) {
-    guarantee(regions_at(index) == NULL,
-              "all entries before _front should be NULL");
-    index += 1;
-  }
-  HeapRegion *prev = NULL;
-  while (index < _end) {
-    HeapRegion *curr = regions_at(index++);
-    guarantee(curr != NULL, "Regions in _regions array cannot be NULL");
-    guarantee(!curr->is_young(), "should not be young!");
-    guarantee(!curr->is_pinned(),
-              "Pinned region should not be in collection set (index %u)", curr->hrm_index());
-    if (prev != NULL) {
-      guarantee(order_regions(prev, curr) != 1,
-                "GC eff prev: %1.4f GC eff curr: %1.4f",
-                prev->gc_efficiency(), curr->gc_efficiency());
+    void sort_and_copy_into(HeapRegion** dest, uint num_regions) {
+      if (_cur_claim_idx == 0) {
+        return;
+      }
+      for (uint i = _cur_claim_idx; i < _max_size; i++) {
+        assert(_data[i] == NULL, "must be");
+      }
+      QuickSort::sort(_data, _cur_claim_idx, order_regions, true);
+      for (uint i = num_regions; i < _max_size; i++) {
+        assert(_data[i] == NULL, "must be");
+      }
+      for (uint i = 0; i < num_regions; i++) {
+        dest[i] = _data[i];
+      }
+    }
+  };
+
+  // Per-region closure. In addition to determining whether a region should be
+  // added to the candidates, and calculating those regions' gc efficiencies, also
+  // gather additional statistics.
+  class G1BuildCandidateRegionsClosure : public HeapRegionClosure {
+    G1BuildCandidateArray* _array;
+
+    uint _cur_chunk_idx;
+    uint _cur_chunk_end;
+
+    uint _regions_added;
+    size_t _reclaimable_bytes_added;
+
+    void add_region(HeapRegion* hr) {
+      if (_cur_chunk_idx == _cur_chunk_end) {
+        _array->claim_chunk(_cur_chunk_idx, _cur_chunk_end);
+      }
+      assert(_cur_chunk_idx < _cur_chunk_end, "Must be");
+
+      hr->calc_gc_efficiency();
+      _array->set(_cur_chunk_idx, hr);
+
+      _cur_chunk_idx++;
+
+      _regions_added++;
+      _reclaimable_bytes_added += hr->reclaimable_bytes();
     }
-    sum_of_reclaimable_bytes += curr->reclaimable_bytes();
-    prev = curr;
-  }
-  guarantee(sum_of_reclaimable_bytes == _remaining_reclaimable_bytes,
-            "reclaimable bytes inconsistent, "
-            "remaining: " SIZE_FORMAT " sum: " SIZE_FORMAT,
-            _remaining_reclaimable_bytes, sum_of_reclaimable_bytes);
-}
-#endif // !PRODUCT
+
+    bool should_add(HeapRegion* hr) { return CollectionSetChooser::should_add(hr); }
+
+  public:
+    G1BuildCandidateRegionsClosure(G1BuildCandidateArray* array) :
+      _array(array),
+      _cur_chunk_idx(0),
+      _cur_chunk_end(0),
+      _regions_added(0),
+      _reclaimable_bytes_added(0) { }
 
-void CollectionSetChooser::sort_regions() {
-  // First trim any unused portion of the top in the parallel case.
-  if (_first_par_unreserved_idx > 0) {
-    assert(_first_par_unreserved_idx <= regions_length(),
-           "Or we didn't reserved enough length");
-    regions_trunc_to(_first_par_unreserved_idx);
-  }
-  _regions.sort(order_regions);
-  assert(_end <= regions_length(), "Requirement");
-#ifdef ASSERT
-  for (uint i = 0; i < _end; i++) {
-    assert(regions_at(i) != NULL, "Should be true by sorting!");
-  }
-#endif // ASSERT
-  if (log_is_enabled(Trace, gc, liveness)) {
-    G1PrintRegionLivenessInfoClosure cl("Post-Sorting");
-    for (uint i = 0; i < _end; ++i) {
-      HeapRegion* r = regions_at(i);
-      cl.do_heap_region(r);
+    bool do_heap_region(HeapRegion* r) {
+      // We will skip any region that's currently used as an old GC
+      // alloc region (we should not consider those for collection
+      // before we fill them up).
+      if (should_add(r) && !G1CollectedHeap::heap()->is_old_gc_alloc_region(r)) {
+        add_region(r);
+      } else if (r->is_old()) {
+        // Keep remembered sets for humongous regions, otherwise clean out remembered
+        // sets for old regions.
+        r->rem_set()->clear(true /* only_cardset */);
+      } else {
+        assert(r->is_archive() || !r->is_old() || !r->rem_set()->is_tracked(),
+               "Missed to clear unused remembered set of region %u (%s) that is %s",
+               r->hrm_index(), r->get_type_str(), r->rem_set()->get_state_str());
+      }
+      return false;
+    }
+
+    uint regions_added() const { return _regions_added; }
+    size_t reclaimable_bytes_added() const { return _reclaimable_bytes_added; }
+  };
+
+  G1CollectedHeap* _g1h;
+  HeapRegionClaimer _hrclaimer;
+
+  uint volatile _num_regions_added;
+  size_t volatile _reclaimable_bytes_added;
+
+  G1BuildCandidateArray _result;
+
+  void update_totals(uint num_regions, size_t reclaimable_bytes) {
+    if (num_regions > 0) {
+      assert(reclaimable_bytes > 0, "invariant");
+      Atomic::add(num_regions, &_num_regions_added);
+      Atomic::add(reclaimable_bytes, &_reclaimable_bytes_added);
+    } else {
+      assert(reclaimable_bytes == 0, "invariant");
     }
   }
-  verify();
-}
-
-void CollectionSetChooser::add_region(HeapRegion* hr) {
-  assert(!hr->is_pinned(),
-         "Pinned region shouldn't be added to the collection set (index %u)", hr->hrm_index());
-  assert(hr->is_old(), "should be old but is %s", hr->get_type_str());
-  assert(hr->rem_set()->is_complete(),
-         "Trying to add region %u to the collection set with incomplete remembered set", hr->hrm_index());
-  _regions.append(hr);
-  _end++;
-  _remaining_reclaimable_bytes += hr->reclaimable_bytes();
-  hr->calc_gc_efficiency();
-}
-
-void CollectionSetChooser::push(HeapRegion* hr) {
-  assert(hr != NULL, "Can't put back a NULL region");
-  assert(_front >= 1, "Too many regions have been put back");
-  _front--;
-  regions_at_put(_front, hr);
-  _remaining_reclaimable_bytes += hr->reclaimable_bytes();
-}
-
-void CollectionSetChooser::prepare_for_par_region_addition(uint n_threads,
-                                                           uint n_regions,
-                                                           uint chunk_size) {
-  _first_par_unreserved_idx = 0;
-  uint max_waste = n_threads * chunk_size;
-  // it should be aligned with respect to chunk_size
-  uint aligned_n_regions = (n_regions + chunk_size - 1) / chunk_size * chunk_size;
-  assert(aligned_n_regions % chunk_size == 0, "should be aligned");
-  regions_at_put_grow(aligned_n_regions + max_waste - 1, NULL);
-}
-
-uint CollectionSetChooser::claim_array_chunk(uint chunk_size) {
-  uint res = (uint) Atomic::add((jint) chunk_size,
-                                (volatile jint*) &_first_par_unreserved_idx);
-  assert(regions_length() > res + chunk_size - 1,
-         "Should already have been expanded");
-  return res - chunk_size;
-}
-
-void CollectionSetChooser::set_region(uint index, HeapRegion* hr) {
-  assert(regions_at(index) == NULL, "precondition");
-  assert(hr->is_old(), "should be old but is %s", hr->get_type_str());
-  regions_at_put(index, hr);
-  hr->calc_gc_efficiency();
-}
-
-void CollectionSetChooser::update_totals(uint region_num,
-                                         size_t reclaimable_bytes) {
-  // Only take the lock if we actually need to update the totals.
-  if (region_num > 0) {
-    assert(reclaimable_bytes > 0, "invariant");
-    // We could have just used atomics instead of taking the
-    // lock. However, we currently don't have an atomic add for size_t.
-    MutexLockerEx x(ParGCRareEvent_lock, Mutex::_no_safepoint_check_flag);
-    _end += region_num;
-    _remaining_reclaimable_bytes += reclaimable_bytes;
-  } else {
-    assert(reclaimable_bytes == 0, "invariant");
-  }
-}
-
-void CollectionSetChooser::iterate(HeapRegionClosure* cl) {
-  for (uint i = _front; i < _end; i++) {
-    HeapRegion* r = regions_at(i);
-    if (cl->do_heap_region(r)) {
-      cl->set_incomplete();
-      break;
-    }
-  }
-}
-
-void CollectionSetChooser::clear() {
-  _regions.clear();
-  _front = 0;
-  _end = 0;
-  _remaining_reclaimable_bytes = 0;
-}
-
-class ParKnownGarbageHRClosure: public HeapRegionClosure {
-  G1CollectedHeap* _g1h;
-  CSetChooserParUpdater _cset_updater;
 
 public:
-  ParKnownGarbageHRClosure(CollectionSetChooser* hrSorted,
-                           uint chunk_size) :
+  G1BuildCandidateRegionsTask(uint max_num_regions, uint chunk_size, uint num_workers) :
+    AbstractGangTask("G1 Build Candidate Regions"),
     _g1h(G1CollectedHeap::heap()),
-    _cset_updater(hrSorted, true /* parallel */, chunk_size) { }
+    _hrclaimer(num_workers),
+    _num_regions_added(0),
+    _reclaimable_bytes_added(0),
+    _result(max_num_regions, chunk_size, num_workers) { }
 
-  bool do_heap_region(HeapRegion* r) {
-    // We will skip any region that's currently used as an old GC
-    // alloc region (we should not consider those for collection
-    // before we fill them up).
-    if (_cset_updater.should_add(r) && !_g1h->is_old_gc_alloc_region(r)) {
-      _cset_updater.add_region(r);
-    } else if (r->is_old()) {
-      // Keep remembered sets for humongous regions, otherwise clean out remembered
-      // sets for old regions.
-      r->rem_set()->clear(true /* only_cardset */);
-    } else {
-      assert(r->is_archive() || !r->is_old() || !r->rem_set()->is_tracked(),
-             "Missed to clear unused remembered set of region %u (%s) that is %s",
-             r->hrm_index(), r->get_type_str(), r->rem_set()->get_state_str());
-    }
-    return false;
+  void work(uint worker_id) {
+    G1BuildCandidateRegionsClosure cl(&_result);
+    _g1h->heap_region_par_iterate_from_worker_offset(&cl, &_hrclaimer, worker_id);
+    update_totals(cl.regions_added(), cl.reclaimable_bytes_added());
+  }
+
+  G1CollectionSetCandidates* get_sorted_candidates() {
+    HeapRegion** regions = NEW_C_HEAP_ARRAY(HeapRegion*, _num_regions_added, mtGC);
+    _result.sort_and_copy_into(regions, _num_regions_added);
+    return new G1CollectionSetCandidates(regions,
+                                         _num_regions_added,
+                                         _reclaimable_bytes_added);
   }
 };
 
-class ParKnownGarbageTask: public AbstractGangTask {
-  CollectionSetChooser* _hrSorted;
-  uint _chunk_size;
-  G1CollectedHeap* _g1h;
-  HeapRegionClaimer _hrclaimer;
-
-public:
-  ParKnownGarbageTask(CollectionSetChooser* hrSorted, uint chunk_size, uint n_workers) :
-      AbstractGangTask("ParKnownGarbageTask"),
-      _hrSorted(hrSorted), _chunk_size(chunk_size),
-      _g1h(G1CollectedHeap::heap()), _hrclaimer(n_workers) {}
-
-  void work(uint worker_id) {
-    ParKnownGarbageHRClosure par_known_garbage_cl(_hrSorted, _chunk_size);
-    _g1h->heap_region_par_iterate_from_worker_offset(&par_known_garbage_cl, &_hrclaimer, worker_id);
-  }
-};
-
-uint CollectionSetChooser::calculate_parallel_work_chunk_size(uint n_workers, uint n_regions) const {
-  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);
+uint CollectionSetChooser::calculate_work_chunk_size(uint num_workers, uint num_regions) {
+  assert(num_workers > 0, "Active gc workers should be greater than 0");
+  return MAX2(num_regions / num_workers, 1U);
 }
 
-bool CollectionSetChooser::region_occupancy_low_enough_for_evac(size_t live_bytes) {
-  return live_bytes < mixed_gc_live_threshold_bytes();
-}
-
-bool CollectionSetChooser::should_add(HeapRegion* hr) const {
+bool CollectionSetChooser::should_add(HeapRegion* hr) {
   return !hr->is_young() &&
          !hr->is_pinned() &&
          region_occupancy_low_enough_for_evac(hr->live_bytes()) &&
          hr->rem_set()->is_complete();
 }
 
-void CollectionSetChooser::rebuild(WorkGang* workers, uint n_regions) {
-  clear();
-
-  uint n_workers = workers->active_workers();
+G1CollectionSetCandidates* CollectionSetChooser::build(WorkGang* workers, uint max_num_regions) {
+  uint num_workers = workers->active_workers();
+  uint chunk_size = calculate_work_chunk_size(num_workers, max_num_regions);
 
-  uint chunk_size = calculate_parallel_work_chunk_size(n_workers, n_regions);
-  prepare_for_par_region_addition(n_workers, n_regions, chunk_size);
+  G1BuildCandidateRegionsTask cl(max_num_regions, chunk_size, num_workers);
+  workers->run_task(&cl, num_workers);
 
-  ParKnownGarbageTask par_known_garbage_task(this, chunk_size, n_workers);
-  workers->run_task(&par_known_garbage_task);
-
-  sort_regions();
+  G1CollectionSetCandidates* result = cl.get_sorted_candidates();
+  result->verify();
+  return result;
 }
--- a/src/hotspot/share/gc/g1/collectionSetChooser.hpp	Fri Feb 08 12:55:20 2019 +0100
+++ b/src/hotspot/share/gc/g1/collectionSetChooser.hpp	Fri Feb 08 12:55:20 2019 +0100
@@ -26,177 +26,35 @@
 #define SHARE_GC_G1_COLLECTIONSETCHOOSER_HPP
 
 #include "gc/g1/heapRegion.hpp"
-#include "utilities/growableArray.hpp"
-
-class CollectionSetChooser: public CHeapObj<mtGC> {
-
-  GrowableArray<HeapRegion*> _regions;
-
-  // Unfortunately, GrowableArray uses ints for length and indexes. To
-  // avoid excessive casting in the rest of the class the following
-  // wrapper methods are provided that use uints.
+#include "memory/allocation.hpp"
+#include "runtime/globals.hpp"
 
-  uint regions_length()          { return (uint) _regions.length(); }
-  HeapRegion* regions_at(uint i) { return _regions.at((int) i);     }
-  void regions_at_put(uint i, HeapRegion* hr) {
-    _regions.at_put((int) i, hr);
-  }
-  void regions_at_put_grow(uint i, HeapRegion* hr) {
-    _regions.at_put_grow((int) i, hr);
-  }
-  void regions_trunc_to(uint i)  { _regions.trunc_to((uint) i); }
-
-  // The index of the next candidate old region to be considered for
-  // addition to the CSet.
-  uint _front;
-
-  // The index of the last candidate old region
-  uint _end;
-
-  // Keeps track of the start of the next array chunk to be claimed by
-  // parallel GC workers.
-  uint _first_par_unreserved_idx;
-
-  // If a region has more live bytes than this threshold, it will not
-  // be added to the CSet chooser and will not be a candidate for
-  // collection.
-  size_t _region_live_threshold_bytes;
+class G1CollectionSetCandidates;
+class WorkGang;
 
-  // The sum of reclaimable bytes over all the regions in the CSet chooser.
-  size_t _remaining_reclaimable_bytes;
-
-  // Calculate and return chunk size (in number of regions) for parallel
-  // addition of regions
-  uint calculate_parallel_work_chunk_size(uint n_workers, uint n_regions) const;
+// Helper class to calculate collection set candidates, and containing some related
+// methods.
+class CollectionSetChooser : public AllStatic {
+  static uint calculate_work_chunk_size(uint num_workers, uint num_regions);
 public:
 
-  // Return the current candidate region to be considered for
-  // collection without removing it from the CSet chooser.
-  HeapRegion* peek() {
-    HeapRegion* res = NULL;
-    if (_front < _end) {
-      res = regions_at(_front);
-      assert(res != NULL, "Unexpected NULL hr in _regions at index %u", _front);
-    }
-    return res;
-  }
-
-  // Remove the given region from the CSet chooser and move to the
-  // next one.
-  HeapRegion* pop() {
-    HeapRegion* hr = regions_at(_front);
-    assert(hr != NULL, "pre-condition");
-    assert(_front < _end, "pre-condition");
-    regions_at_put(_front, NULL);
-    assert(hr->reclaimable_bytes() <= _remaining_reclaimable_bytes,
-           "remaining reclaimable bytes inconsistent "
-           "from region: " SIZE_FORMAT " remaining: " SIZE_FORMAT,
-           hr->reclaimable_bytes(), _remaining_reclaimable_bytes);
-    _remaining_reclaimable_bytes -= hr->reclaimable_bytes();
-    _front += 1;
-    return hr;
-  }
-
-  void push(HeapRegion* hr);
-
-  CollectionSetChooser();
-
   static size_t mixed_gc_live_threshold_bytes() {
     return HeapRegion::GrainBytes * (size_t) G1MixedGCLiveThresholdPercent / 100;
   }
 
-  static bool region_occupancy_low_enough_for_evac(size_t live_bytes);
+  static bool region_occupancy_low_enough_for_evac(size_t live_bytes) {
+    return live_bytes < mixed_gc_live_threshold_bytes();
+  }
 
-  void sort_regions();
-
-  // Determine whether to add the given region to the CSet chooser or
+  // Determine whether to add the given region to the collection set candidates or
   // not. Currently, we skip pinned regions and regions whose live
   // bytes are over the threshold. Humongous regions may be reclaimed during cleanup.
   // Regions also need a complete remembered set to be a candidate.
-  bool should_add(HeapRegion* hr) const;
-
-  // Returns the number candidate old regions added
-  uint length() { return _end; }
-
-  // Serial version.
-  void add_region(HeapRegion *hr);
-
-  // Must be called before calls to claim_array_chunk().
-  // n_regions is the number of regions, chunk_size the chunk size.
-  void prepare_for_par_region_addition(uint n_threads, uint n_regions, uint chunk_size);
-  // Returns the first index in a contiguous chunk of chunk_size indexes
-  // that the calling thread has reserved.  These must be set by the
-  // calling thread using set_region() (to NULL if necessary).
-  uint claim_array_chunk(uint chunk_size);
-  // Set the marked array entry at index to hr.  Careful to claim the index
-  // first if in parallel.
-  void set_region(uint index, HeapRegion* hr);
-  // Atomically increment the number of added regions by region_num
-  // and the amount of reclaimable bytes by reclaimable_bytes.
-  void update_totals(uint region_num, size_t reclaimable_bytes);
-
-  // Iterate over all collection set candidate regions.
-  void iterate(HeapRegionClosure* cl);
-
-  void clear();
-
-  void rebuild(WorkGang* workers, uint n_regions);
-
-  // Return the number of candidate regions that remain to be collected.
-  uint remaining_regions() { return _end - _front; }
-
-  // Determine whether the CSet chooser has more candidate regions or not.
-  bool is_empty() { return remaining_regions() == 0; }
-
-  // Return the reclaimable bytes that remain to be collected on
-  // all the candidate regions in the CSet chooser.
-  size_t remaining_reclaimable_bytes() { return _remaining_reclaimable_bytes; }
+  static bool should_add(HeapRegion* hr);
 
-  // Returns true if the used portion of "_regions" is properly
-  // sorted, otherwise asserts false.
-  void verify() PRODUCT_RETURN;
-};
-
-class CSetChooserParUpdater : public StackObj {
-private:
-  CollectionSetChooser* _chooser;
-  bool _parallel;
-  uint _chunk_size;
-  uint _cur_chunk_idx;
-  uint _cur_chunk_end;
-  uint _regions_added;
-  size_t _reclaimable_bytes_added;
-
-public:
-  CSetChooserParUpdater(CollectionSetChooser* chooser,
-                        bool parallel, uint chunk_size) :
-    _chooser(chooser), _parallel(parallel), _chunk_size(chunk_size),
-    _cur_chunk_idx(0), _cur_chunk_end(0),
-    _regions_added(0), _reclaimable_bytes_added(0) { }
-
-  ~CSetChooserParUpdater() {
-    if (_parallel && _regions_added > 0) {
-      _chooser->update_totals(_regions_added, _reclaimable_bytes_added);
-    }
-  }
-
-  void add_region(HeapRegion* hr) {
-    if (_parallel) {
-      if (_cur_chunk_idx == _cur_chunk_end) {
-        _cur_chunk_idx = _chooser->claim_array_chunk(_chunk_size);
-        _cur_chunk_end = _cur_chunk_idx + _chunk_size;
-      }
-      assert(_cur_chunk_idx < _cur_chunk_end, "invariant");
-      _chooser->set_region(_cur_chunk_idx, hr);
-      _cur_chunk_idx += 1;
-    } else {
-      _chooser->add_region(hr);
-    }
-    _regions_added += 1;
-    _reclaimable_bytes_added += hr->reclaimable_bytes();
-  }
-
-  bool should_add(HeapRegion* hr) { return _chooser->should_add(hr); }
+  // Build and return set of collection set candidates sorted by decreasing gc
+  // efficiency.
+  static G1CollectionSetCandidates* build(WorkGang* workers, uint max_num_regions);
 };
 
 #endif // SHARE_GC_G1_COLLECTIONSETCHOOSER_HPP
--- a/src/hotspot/share/gc/g1/g1CollectionSet.cpp	Fri Feb 08 12:55:20 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1CollectionSet.cpp	Fri Feb 08 12:55:20 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, 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
@@ -25,6 +25,7 @@
 #include "precompiled.hpp"
 #include "gc/g1/g1CollectedHeap.inline.hpp"
 #include "gc/g1/g1CollectionSet.hpp"
+#include "gc/g1/g1CollectionSetCandidates.hpp"
 #include "gc/g1/g1CollectorState.hpp"
 #include "gc/g1/g1ParScanThreadState.hpp"
 #include "gc/g1/g1Policy.hpp"
@@ -44,10 +45,6 @@
   return _policy->phase_times();
 }
 
-CollectionSetChooser* G1CollectionSet::cset_chooser() {
-  return _cset_chooser;
-}
-
 double G1CollectionSet::predict_region_elapsed_time_ms(HeapRegion* hr) {
   return _policy->predict_region_elapsed_time_ms(hr, collector_state()->in_young_only_phase());
 }
@@ -55,7 +52,7 @@
 G1CollectionSet::G1CollectionSet(G1CollectedHeap* g1h, G1Policy* policy) :
   _g1h(g1h),
   _policy(policy),
-  _cset_chooser(new CollectionSetChooser()),
+  _candidates(NULL),
   _eden_region_length(0),
   _survivor_region_length(0),
   _old_region_length(0),
@@ -80,7 +77,7 @@
     FREE_C_HEAP_ARRAY(uint, _collection_set_regions);
   }
   free_optional_regions();
-  delete _cset_chooser;
+  clear_candidates();
 }
 
 void G1CollectionSet::init_region_lengths(uint eden_cset_region_length,
@@ -120,6 +117,11 @@
   }
 }
 
+void G1CollectionSet::clear_candidates() {
+  delete _candidates;
+  _candidates = NULL;
+}
+
 void G1CollectionSet::set_recorded_rs_lengths(size_t rs_lengths) {
   _recorded_rs_lengths = rs_lengths;
 }
@@ -439,14 +441,14 @@
 }
 
 void G1CollectionSet::add_as_old(HeapRegion* hr) {
-  cset_chooser()->pop(); // already have region via peek()
+  candidates()->pop_front(); // already have region via peek()
   _g1h->old_set_remove(hr);
   add_old_region(hr);
 }
 
 void G1CollectionSet::add_as_optional(HeapRegion* hr) {
   assert(_optional_regions != NULL, "Must not be called before array is allocated");
-  cset_chooser()->pop(); // already have region via peek()
+  candidates()->pop_front(); // already have region via peek()
   _g1h->old_set_remove(hr);
   add_optional_region(hr);
 }
@@ -480,7 +482,7 @@
   uint expensive_region_num = 0;
 
   if (collector_state()->in_mixed_phase()) {
-    cset_chooser()->verify();
+    candidates()->verify();
     const uint min_old_cset_length = _policy->calc_min_old_cset_length();
     const uint max_old_cset_length = MAX2(min_old_cset_length, _policy->calc_max_old_cset_length());
     bool check_time_remaining = _policy->adaptive_young_list_length();
@@ -490,7 +492,7 @@
                               "time remaining %1.2fms, optional threshold %1.2fms",
                               min_old_cset_length, max_old_cset_length, time_remaining_ms, optional_threshold_ms);
 
-    HeapRegion* hr = cset_chooser()->peek();
+    HeapRegion* hr = candidates()->peek_front();
     while (hr != NULL) {
       if (old_region_length() + optional_region_length() >= max_old_cset_length) {
         // Added maximum number of old regions to the CSet.
@@ -502,7 +504,7 @@
 
       // Stop adding regions if the remaining reclaimable space is
       // not above G1HeapWastePercent.
-      size_t reclaimable_bytes = cset_chooser()->remaining_reclaimable_bytes();
+      size_t reclaimable_bytes = candidates()->remaining_reclaimable_bytes();
       double reclaimable_percent = _policy->reclaimable_bytes_percent(reclaimable_bytes);
       double threshold = (double) G1HeapWastePercent;
       if (reclaimable_percent <= threshold) {
@@ -551,13 +553,13 @@
           break;
         }
       }
-      hr = cset_chooser()->peek();
+      hr = candidates()->peek_front();
     }
     if (hr == NULL) {
       log_debug(gc, ergo, cset)("Finish adding old regions to CSet (candidate old regions not available)");
     }
 
-    cset_chooser()->verify();
+    candidates()->verify();
   }
 
   stop_incremental_building();
@@ -630,15 +632,15 @@
 G1OptionalCSet::~G1OptionalCSet() {
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
   while (!is_empty()) {
-    // We want to return regions not evacuated to the
-    // chooser in reverse order to maintain the old order.
+    // We want to return regions not evacuated to the collection set candidates
+    // in reverse order to maintain the old order.
     HeapRegion* hr = _cset->remove_last_optional_region();
     assert(hr != NULL, "Should be valid region left");
     _pset->record_unused_optional_region(hr);
     g1h->old_set_add(hr);
     g1h->clear_in_cset(hr);
     hr->set_index_in_opt_cset(InvalidCSetIndex);
-    _cset->cset_chooser()->push(hr);
+    _cset->candidates()->push_front(hr);
   }
   _cset->free_optional_regions();
 }
--- a/src/hotspot/share/gc/g1/g1CollectionSet.hpp	Fri Feb 08 12:55:20 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1CollectionSet.hpp	Fri Feb 08 12:55:20 2019 +0100
@@ -25,23 +25,25 @@
 #ifndef SHARE_GC_G1_G1COLLECTIONSET_HPP
 #define SHARE_GC_G1_G1COLLECTIONSET_HPP
 
-#include "gc/g1/collectionSetChooser.hpp"
 #include "utilities/debug.hpp"
 #include "utilities/globalDefinitions.hpp"
 
 class G1CollectedHeap;
+class G1CollectionSetCandidates;
 class G1CollectorState;
 class G1GCPhaseTimes;
 class G1ParScanThreadStateSet;
 class G1Policy;
 class G1SurvivorRegions;
 class HeapRegion;
+class HeapRegionClosure;
 
 class G1CollectionSet {
   G1CollectedHeap* _g1h;
   G1Policy* _policy;
 
-  CollectionSetChooser* _cset_chooser;
+  // All old gen collection set candidate regions for the current mixed gc phase.
+  G1CollectionSetCandidates* _candidates;
 
   uint _eden_region_length;
   uint _survivor_region_length;
@@ -128,7 +130,13 @@
   void initialize_optional(uint max_length);
   void free_optional_regions();
 
-  CollectionSetChooser* cset_chooser();
+  void clear_candidates();
+
+  void set_candidates(G1CollectionSetCandidates* candidates) {
+    assert(_candidates == NULL, "Trying to replace collection set candidates.");
+    _candidates = candidates;
+  }
+  G1CollectionSetCandidates* candidates() { return _candidates; }
 
   void init_region_lengths(uint eden_cset_region_length,
                            uint survivor_cset_region_length);
@@ -253,8 +261,8 @@
     _current_limit(0),
     _prepare_failed(false),
     _evacuation_failed(false) { }
-  // The destructor returns regions to the cset-chooser and
-  // frees the optional structure in the cset.
+  // The destructor returns regions to the collection set candidates set and
+  // frees the optional structure in the collection set.
   ~G1OptionalCSet();
 
   uint current_index() { return _current_index; }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/hotspot/share/gc/g1/g1CollectionSetCandidates.cpp	Fri Feb 08 12:55:20 2019 +0100
@@ -0,0 +1,89 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#include "precompiled.hpp"
+#include "gc/g1/collectionSetChooser.hpp"
+#include "gc/g1/g1CollectionSetCandidates.hpp"
+#include "gc/g1/heapRegion.inline.hpp"
+
+HeapRegion* G1CollectionSetCandidates::pop_front() {
+  assert(_front_idx < _num_regions, "pre-condition");
+  HeapRegion* hr = _regions[_front_idx];
+  assert(hr != NULL, "pre-condition");
+  _regions[_front_idx] = NULL;
+  assert(hr->reclaimable_bytes() <= _remaining_reclaimable_bytes,
+         "Remaining reclaimable bytes inconsistent "
+         "from region: " SIZE_FORMAT " remaining: " SIZE_FORMAT,
+         hr->reclaimable_bytes(), _remaining_reclaimable_bytes);
+  _remaining_reclaimable_bytes -= hr->reclaimable_bytes();
+  _front_idx++;
+  return hr;
+}
+
+void G1CollectionSetCandidates::push_front(HeapRegion* hr) {
+  assert(hr != NULL, "Can't put back a NULL region");
+  assert(_front_idx >= 1, "Too many regions have been put back.");
+  _front_idx--;
+  _regions[_front_idx] = hr;
+  _remaining_reclaimable_bytes += hr->reclaimable_bytes();
+}
+
+void G1CollectionSetCandidates::iterate(HeapRegionClosure* cl) {
+  for (uint i = _front_idx; i < _num_regions; i++) {
+    HeapRegion* r = _regions[i];
+    if (cl->do_heap_region(r)) {
+      cl->set_incomplete();
+      break;
+    }
+  }
+}
+
+#ifndef PRODUCT
+void G1CollectionSetCandidates::verify() const {
+  guarantee(_front_idx <= _num_regions, "Index: %u Num_regions: %u", _front_idx, _num_regions);
+  uint idx = 0;
+  size_t sum_of_reclaimable_bytes = 0;
+  while (idx < _front_idx) {
+    guarantee(_regions[idx] == NULL, "All entries before _front_idx %u should be NULL, but %u is not",
+              _front_idx, idx);
+    idx++;
+  }
+  HeapRegion *prev = NULL;
+  for (; idx < _num_regions; idx++) {
+    HeapRegion *cur = _regions[idx];
+    guarantee(cur != NULL, "Regions after _front_idx %u cannot be NULL but %u is", _front_idx, idx);
+    guarantee(CollectionSetChooser::should_add(cur), "Region %u should be eligible for addition.", cur->hrm_index());
+    if (prev != NULL) {
+      guarantee(prev->gc_efficiency() >= cur->gc_efficiency(),
+                "GC efficiency for region %u: %1.4f smaller than for region %u: %1.4f",
+                prev->hrm_index(), prev->gc_efficiency(), cur->hrm_index(), cur->gc_efficiency());
+    }
+    sum_of_reclaimable_bytes += cur->reclaimable_bytes();
+    prev = cur;
+  }
+  guarantee(sum_of_reclaimable_bytes == _remaining_reclaimable_bytes,
+            "Inconsistent remaining_reclaimable bytes, remaining " SIZE_FORMAT " calculated " SIZE_FORMAT,
+            _remaining_reclaimable_bytes, sum_of_reclaimable_bytes);
+}
+#endif // !PRODUCT
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/hotspot/share/gc/g1/g1CollectionSetCandidates.hpp	Fri Feb 08 12:55:20 2019 +0100
@@ -0,0 +1,99 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+#ifndef SHARE_GC_G1_G1COLLECTIONSETCANDIDATES_HPP
+#define SHARE_GC_G1_G1COLLECTIONSETCANDIDATES_HPP
+
+#include "gc/g1/g1CollectionSetCandidates.hpp"
+#include "gc/shared/workgroup.hpp"
+#include "memory/allocation.hpp"
+#include "runtime/globals.hpp"
+
+class HeapRegion;
+class HeapRegionClosure;
+
+// Set of collection set candidates, i.e. all old gen regions we consider worth
+// collecting in the remainder of the current mixed phase. Regions are sorted by decreasing
+// gc efficiency.
+// Maintains a cursor into the list that specifies the next collection set candidate
+// to put into the current collection set.
+class G1CollectionSetCandidates : public CHeapObj<mtGC> {
+  HeapRegion** _regions;
+  uint _num_regions; // Total number of regions in the collection set candidate set.
+
+  // The sum of bytes that can be reclaimed in the remaining set of collection
+  // set candidates.
+  size_t _remaining_reclaimable_bytes;
+  // The index of the next candidate old region to be considered for
+  // addition to the current collection set.
+  uint _front_idx;
+
+public:
+  G1CollectionSetCandidates(HeapRegion** regions, uint num_regions, size_t remaining_reclaimable_bytes) :
+    _regions(regions),
+    _num_regions(num_regions),
+    _remaining_reclaimable_bytes(remaining_reclaimable_bytes),
+    _front_idx(0) { }
+
+  ~G1CollectionSetCandidates() {
+    FREE_C_HEAP_ARRAY(HeapRegion*, _regions);
+  }
+
+  // Returns the total number of collection set candidate old regions added.
+  uint num_regions() { return _num_regions; }
+
+  // Return the candidate region at the cursor position to be considered for collection without
+  // removing it.
+  HeapRegion* peek_front() {
+    HeapRegion* res = NULL;
+    if (_front_idx < _num_regions) {
+      res = _regions[_front_idx];
+      assert(res != NULL, "Unexpected NULL HeapRegion at index %u", _front_idx);
+    }
+    return res;
+  }
+
+  // Remove the given region from the candidates set and move the cursor to the next one.
+  HeapRegion* pop_front();
+
+  // Add the given HeapRegion to the front of the collection set candidate set again.
+  void push_front(HeapRegion* hr);
+
+  // Iterate over all remaining collection set candidate regions.
+  void iterate(HeapRegionClosure* cl);
+
+  // Return the number of candidate regions remaining.
+  uint num_remaining() { return _num_regions - _front_idx; }
+
+  bool is_empty() { return num_remaining() == 0; }
+
+  // Return the amount of reclaimable bytes that may be collected by the remaining
+  // candidate regions.
+  size_t remaining_reclaimable_bytes() { return _remaining_reclaimable_bytes; }
+
+  void verify() const PRODUCT_RETURN;
+};
+
+#endif /* SHARE_GC_G1_G1COLLECTIONSETCANDIDATES_HPP */
+
--- a/src/hotspot/share/gc/g1/g1Policy.cpp	Fri Feb 08 12:55:20 2019 +0100
+++ b/src/hotspot/share/gc/g1/g1Policy.cpp	Fri Feb 08 12:55:20 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2019, 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
@@ -23,9 +23,11 @@
  */
 
 #include "precompiled.hpp"
+#include "gc/g1/collectionSetChooser.hpp"
 #include "gc/g1/g1Analytics.hpp"
 #include "gc/g1/g1CollectedHeap.inline.hpp"
 #include "gc/g1/g1CollectionSet.hpp"
+#include "gc/g1/g1CollectionSetCandidates.hpp"
 #include "gc/g1/g1ConcurrentMark.hpp"
 #include "gc/g1/g1ConcurrentMarkThread.inline.hpp"
 #include "gc/g1/g1ConcurrentRefine.hpp"
@@ -438,7 +440,7 @@
   // Release the future to-space so that it is available for compaction into.
   collector_state()->set_in_young_only_phase(false);
   collector_state()->set_in_full_gc(true);
-  cset_chooser()->clear();
+  _collection_set->clear_candidates();
 }
 
 void G1Policy::record_full_collection_end() {
@@ -546,10 +548,6 @@
   return other_time_ms(pause_time_ms) - phase_times()->total_free_cset_time_ms();
 }
 
-CollectionSetChooser* G1Policy::cset_chooser() const {
-  return _collection_set->cset_chooser();
-}
-
 bool G1Policy::about_to_start_mixed_phase() const {
   return _g1h->concurrent_mark()->cm_thread()->during_cycle() || collector_state()->in_young_gc_before_mixed();
 }
@@ -773,8 +771,6 @@
   _g1h->concurrent_refine()->adjust(average_time_ms(G1GCPhaseTimes::UpdateRS),
                                     phase_times()->sum_thread_work_items(G1GCPhaseTimes::UpdateRS),
                                     update_rs_time_goal_ms);
-
-  cset_chooser()->verify();
 }
 
 G1IHOPControl* G1Policy::create_ihop_control(const G1Predictions* predictor){
@@ -1032,7 +1028,8 @@
 }
 
 void G1Policy::record_concurrent_mark_cleanup_end() {
-  cset_chooser()->rebuild(_g1h->workers(), _g1h->num_regions());
+  G1CollectionSetCandidates* candidates = CollectionSetChooser::build(_g1h->workers(), _g1h->num_regions());
+  _collection_set->set_candidates(candidates);
 
   bool mixed_gc_pending = next_gc_should_be_mixed("request mixed gcs", "request young-only gcs");
   if (!mixed_gc_pending) {
@@ -1063,10 +1060,10 @@
 
 void G1Policy::clear_collection_set_candidates() {
   // Clear remembered sets of remaining candidate regions and the actual candidate
-  // list.
+  // set.
   G1ClearCollectionSetCandidateRemSets cl;
-  cset_chooser()->iterate(&cl);
-  cset_chooser()->clear();
+  _collection_set->candidates()->iterate(&cl);
+  _collection_set->clear_candidates();
 }
 
 void G1Policy::maybe_start_marking() {
@@ -1132,22 +1129,24 @@
 
 bool G1Policy::next_gc_should_be_mixed(const char* true_action_str,
                                        const char* false_action_str) const {
-  if (cset_chooser()->is_empty()) {
+  G1CollectionSetCandidates* candidates = _collection_set->candidates();
+
+  if (candidates->is_empty()) {
     log_debug(gc, ergo)("%s (candidate old regions not available)", false_action_str);
     return false;
   }
 
   // Is the amount of uncollected reclaimable space above G1HeapWastePercent?
-  size_t reclaimable_bytes = cset_chooser()->remaining_reclaimable_bytes();
+  size_t reclaimable_bytes = candidates->remaining_reclaimable_bytes();
   double reclaimable_percent = reclaimable_bytes_percent(reclaimable_bytes);
   double threshold = (double) G1HeapWastePercent;
   if (reclaimable_percent <= threshold) {
     log_debug(gc, ergo)("%s (reclaimable percentage not over threshold). candidate old regions: %u reclaimable: " SIZE_FORMAT " (%1.2f) threshold: " UINTX_FORMAT,
-                        false_action_str, cset_chooser()->remaining_regions(), reclaimable_bytes, reclaimable_percent, G1HeapWastePercent);
+                        false_action_str, candidates->num_remaining(), reclaimable_bytes, reclaimable_percent, G1HeapWastePercent);
     return false;
   }
   log_debug(gc, ergo)("%s (candidate old regions available). candidate old regions: %u reclaimable: " SIZE_FORMAT " (%1.2f) threshold: " UINTX_FORMAT,
-                      true_action_str, cset_chooser()->remaining_regions(), reclaimable_bytes, reclaimable_percent, G1HeapWastePercent);
+                      true_action_str, candidates->num_remaining(), reclaimable_bytes, reclaimable_percent, G1HeapWastePercent);
   return true;
 }
 
@@ -1159,10 +1158,10 @@
   // maximum desired number of mixed GCs.
   //
   // The calculation is based on the number of marked regions we added
-  // to the CSet chooser in the first place, not how many remain, so
+  // to the CSet candidates in the first place, not how many remain, so
   // that the result is the same during all mixed GCs that follow a cycle.
 
-  const size_t region_num = (size_t) cset_chooser()->length();
+  const size_t region_num = _collection_set->candidates()->num_regions();
   const size_t gc_num = (size_t) MAX2(G1MixedGCCountTarget, (uintx) 1);
   size_t result = region_num / gc_num;
   // emulate ceiling
--- a/src/hotspot/share/gc/g1/heapRegion.hpp	Fri Feb 08 12:55:20 2019 +0100
+++ b/src/hotspot/share/gc/g1/heapRegion.hpp	Fri Feb 08 12:55:20 2019 +0100
@@ -547,7 +547,7 @@
   }
 
   void calc_gc_efficiency(void);
-  double gc_efficiency() { return _gc_efficiency;}
+  double gc_efficiency() const { return _gc_efficiency;}
 
   uint index_in_opt_cset() const { return _index_in_opt_cset; }
   void set_index_in_opt_cset(uint index) { _index_in_opt_cset = index; }
@@ -705,7 +705,7 @@
 class HeapRegionClosure : public StackObj {
   friend class HeapRegionManager;
   friend class G1CollectionSet;
-  friend class CollectionSetChooser;
+  friend class G1CollectionSetCandidates;
 
   bool _is_complete;
   void set_incomplete() { _is_complete = false; }
--- a/src/hotspot/share/memory/allocation.cpp	Fri Feb 08 12:55:20 2019 +0100
+++ b/src/hotspot/share/memory/allocation.cpp	Fri Feb 08 12:55:20 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2019, 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
@@ -169,8 +169,7 @@
   ResourceObj* resobj = (ResourceObj *)res;
   resobj->_allocation_t[0] = ~(allocation + type);
   if (type != STACK_OR_EMBEDDED) {
-    // Called from operator new() and CollectionSetChooser(),
-    // set verification value.
+    // Called from operator new(), set verification value.
     resobj->_allocation_t[1] = (uintptr_t)&(resobj->_allocation_t[1]) + type;
   }
 }