8385606: Shenandoah: Remove PLAB card-table alignment and per-object register_object during old-gen allocation

Reviewed-by: wkemper, kdnilsen

src/hotspot/share/gc/shenandoah/shenandoahAffiliation.hpp

@@ -25,6 +25,8 @@
 #ifndef SHARE_GC_SHENANDOAH_SHENANDOAHAFFILIATION_HPP
 #define SHARE_GC_SHENANDOAH_SHENANDOAHAFFILIATION_HPP
 
+#include "utilities/debug.hpp"
+
 enum ShenandoahAffiliation {
   FREE,
   YOUNG_GENERATION,

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.cpp

@@ -1478,55 +1478,6 @@ HeapWord* ShenandoahFreeSet::try_allocate_from_mutator(ShenandoahAllocRequest& r
   return nullptr;
 }
 
-// This work method takes an argument corresponding to the number of bytes
-// free in a region, and returns the largest amount in heapwords that can be allocated
-// such that both of the following conditions are satisfied:
-//
-// 1. it is a multiple of card size
-// 2. any remaining shard may be filled with a filler object
-//
-// The idea is that the allocation starts and ends at card boundaries. Because
-// a region ('s end) is card-aligned, the remainder shard that must be filled is
-// at the start of the free space.
-//
-// This is merely a helper method to use for the purpose of such a calculation.
-size_t ShenandoahFreeSet::get_usable_free_words(size_t free_bytes) const {
-  // e.g. card_size is 512, card_shift is 9, min_fill_size() is 8
-  //      free is 514
-  //      usable_free is 512, which is decreased to 0
-  size_t usable_free = (free_bytes / CardTable::card_size()) << CardTable::card_shift();
-  assert(usable_free <= free_bytes, "Sanity check");
-  if ((free_bytes != usable_free) && (free_bytes - usable_free < ShenandoahHeap::min_fill_size() * HeapWordSize)) {
-    // After aligning to card multiples, the remainder would be smaller than
-    // the minimum filler object, so we'll need to take away another card's
-    // worth to construct a filler object.
-    if (usable_free >= CardTable::card_size()) {
-      usable_free -= CardTable::card_size();
-    } else {
-      assert(usable_free == 0, "usable_free is a multiple of card_size and card_size > min_fill_size");
-    }
-  }
-
-  return usable_free / HeapWordSize;
-}
-
-// Given a size argument, which is a multiple of card size, a request struct
-// for a PLAB, and an old region, return a pointer to the allocated space for
-// a PLAB which is card-aligned and where any remaining shard in the region
-// has been suitably filled by a filler object.
-// It is assumed (and assertion-checked) that such an allocation is always possible.
-HeapWord* ShenandoahFreeSet::allocate_aligned_plab(size_t size, ShenandoahAllocRequest& req, ShenandoahHeapRegion* r) {
-  assert(_heap->mode()->is_generational(), "PLABs are only for generational mode");
-  assert(r->is_old(), "All PLABs reside in old-gen");
-  assert(!req.is_mutator_alloc(), "PLABs should not be allocated by mutators.");
-  assert(is_aligned(size, CardTable::card_size_in_words()), "Align by design");
-
-  HeapWord* result = r->allocate_aligned(size, req, CardTable::card_size());
-  assert(result != nullptr, "Allocation cannot fail");
-  assert(r->top() <= r->end(), "Allocation cannot span end of region");
-  assert(is_aligned(result, CardTable::card_size_in_words()), "Align by design");
-  return result;
-}
 
 HeapWord* ShenandoahFreeSet::try_allocate_in(ShenandoahHeapRegion* r, ShenandoahAllocRequest& req, bool& in_new_region) {
   assert (has_alloc_capacity(r), "Performance: should avoid full regions on this path: %zu", r->index());
@@ -1578,44 +1529,17 @@ HeapWord* ShenandoahFreeSet::try_allocate_in(ShenandoahHeapRegion* r, Shenandoah
   // req.size() is in words, r->free() is in bytes.
   if (req.is_lab_alloc()) {
     size_t adjusted_size = req.size();
-    size_t free = r->free();    // free represents bytes available within region r
-    if (req.is_old()) {
-      // This is a PLAB allocation(lab alloc in old gen)
-      assert(_heap->mode()->is_generational(), "PLABs are only for generational mode");
-      assert(_partitions.in_free_set(ShenandoahFreeSetPartitionId::OldCollector, r->index()),
-             "PLABS must be allocated in old_collector_free regions");
-
-      // Need to assure that plabs are aligned on multiple of card region
-      // Convert free from unaligned bytes to aligned number of words
-      size_t usable_free = get_usable_free_words(free);
-      if (adjusted_size > usable_free) {
-        adjusted_size = usable_free;
-      }
-      adjusted_size = align_down(adjusted_size, CardTable::card_size_in_words());
-      if (adjusted_size >= req.min_size()) {
-        result = allocate_aligned_plab(adjusted_size, req, r);
-        assert(result != nullptr, "allocate must succeed");
-        req.set_actual_size(adjusted_size);
-      } else {
-        // Otherwise, leave result == nullptr because the adjusted size is smaller than min size.
-        log_trace(gc, free)("Failed to shrink PLAB request (%zu) in region %zu to %zu"
-                            " because min_size() is %zu", req.size(), r->index(), adjusted_size, req.min_size());
-      }
+    size_t free = align_down(r->free() >> LogHeapWordSize, MinObjAlignment);
+    if (adjusted_size > free) {
+      adjusted_size = free;
+    }
+    if (adjusted_size >= req.min_size()) {
+      result = r->allocate(adjusted_size, req);
+      assert (result != nullptr, "Allocation must succeed: free %zu, actual %zu", free, adjusted_size);
+      req.set_actual_size(adjusted_size);
     } else {
-      // This is a GCLAB or a TLAB allocation
-      // Convert free from unaligned bytes to aligned number of words
-      free = align_down(free >> LogHeapWordSize, MinObjAlignment);
-      if (adjusted_size > free) {
-        adjusted_size = free;
-      }
-      if (adjusted_size >= req.min_size()) {
-        result = r->allocate(adjusted_size, req);
-        assert (result != nullptr, "Allocation must succeed: free %zu, actual %zu", free, adjusted_size);
-        req.set_actual_size(adjusted_size);
-      } else {
-        log_trace(gc, free)("Failed to shrink TLAB or GCLAB request (%zu) in region %zu to %zu"
-                            " because min_size() is %zu", req.size(), r->index(), adjusted_size, req.min_size());
-      }
+      log_trace(gc, free)("Failed to shrink LAB request (%zu) in region %zu to %zu"
+                          " because min_size() is %zu", req.size(), r->index(), adjusted_size, req.min_size());
     }
   } else {
     size_t size = req.size();

src/hotspot/share/gc/shenandoah/shenandoahFreeSet.hpp

@@ -453,7 +453,6 @@ private:
   // locks will acquire them in the same order: first the global heap lock and then the rebuild lock.
   ShenandoahRebuildLock _rebuild_lock;
 
-  HeapWord* allocate_aligned_plab(size_t size, ShenandoahAllocRequest& req, ShenandoahHeapRegion* r);
 
   size_t _total_humongous_waste;
 
@@ -639,7 +638,6 @@ private:
 
   // Determine whether we prefer to allocate from left to right or from right to left within the OldCollector free-set.
   void establish_old_collector_alloc_bias();
-  size_t get_usable_free_words(size_t free_bytes) const;
 
   void reduce_young_reserve(size_t adjusted_young_reserve, size_t requested_young_reserve);
   void reduce_old_reserve(size_t adjusted_old_reserve, size_t requested_old_reserve);

src/hotspot/share/gc/shenandoah/shenandoahGenerationalHeap.cpp

@@ -65,12 +65,11 @@ protected:
 };
 
 size_t ShenandoahGenerationalHeap::calculate_min_plab() {
-  return align_up(PLAB::min_size(), CardTable::card_size_in_words());
+  return PLAB::min_size();
 }
 
 size_t ShenandoahGenerationalHeap::calculate_max_plab() {
-  size_t MaxTLABSizeWords = ShenandoahHeapRegion::max_tlab_size_words();
-  return align_down(MaxTLABSizeWords, CardTable::card_size_in_words());
+  return ShenandoahHeapRegion::max_tlab_size_words();
 }
 
 // Returns size in bytes
@@ -86,8 +85,6 @@ ShenandoahGenerationalHeap::ShenandoahGenerationalHeap(ShenandoahCollectorPolicy
   _regulator_thread(nullptr),
   _young_gen_memory_pool(nullptr),
   _old_gen_memory_pool(nullptr) {
-  assert(is_aligned(_min_plab_size, CardTable::card_size_in_words()), "min_plab_size must be aligned");
-  assert(is_aligned(_max_plab_size, CardTable::card_size_in_words()), "max_plab_size must be aligned");
 }
 
 void ShenandoahGenerationalHeap::initialize_generations() {

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp

@@ -1035,7 +1035,6 @@ HeapWord* ShenandoahHeap::allocate_memory_under_lock(ShenandoahAllocRequest& req
   // memory.
   HeapWord* result = _free_set->allocate(req, in_new_region);
 
-  // Record the plab configuration for this result and register the object.
   if (result != nullptr) {
     if (req.is_mutator_alloc()) {
       _alloc_rate.allocated((req.actual_size() + req.waste()) * HeapWordSize);
@@ -1044,33 +1043,10 @@ HeapWord* ShenandoahHeap::allocate_memory_under_lock(ShenandoahAllocRequest& req
     if (req.is_old()) {
       if (req.is_lab_alloc()) {
         old_generation()->configure_plab_for_current_thread(req);
-      } else {
-        // Register the newly allocated object while we're holding the global lock since there's no synchronization
-        // built in to the implementation of register_object().  There are potential races when multiple independent
-        // threads are allocating objects, some of which might span the same card region.  For example, consider
-        // a card table's memory region within which three objects are being allocated by three different threads:
-        //
-        // objects being "concurrently" allocated:
-        //    [-----a------][-----b-----][--------------c------------------]
-        //            [---- card table memory range --------------]
-        //
-        // Before any objects are allocated, this card's memory range holds no objects.  Note that allocation of object a
-        // wants to set the starts-object, first-start, and last-start attributes of the preceding card region.
-        // Allocation of object b wants to set the starts-object, first-start, and last-start attributes of this card region.
-        // Allocation of object c also wants to set the starts-object, first-start, and last-start attributes of this
-        // card region.
-        //
-        // The thread allocating b and the thread allocating c can "race" in various ways, resulting in confusion, such as
-        // last-start representing object b while first-start represents object c.  This is why we need to require all
-        // register_object() invocations to be "mutually exclusive" with respect to each card's memory range.
-        old_generation()->card_scan()->register_object(result);
-
-        if (req.is_promotion()) {
-          // Shared promotion.
-          const size_t actual_size = req.actual_size() * HeapWordSize;
-          log_debug(gc, plab)("Expend shared promotion of %zu bytes", actual_size);
-          old_generation()->expend_promoted(actual_size);
-        }
+      } else if (req.is_promotion()) {
+        const size_t actual_size = req.actual_size() * HeapWordSize;
+        log_debug(gc, plab)("Expend shared promotion of %zu bytes", actual_size);
+        old_generation()->expend_promoted(actual_size);
       }
     }
   }

src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.hpp

@@ -386,12 +386,6 @@ public:
   HeapWord* get_top_at_evac_start() const { return _top_at_evac_start; }
   void record_top_at_evac_start()         { _top_at_evac_start = _top; }
 
-  // If next available memory is not aligned on address that is multiple of alignment, fill the empty space
-  // so that returned object is aligned on an address that is a multiple of alignment_in_bytes.  Requested
-  // size is in words.  It is assumed that this->is_old().  A pad object is allocated, filled, and registered
-  // if necessary to assure the new allocation is properly aligned.  Return nullptr if memory is not available.
-  inline HeapWord* allocate_aligned(size_t word_size, ShenandoahAllocRequest &req, size_t alignment_in_bytes);
-
   // Allocation (return nullptr if full)
   inline HeapWord* allocate(size_t word_size, const ShenandoahAllocRequest& req);
 

src/hotspot/share/gc/shenandoah/shenandoahHeapRegion.inline.hpp

@@ -33,59 +33,6 @@
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
 #include "gc/shenandoah/shenandoahOldGeneration.hpp"
 
-HeapWord* ShenandoahHeapRegion::allocate_aligned(size_t size, ShenandoahAllocRequest &req, size_t alignment_in_bytes) {
-  shenandoah_assert_heaplocked_or_safepoint();
-  assert(req.is_lab_alloc(), "allocate_aligned() only applies to LAB allocations");
-  assert(is_object_aligned(size), "alloc size breaks alignment: %zu", size);
-  assert(is_old(), "aligned allocations are only taken from OLD regions to support PLABs");
-  assert(is_aligned(alignment_in_bytes, HeapWordSize), "Expect heap word alignment");
-
-  HeapWord* orig_top = top();
-  size_t alignment_in_words = alignment_in_bytes / HeapWordSize;
-
-  // unalignment_words is the amount by which current top() exceeds the desired alignment point.  We subtract this amount
-  // from alignment_in_words to determine padding required to next alignment point.
-
-  HeapWord* aligned_obj = (HeapWord*) align_up(orig_top, alignment_in_bytes);
-  size_t pad_words = aligned_obj - orig_top;
-  if ((pad_words > 0) && (pad_words < ShenandoahHeap::min_fill_size())) {
-    pad_words += alignment_in_words;
-    aligned_obj += alignment_in_words;
-  }
-
-  if (pointer_delta(end(), aligned_obj) < size) {
-    // Shrink size to fit within available space and align it
-    size = pointer_delta(end(), aligned_obj);
-    size = align_down(size, alignment_in_words);
-  }
-
-  // Both originally requested size and adjusted size must be properly aligned
-  assert (is_aligned(size, alignment_in_words), "Size must be multiple of alignment constraint");
-  if (size >= req.min_size()) {
-    // Even if req.min_size() may not be a multiple of card size, we know that size is.
-    if (pad_words > 0) {
-      assert(pad_words >= ShenandoahHeap::min_fill_size(), "pad_words expanded above to meet size constraint");
-      ShenandoahHeap::fill_with_object(orig_top, pad_words);
-      ShenandoahGenerationalHeap::heap()->old_generation()->card_scan()->register_object(orig_top);
-    }
-
-    make_regular_allocation(req.affiliation());
-    adjust_alloc_metadata(req, size);
-
-    HeapWord* new_top = aligned_obj + size;
-    assert(new_top <= end(), "PLAB cannot span end of heap region");
-    set_top(new_top);
-    // We do not req.set_actual_size() here.  The caller sets it.
-    req.set_waste(pad_words);
-    assert(is_object_aligned(new_top), "new top breaks alignment: " PTR_FORMAT, p2i(new_top));
-    assert(is_aligned(aligned_obj, alignment_in_bytes), "obj is not aligned: " PTR_FORMAT, p2i(aligned_obj));
-    return aligned_obj;
-  } else {
-    // The aligned size that fits in this region is smaller than min_size, so don't align top and don't allocate.  Return failure.
-    return nullptr;
-  }
-}
-
 HeapWord* ShenandoahHeapRegion::allocate_fill(size_t size) {
   shenandoah_assert_heaplocked_or_safepoint();
   assert(is_object_aligned(size), "alloc size breaks alignment: %zu", size);

src/hotspot/share/gc/shenandoah/shenandoahPLAB.cpp

@@ -22,7 +22,6 @@
  *
  */
 
-#include "gc/shared/cardTable.hpp"
 #include "gc/shenandoah/shenandoahAllocRequest.hpp"
 #include "gc/shenandoah/shenandoahGenerationalHeap.hpp"
 #include "gc/shenandoah/shenandoahHeap.inline.hpp"
@@ -43,7 +42,7 @@ ShenandoahPLAB::ShenandoahPLAB() :
   _allows_promotion(false),
   _retries_enabled(false),
   _heap(ShenandoahGenerationalHeap::heap()) {
-  _plab = new PLAB(align_up(PLAB::min_size(), CardTable::card_size_in_words()));
+  _plab = new PLAB(PLAB::min_size());
 }
 
 ShenandoahPLAB::~ShenandoahPLAB() {
@@ -93,10 +92,8 @@ HeapWord* ShenandoahPLAB::allocate(size_t size, bool is_promotion) {
 HeapWord* ShenandoahPLAB::allocate_slow(size_t size, bool is_promotion) {
   assert(_heap->mode()->is_generational(), "PLABs only relevant to generational GC");
 
-  // PLABs are aligned to card boundaries to avoid synchronization with concurrent
-  // allocations in other PLABs.
   const size_t plab_min_size = _heap->plab_min_size();
-  const size_t min_size = (size > plab_min_size)? align_up(size, CardTable::card_size_in_words()): plab_min_size;
+  const size_t min_size = (size > plab_min_size) ? size : plab_min_size;
 
   // Figure out size of new PLAB, using value determined at last refill.
   size_t cur_size = _desired_size;
@@ -105,12 +102,7 @@ HeapWord* ShenandoahPLAB::allocate_slow(size_t size, bool is_promotion) {
   }
 
   // Expand aggressively, doubling at each refill in this epoch, ceiling at plab_max_size()
-  // Doubling, starting at a card-multiple, should give us a card-multiple. (Ceiling and floor
-  // are card multiples.)
   const size_t future_size = MIN2(cur_size * 2, _heap->plab_max_size());
-  assert(is_aligned(future_size, CardTable::card_size_in_words()), "Card multiple by construction, future_size: %zu"
-          ", card_size: %u, cur_size: %zu, max: %zu",
-         future_size, CardTable::card_size_in_words(), cur_size, _heap->plab_max_size());
 
   // Record new heuristic value even if we take any shortcut. This captures
   // the case when moderately-sized objects always take a shortcut. At some point,
@@ -128,8 +120,6 @@ HeapWord* ShenandoahPLAB::allocate_slow(size_t size, bool is_promotion) {
 
   if (_plab->words_remaining() < plab_min_size) {
     // Retire current PLAB. This takes care of any PLAB book-keeping.
-    // retire_plab() registers the remnant filler object with the remembered set scanner without a lock.
-    // Since PLABs are card-aligned, concurrent registrations in other PLABs don't interfere.
     retire();
 
     size_t actual_size = 0;
@@ -157,7 +147,6 @@ HeapWord* ShenandoahPLAB::allocate_slow(size_t size, bool is_promotion) {
       Copy::fill_to_words(plab_buf + hdr_size, actual_size - hdr_size, badHeapWordVal);
 #endif
     }
-    assert(is_aligned(actual_size, CardTable::card_size_in_words()), "Align by design");
     _plab->set_buf(plab_buf, actual_size);
     if (is_promotion && !_allows_promotion) {
       return nullptr;
@@ -173,7 +162,6 @@ HeapWord* ShenandoahPLAB::allocate_slow(size_t size, bool is_promotion) {
 }
 
 HeapWord* ShenandoahPLAB::allocate_new_plab(size_t min_size, size_t word_size, size_t* actual_size) {
-  assert(is_aligned(min_size, CardTable::card_size_in_words()), "Align by design");
   assert(word_size >= min_size, "Requested PLAB is too small");
 
   ShenandoahAllocRequest req = ShenandoahAllocRequest::for_plab(min_size, word_size);
@@ -183,7 +171,6 @@ HeapWord* ShenandoahPLAB::allocate_new_plab(size_t min_size, size_t word_size, s
   } else {
     *actual_size = 0;
   }
-  assert(is_aligned(res, CardTable::card_size_in_words()), "Align by design");
   return res;
 }