8036025: Sort the freelist in order to shrink the heap

The free list is being maintained in a sorted fashion and old and humongous regions are allocated from the bottom of the heap while young regions are allocated at the top.

Co-authored-by: Staffan Friberg <staffan.friberg@oracle.com>
Reviewed-by: tschatzl, mgerdin

hotspot/src/share/vm/gc_implementation/g1/concurrentMark.cpp

@@ -1929,7 +1929,7 @@ public:
         }
       }
 
-      _cleanup_list->add_as_tail(&local_cleanup_list);
+      _cleanup_list->add_ordered(&local_cleanup_list);
       assert(local_cleanup_list.is_empty(), "post-condition");
 
       HeapRegionRemSet::finish_cleanup_task(&hrrs_cleanup_task);
@@ -2158,9 +2158,9 @@ void ConcurrentMark::completeCleanup() {
   // so it's not necessary to take any locks
   while (!_cleanup_list.is_empty()) {
     HeapRegion* hr = _cleanup_list.remove_head();
-    assert(hr != NULL, "the list was not empty");
+    assert(hr != NULL, "Got NULL from a non-empty list");
     hr->par_clear();
-    tmp_free_list.add_as_tail(hr);
+    tmp_free_list.add_ordered(hr);
 
     // Instead of adding one region at a time to the secondary_free_list,
     // we accumulate them in the local list and move them a few at a
@@ -2180,7 +2180,7 @@ void ConcurrentMark::completeCleanup() {
 
       {
         MutexLockerEx x(SecondaryFreeList_lock, Mutex::_no_safepoint_check_flag);
-        g1h->secondary_free_list_add_as_tail(&tmp_free_list);
+        g1h->secondary_free_list_add(&tmp_free_list);
         SecondaryFreeList_lock->notify_all();
       }
 

hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp

@@ -516,7 +516,7 @@ G1CollectedHeap* G1CollectedHeap::_g1h;
 // Private methods.
 
 HeapRegion*
-G1CollectedHeap::new_region_try_secondary_free_list() {
+G1CollectedHeap::new_region_try_secondary_free_list(bool is_old) {
   MutexLockerEx x(SecondaryFreeList_lock, Mutex::_no_safepoint_check_flag);
   while (!_secondary_free_list.is_empty() || free_regions_coming()) {
     if (!_secondary_free_list.is_empty()) {
@@ -532,7 +532,7 @@ G1CollectedHeap::new_region_try_secondary_free_list() {
 
       assert(!_free_list.is_empty(), "if the secondary_free_list was not "
              "empty we should have moved at least one entry to the free_list");
-      HeapRegion* res = _free_list.remove_head();
+      HeapRegion* res = _free_list.remove_region(is_old);
       if (G1ConcRegionFreeingVerbose) {
         gclog_or_tty->print_cr("G1ConcRegionFreeing [region alloc] : "
                                "allocated "HR_FORMAT" from secondary_free_list",
@@ -554,7 +554,7 @@ G1CollectedHeap::new_region_try_secondary_free_list() {
   return NULL;
 }
 
-HeapRegion* G1CollectedHeap::new_region(size_t word_size, bool do_expand) {
+HeapRegion* G1CollectedHeap::new_region(size_t word_size, bool is_old, bool do_expand) {
   assert(!isHumongous(word_size) || word_size <= HeapRegion::GrainWords,
          "the only time we use this to allocate a humongous region is "
          "when we are allocating a single humongous region");
@@ -566,19 +566,21 @@ HeapRegion* G1CollectedHeap::new_region(size_t word_size, bool do_expand) {
         gclog_or_tty->print_cr("G1ConcRegionFreeing [region alloc] : "
                                "forced to look at the secondary_free_list");
       }
-      res = new_region_try_secondary_free_list();
+      res = new_region_try_secondary_free_list(is_old);
       if (res != NULL) {
         return res;
       }
     }
   }
-  res = _free_list.remove_head_or_null();
+
+  res = _free_list.remove_region(is_old);
+
   if (res == NULL) {
     if (G1ConcRegionFreeingVerbose) {
       gclog_or_tty->print_cr("G1ConcRegionFreeing [region alloc] : "
                              "res == NULL, trying the secondary_free_list");
     }
-    res = new_region_try_secondary_free_list();
+    res = new_region_try_secondary_free_list(is_old);
   }
   if (res == NULL && do_expand && _expand_heap_after_alloc_failure) {
     // Currently, only attempts to allocate GC alloc regions set
@@ -595,12 +597,9 @@ HeapRegion* G1CollectedHeap::new_region(size_t word_size, bool do_expand) {
     if (expand(word_size * HeapWordSize)) {
       // Given that expand() succeeded in expanding the heap, and we
       // always expand the heap by an amount aligned to the heap
-      // region size, the free list should in theory not be empty. So
+      // region size, the free list should in theory not be empty.
-      // it would probably be OK to use remove_head(). But the extra
+      // In either case remove_region() will check for NULL.
-      // check for NULL is unlikely to be a performance issue here (we
+      res = _free_list.remove_region(is_old);
-      // just expanded the heap!) so let's just be conservative and
-      // use remove_head_or_null().
-      res = _free_list.remove_head_or_null();
     } else {
       _expand_heap_after_alloc_failure = false;
     }
@@ -618,7 +617,7 @@ uint G1CollectedHeap::humongous_obj_allocate_find_first(uint num_regions,
     // Only one region to allocate, no need to go through the slower
     // path. The caller will attempt the expansion if this fails, so
     // let's not try to expand here too.
-    HeapRegion* hr = new_region(word_size, false /* do_expand */);
+    HeapRegion* hr = new_region(word_size, true /* is_old */, false /* do_expand */);
     if (hr != NULL) {
       first = hr->hrs_index();
     } else {
@@ -5907,7 +5906,7 @@ void G1CollectedHeap::free_region(HeapRegion* hr,
     _cg1r->hot_card_cache()->reset_card_counts(hr);
   }
   hr->hr_clear(par, true /* clear_space */, locked /* locked */);
-  free_list->add_as_head(hr);
+  free_list->add_ordered(hr);
 }
 
 void G1CollectedHeap::free_humongous_region(HeapRegion* hr,
@@ -5947,7 +5946,7 @@ void G1CollectedHeap::prepend_to_freelist(FreeRegionList* list) {
   assert(list != NULL, "list can't be null");
   if (!list->is_empty()) {
     MutexLockerEx x(FreeList_lock, Mutex::_no_safepoint_check_flag);
-    _free_list.add_as_head(list);
+    _free_list.add_ordered(list);
   }
 }
 
@@ -6413,6 +6412,7 @@ HeapRegion* G1CollectedHeap::new_mutator_alloc_region(size_t word_size,
   bool young_list_full = g1_policy()->is_young_list_full();
   if (force || !young_list_full) {
     HeapRegion* new_alloc_region = new_region(word_size,
+                                              false /* is_old */,
                                               false /* do_expand */);
     if (new_alloc_region != NULL) {
       set_region_short_lived_locked(new_alloc_region);
@@ -6471,14 +6471,16 @@ HeapRegion* G1CollectedHeap::new_gc_alloc_region(size_t word_size,
   assert(FreeList_lock->owned_by_self(), "pre-condition");
 
   if (count < g1_policy()->max_regions(ap)) {
+    bool survivor = (ap == GCAllocForSurvived);
     HeapRegion* new_alloc_region = new_region(word_size,
+                                              !survivor,
                                               true /* do_expand */);
     if (new_alloc_region != NULL) {
       // We really only need to do this for old regions given that we
       // should never scan survivors. But it doesn't hurt to do it
       // for survivors too.
       new_alloc_region->set_saved_mark();
-      if (ap == GCAllocForSurvived) {
+      if (survivor) {
         new_alloc_region->set_survivor();
         _hr_printer.alloc(new_alloc_region, G1HRPrinter::Survivor);
       } else {

hotspot/src/share/vm/gc_implementation/g1/g1CollectedHeap.hpp

@@ -497,13 +497,14 @@ protected:
   // check whether there's anything available on the
   // secondary_free_list and/or wait for more regions to appear on
   // that list, if _free_regions_coming is set.
-  HeapRegion* new_region_try_secondary_free_list();
+  HeapRegion* new_region_try_secondary_free_list(bool is_old);
 
   // Try to allocate a single non-humongous HeapRegion sufficient for
   // an allocation of the given word_size. If do_expand is true,
   // attempt to expand the heap if necessary to satisfy the allocation
-  // request.
+  // request. If the region is to be used as an old region or for a
-  HeapRegion* new_region(size_t word_size, bool do_expand);
+  // humongous object, set is_old to true. If not, to false.
+  HeapRegion* new_region(size_t word_size, bool is_old, bool do_expand);
 
   // Attempt to satisfy a humongous allocation request of the given
   // size by finding a contiguous set of free regions of num_regions
@@ -1232,12 +1233,12 @@ public:
   // Wrapper for the region list operations that can be called from
   // methods outside this class.
 
-  void secondary_free_list_add_as_tail(FreeRegionList* list) {
+  void secondary_free_list_add(FreeRegionList* list) {
-    _secondary_free_list.add_as_tail(list);
+    _secondary_free_list.add_ordered(list);
   }
 
   void append_secondary_free_list() {
-    _free_list.add_as_head(&_secondary_free_list);
+    _free_list.add_ordered(&_secondary_free_list);
   }
 
   void append_secondary_free_list_if_not_empty_with_lock() {

hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp

@@ -356,7 +356,7 @@ HeapRegion::HeapRegion(uint hrs_index,
     _claimed(InitialClaimValue), _evacuation_failed(false),
     _prev_marked_bytes(0), _next_marked_bytes(0), _gc_efficiency(0.0),
     _young_type(NotYoung), _next_young_region(NULL),
-    _next_dirty_cards_region(NULL), _next(NULL), _pending_removal(false),
+    _next_dirty_cards_region(NULL), _next(NULL), _prev(NULL), _pending_removal(false),
 #ifdef ASSERT
     _containing_set(NULL),
 #endif // ASSERT

hotspot/src/share/vm/gc_implementation/g1/heapRegion.hpp

@@ -271,6 +271,7 @@ class HeapRegion: public G1OffsetTableContigSpace {
 
   // Fields used by the HeapRegionSetBase class and subclasses.
   HeapRegion* _next;
+  HeapRegion* _prev;
 #ifdef ASSERT
   HeapRegionSetBase* _containing_set;
 #endif // ASSERT
@@ -531,11 +532,13 @@ class HeapRegion: public G1OffsetTableContigSpace {
 
   // Methods used by the HeapRegionSetBase class and subclasses.
 
-  // Getter and setter for the next field used to link regions into
+  // Getter and setter for the next and prev fields used to link regions into
   // linked lists.
   HeapRegion* next()              { return _next; }
+  HeapRegion* prev()              { return _prev; }
 
   void set_next(HeapRegion* next) { _next = next; }
+  void set_prev(HeapRegion* prev) { _prev = prev; }
 
   // Every region added to a set is tagged with a reference to that
   // set. This is used for doing consistency checking to make sure that

hotspot/src/share/vm/gc_implementation/g1/heapRegionSet.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2014, 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
@@ -135,9 +135,11 @@ void FreeRegionList::add_as_head_or_tail(FreeRegionList* from_list, bool as_head
     assert(length() > 0 && _tail != NULL, hrs_ext_msg(this, "invariant"));
     if (as_head) {
       from_list->_tail->set_next(_head);
+      _head->set_prev(from_list->_tail);
       _head = from_list->_head;
     } else {
       _tail->set_next(from_list->_head);
+      from_list->_head->set_prev(_tail);
       _tail = from_list->_tail;
     }
   }
@@ -167,6 +169,7 @@ void FreeRegionList::remove_all() {
 
     HeapRegion* next = curr->next();
     curr->set_next(NULL);
+    curr->set_prev(NULL);
     curr->set_containing_set(NULL);
     curr = next;
   }
@@ -175,6 +178,74 @@ void FreeRegionList::remove_all() {
   verify_optional();
 }
 
+void FreeRegionList::add_ordered(FreeRegionList* from_list) {
+  check_mt_safety();
+  from_list->check_mt_safety();
+
+  verify_optional();
+  from_list->verify_optional();
+
+  if (from_list->is_empty()) {
+    return;
+  }
+
+  if (is_empty()) {
+    add_as_head(from_list);
+    return;
+  }
+
+  #ifdef ASSERT
+  FreeRegionListIterator iter(from_list);
+  while (iter.more_available()) {
+    HeapRegion* hr = iter.get_next();
+    // In set_containing_set() we check that we either set the value
+    // from NULL to non-NULL or vice versa to catch bugs. So, we have
+    // to NULL it first before setting it to the value.
+    hr->set_containing_set(NULL);
+    hr->set_containing_set(this);
+  }
+  #endif // ASSERT
+
+  HeapRegion* curr_to = _head;
+  HeapRegion* curr_from = from_list->_head;
+
+  while (curr_from != NULL) {
+    while (curr_to != NULL && curr_to->hrs_index() < curr_from->hrs_index()) {
+      curr_to = curr_to->next();
+    }
+
+    if (curr_to == NULL) {
+      // The rest of the from list should be added as tail
+      _tail->set_next(curr_from);
+      curr_from->set_prev(_tail);
+      curr_from = NULL;
+    } else {
+      HeapRegion* next_from = curr_from->next();
+
+      curr_from->set_next(curr_to);
+      curr_from->set_prev(curr_to->prev());
+      if (curr_to->prev() == NULL) {
+        _head = curr_from;
+      } else {
+        curr_to->prev()->set_next(curr_from);
+      }
+      curr_to->set_prev(curr_from);
+
+      curr_from = next_from;
+    }
+  }
+
+  if (_tail->hrs_index() < from_list->_tail->hrs_index()) {
+    _tail = from_list->_tail;
+  }
+
+  _count.increment(from_list->length(), from_list->total_capacity_bytes());
+  from_list->clear();
+
+  verify_optional();
+  from_list->verify_optional();
+}
+
 void FreeRegionList::remove_all_pending(uint target_count) {
   check_mt_safety();
   assert(target_count > 1, hrs_ext_msg(this, "pre-condition"));
@@ -184,11 +255,11 @@ void FreeRegionList::remove_all_pending(uint target_count) {
   DEBUG_ONLY(uint old_length = length();)
 
   HeapRegion* curr = _head;
-  HeapRegion* prev = NULL;
   uint count = 0;
   while (curr != NULL) {
     verify_region(curr);
     HeapRegion* next = curr->next();
+    HeapRegion* prev = curr->prev();
 
     if (curr->pending_removal()) {
       assert(count < target_count,
@@ -208,9 +279,14 @@ void FreeRegionList::remove_all_pending(uint target_count) {
         _tail = prev;
       } else {
         assert(_tail != curr, hrs_ext_msg(this, "invariant"));
+        next->set_prev(prev);
+      }
+      if (_last = curr) {
+        _last = NULL;
       }
 
       curr->set_next(NULL);
+      curr->set_prev(NULL);
       remove(curr);
       curr->set_pending_removal(false);
 
@@ -221,8 +297,6 @@ void FreeRegionList::remove_all_pending(uint target_count) {
       // carry on iterating to make sure there are not more regions
       // tagged with pending removal.
       DEBUG_ONLY(if (count == target_count) break;)
-    } else {
-      prev = curr;
     }
     curr = next;
   }
@@ -255,6 +329,7 @@ void FreeRegionList::clear() {
   _count = HeapRegionSetCount();
   _head = NULL;
   _tail = NULL;
+  _last = NULL;
 }
 
 void FreeRegionList::print_on(outputStream* out, bool print_contents) {
@@ -279,6 +354,9 @@ void FreeRegionList::verify_list() {
   HeapRegion* prev0 = NULL;
   uint count = 0;
   size_t capacity = 0;
+  uint last_index = 0;
+
+  guarantee(_head == NULL || _head->prev() == NULL, "_head should not have a prev");
   while (curr != NULL) {
     verify_region(curr);
 
@@ -286,6 +364,12 @@ void FreeRegionList::verify_list() {
     guarantee(count < _unrealistically_long_length,
         hrs_err_msg("[%s] the calculated length: %u seems very long, is there maybe a cycle? curr: "PTR_FORMAT" prev0: "PTR_FORMAT" " "prev1: "PTR_FORMAT" length: %u", name(), count, curr, prev0, prev1, length()));
 
+    if (curr->next() != NULL) {
+      guarantee(curr->next()->prev() == curr, "Next or prev pointers messed up");
+    }
+    guarantee(curr->hrs_index() == 0 || curr->hrs_index() > last_index, "List should be sorted");
+    last_index = curr->hrs_index();
+
     capacity += curr->capacity();
 
     prev1 = prev0;
@@ -294,7 +378,7 @@ void FreeRegionList::verify_list() {
   }
 
   guarantee(tail() == prev0, err_msg("Expected %s to end with %u but it ended with %u.", name(), tail()->hrs_index(), prev0->hrs_index()));
-
+  guarantee(_tail == NULL || _tail->next() == NULL, "_tail should not have a next");
   guarantee(length() == count, err_msg("%s count mismatch. Expected %u, actual %u.", name(), length(), count));
   guarantee(total_capacity_bytes() == capacity, err_msg("%s capacity mismatch. Expected " SIZE_FORMAT ", actual " SIZE_FORMAT,
       name(), total_capacity_bytes(), capacity));

hotspot/src/share/vm/gc_implementation/g1/heapRegionSet.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2014, 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
@@ -191,11 +191,14 @@ public:
   }
 };
 
-// A set that links all the regions added to it in a singly-linked
+// A set that links all the regions added to it in a doubly-linked
 // list. We should try to avoid doing operations that iterate over
 // such lists in performance critical paths. Typically we should
-// add / remove one region at a time or concatenate two lists. All
+// add / remove one region at a time or concatenate two lists. There are
-// those operations are done in constant time.
+// two ways to treat your lists, ordered and un-ordered. All un-ordered
+// operations are done in constant time. To keep a list ordered only use
+// add_ordered() to add elements to the list. If a list is not ordered
+// from start, there is no way to sort it later.
 
 class FreeRegionListIterator;
 
@@ -206,6 +209,11 @@ private:
   HeapRegion* _head;
   HeapRegion* _tail;
 
+  // _last is used to keep track of where we added an element the last
+  // time in ordered lists. It helps to improve performance when adding
+  // several ordered items in a row.
+  HeapRegion* _last;
+
   static uint _unrealistically_long_length;
 
   void add_as_head_or_tail(FreeRegionList* from_list, bool as_head);
@@ -229,6 +237,11 @@ public:
 
   static void set_unrealistically_long_length(uint len);
 
+  // Add hr to the list. The region should not be a member of another set.
+  // Assumes that the list is ordered and will preserve that order. The order
+  // is determined by hrs_index.
+  inline void add_ordered(HeapRegion* hr);
+
   // It adds hr to the list as the new head. The region should not be
   // a member of another set.
   inline void add_as_head(HeapRegion* hr);
@@ -244,6 +257,20 @@ public:
   // Convenience method.
   inline HeapRegion* remove_head_or_null();
 
+  // Removes and returns the last element (_tail) of the list. It assumes
+  // that the list isn't empty so that it can return a non-NULL value.
+  inline HeapRegion* remove_tail();
+
+  // Convenience method
+  inline HeapRegion* remove_tail_or_null();
+
+  // Removes from head or tail based on the given argument.
+  inline HeapRegion* remove_region(bool from_head);
+
+  // Merge two ordered lists. The result is also ordered. The order is
+  // determined by hrs_index.
+  void add_ordered(FreeRegionList* from_list);
+
   // It moves the regions from from_list to this list and empties
   // from_list. The new regions will appear in the same order as they
   // were in from_list and be linked in the beginning of this list.
@@ -276,7 +303,7 @@ public:
 class FreeRegionListIterator : public StackObj {
 private:
   FreeRegionList* _list;
-  HeapRegion*           _curr;
+  HeapRegion*     _curr;
 
 public:
   bool more_available() {
@@ -296,8 +323,7 @@ public:
     return hr;
   }
 
-  FreeRegionListIterator(FreeRegionList* list)
+  FreeRegionListIterator(FreeRegionList* list) : _curr(NULL), _list(list) {
-    : _curr(NULL), _list(list) {
     _curr = list->head();
   }
 };

hotspot/src/share/vm/gc_implementation/g1/heapRegionSet.inline.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2014, 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
@@ -47,6 +47,54 @@ inline void HeapRegionSetBase::remove(HeapRegion* hr) {
   _count.decrement(1u, hr->capacity());
 }
 
+inline void FreeRegionList::add_ordered(HeapRegion* hr) {
+  check_mt_safety();
+  assert((length() == 0 && _head == NULL && _tail == NULL) ||
+         (length() >  0 && _head != NULL && _tail != NULL),
+         hrs_ext_msg(this, "invariant"));
+  // add() will verify the region and check mt safety.
+  add(hr);
+
+  // Now link the region
+  if (_head != NULL) {
+    HeapRegion* curr;
+
+    if (_last != NULL && _last->hrs_index() < hr->hrs_index()) {
+      curr = _last;
+    } else {
+      curr = _head;
+    }
+
+    // Find first entry with a Region Index larger than entry to insert.
+    while (curr != NULL && curr->hrs_index() < hr->hrs_index()) {
+      curr = curr->next();
+    }
+
+    hr->set_next(curr);
+
+    if (curr == NULL) {
+      // Adding at the end
+      hr->set_prev(_tail);
+      _tail->set_next(hr);
+      _tail = hr;
+    } else if (curr->prev() == NULL) {
+      // Adding at the beginning
+      hr->set_prev(NULL);
+      _head = hr;
+      curr->set_prev(hr);
+    } else {
+      hr->set_prev(curr->prev());
+      hr->prev()->set_next(hr);
+      curr->set_prev(hr);
+    }
+  } else {
+    // The list was empty
+    _tail = hr;
+    _head = hr;
+  }
+  _last = hr;
+}
+
 inline void FreeRegionList::add_as_head(HeapRegion* hr) {
   assert((length() == 0 && _head == NULL && _tail == NULL) ||
          (length() >  0 && _head != NULL && _tail != NULL),
@@ -57,6 +105,7 @@ inline void FreeRegionList::add_as_head(HeapRegion* hr) {
   // Now link the region.
   if (_head != NULL) {
     hr->set_next(_head);
+    _head->set_prev(hr);
   } else {
     _tail = hr;
   }
@@ -68,12 +117,13 @@ inline void FreeRegionList::add_as_tail(HeapRegion* hr) {
   assert((length() == 0 && _head == NULL && _tail == NULL) ||
          (length() >  0 && _head != NULL && _tail != NULL),
          hrs_ext_msg(this, "invariant"));
-  // add() will verify the region and check mt safety
+  // add() will verify the region and check mt safety.
   add(hr);
 
   // Now link the region.
   if (_tail != NULL) {
     _tail->set_next(hr);
+    hr->set_prev(_tail);
   } else {
     _head = hr;
   }
@@ -90,10 +140,16 @@ inline HeapRegion* FreeRegionList::remove_head() {
   _head = hr->next();
   if (_head == NULL) {
     _tail = NULL;
+  } else {
+    _head->set_prev(NULL);
   }
   hr->set_next(NULL);
 
-  // remove() will verify the region and check mt safety
+  if (_last == hr) {
+    _last = NULL;
+  }
+
+  // remove() will verify the region and check mt safety.
   remove(hr);
   return hr;
 }
@@ -107,4 +163,47 @@ inline HeapRegion* FreeRegionList::remove_head_or_null() {
   }
 }
 
+inline HeapRegion* FreeRegionList::remove_tail() {
+  assert(!is_empty(), hrs_ext_msg(this, "The list should not be empty"));
+  assert(length() > 0 && _head != NULL && _tail != NULL,
+         hrs_ext_msg(this, "invariant"));
+
+  // We need to unlink it first
+  HeapRegion* hr = _tail;
+
+  _tail = hr->prev();
+  if (_tail == NULL) {
+    _head = NULL;
+  } else {
+    _tail->set_next(NULL);
+  }
+  hr->set_prev(NULL);
+
+  if (_last == hr) {
+    _last = NULL;
+  }
+
+  // remove() will verify the region and check mt safety.
+  remove(hr);
+  return hr;
+}
+
+inline HeapRegion* FreeRegionList::remove_tail_or_null() {
+  check_mt_safety();
+
+  if (!is_empty()) {
+    return remove_tail();
+  } else {
+    return NULL;
+  }
+}
+
+inline HeapRegion* FreeRegionList::remove_region(bool from_head) {
+  if (from_head) {
+    return remove_head_or_null();
+  } else {
+    return remove_tail_or_null();
+  }
+}
+
 #endif // SHARE_VM_GC_IMPLEMENTATION_G1_HEAPREGIONSET_INLINE_HPP