8289538: Make G1BlockOffsetTablePart unaware of block sizes

Reviewed-by: ayang, iwalulya

src/hotspot/share/gc/g1/g1BlockOffsetTable.cpp

@@ -134,9 +134,10 @@ void G1BlockOffsetTablePart::set_remainder_to_point_to_start_incl(size_t start_c
     start_card_for_region = reach + 1;
   }
   assert(start_card_for_region > end_card, "Sanity check");
-  DEBUG_ONLY(check_all_cards(start_card, end_card);)
+  check_all_cards(start_card, end_card);
 }
 
+#ifdef ASSERT
 // The card-interval [start_card, end_card] is a closed interval; this
 // is an expensive check -- use with care and only under protection of
 // suitable flag.
@@ -174,6 +175,7 @@ void G1BlockOffsetTablePart::check_all_cards(size_t start_card, size_t end_card)
     }
   }
 }
+#endif
 
 //
 //              cur_card_boundary
@@ -262,7 +264,7 @@ void G1BlockOffsetTablePart::verify() const {
       HeapWord* obj_end = card_address - entry;
       while (obj_end < card_address) {
         HeapWord* obj = obj_end;
-        size_t obj_size = block_size(obj);
+        size_t obj_size = _hr->block_size(obj);
         obj_end = obj + obj_size;
         guarantee(obj_end > obj && obj_end <= _hr->top(),
                   "Invalid object end. obj: " PTR_FORMAT " obj_size: " SIZE_FORMAT " obj_end: " PTR_FORMAT " top: " PTR_FORMAT,

src/hotspot/share/gc/g1/g1BlockOffsetTable.hpp

@@ -108,13 +108,12 @@ public:
 
 class G1BlockOffsetTablePart {
   friend class G1BlockOffsetTable;
-  friend class HeapRegion;
   friend class VMStructs;
 private:
   // This is the global BlockOffsetTable.
   G1BlockOffsetTable* _bot;
 
-  // The region that owns this subregion.
+  // The region that owns this part of the BOT.
   HeapRegion* _hr;
 
   // Sets the entries corresponding to the cards starting at "start" and ending
@@ -122,7 +121,6 @@ private:
   void set_remainder_to_point_to_start_incl(size_t start, size_t end);
 
   inline size_t block_size(const HeapWord* p) const;
-  inline size_t block_size(const HeapWord* p, HeapWord* pb) const;
 
   // Returns the address of a block whose start is at most "addr".
   inline HeapWord* block_at_or_preceding(const void* addr) const;
@@ -130,7 +128,6 @@ private:
   // Return the address of the beginning of the block that contains "addr".
   // "q" is a block boundary that is <= "addr"; "n" is the address of the
   // next block (or the end of the space.)
-  // "pb" is the current value of the region's parsable_bottom.
   inline HeapWord* forward_to_block_containing_addr(HeapWord* q, HeapWord* n,
                                                     const void* addr,
                                                     HeapWord* pb) const;
@@ -138,13 +135,16 @@ private:
   // Update BOT entries corresponding to the mem range [blk_start, blk_end).
   void update_for_block_work(HeapWord* blk_start, HeapWord* blk_end);
 
-  void check_all_cards(size_t left_card, size_t right_card) const;
+  void check_all_cards(size_t left_card, size_t right_card) const NOT_DEBUG_RETURN;
 
-public:
   static HeapWord* align_up_by_card_size(HeapWord* const addr) {
     return align_up(addr, BOTConstants::card_size());
   }
 
+  void update_for_block(HeapWord* blk_start, size_t size) {
+    update_for_block(blk_start, blk_start + size);
+  }
+public:
   static bool is_crossing_card_boundary(HeapWord* const obj_start,
                                         HeapWord* const obj_end) {
     HeapWord* cur_card_boundary = align_up_by_card_size(obj_start);
@@ -157,13 +157,9 @@ public:
 
   void verify() const;
 
-  // Returns the address of the start of the block containing "addr", or
+  // Returns the address of the start of the block reaching into the card containing
-  // else "null" if it is covered by no block.  (May have side effects,
+  // "addr".
-  // namely updating of shared array entries that "point" too far
+  inline HeapWord* block_start_reaching_into_card(const void* addr) const;
-  // backwards.  This can occur, for example, when lab allocation is used
-  // in a space covered by the table.)
-  // "pb" is the current value of the region's parsable_bottom.
-  inline HeapWord* block_start(const void* addr, HeapWord* pb);
 
   void update_for_block(HeapWord* blk_start, HeapWord* blk_end) {
     if (is_crossing_card_boundary(blk_start, blk_end)) {
@@ -171,10 +167,6 @@ public:
     }
   }
 
-  void update_for_block(HeapWord* blk_start, size_t size) {
-    update_for_block(blk_start, blk_start + size);
-  }
-
   void set_for_starts_humongous(HeapWord* obj_top, size_t fill_size);
 
   void print_on(outputStream* out) PRODUCT_RETURN;

src/hotspot/share/gc/g1/g1BlockOffsetTable.inline.hpp

@@ -32,11 +32,32 @@
 #include "runtime/atomic.hpp"
 #include "oops/oop.inline.hpp"
 
-inline HeapWord* G1BlockOffsetTablePart::block_start(const void* addr, HeapWord* const pb) {
+inline HeapWord* G1BlockOffsetTablePart::block_start_reaching_into_card(const void* addr) const {
   assert(addr >= _hr->bottom() && addr < _hr->top(), "invalid address");
-  HeapWord* q = block_at_or_preceding(addr);
+
-  HeapWord* n = q + block_size(q, pb);
+#ifdef ASSERT
-  return forward_to_block_containing_addr(q, n, addr, pb);
+  if (!_hr->is_continues_humongous()) {
+    // For non-ContinuesHumongous regions, the first obj always starts from bottom.
+    u_char offset = _bot->offset_array(_bot->index_for(_hr->bottom()));
+    assert(offset == 0, "Found offset %u instead of 0 for region %u %s",
+           offset, _hr->hrm_index(), _hr->get_short_type_str());
+  }
+#endif
+
+  size_t index = _bot->index_for(addr);
+
+  uint offset = _bot->offset_array(index);
+  while (offset >= BOTConstants::card_size_in_words()) {
+    // The excess of the offset from N_words indicates a power of Base
+    // to go back by.
+    size_t n_cards_back = BOTConstants::entry_to_cards_back(offset);
+    index -= n_cards_back;
+    offset = _bot->offset_array(index);
+  }
+  assert(offset < BOTConstants::card_size_in_words(), "offset too large");
+
+  HeapWord* q = _bot->address_for_index(index);
+  return q - offset;
 }
 
 u_char G1BlockOffsetTable::offset_array(size_t index) const {
@@ -95,59 +116,4 @@ inline HeapWord* G1BlockOffsetTable::address_for_index(size_t index) const {
   return result;
 }
 
-inline size_t G1BlockOffsetTablePart::block_size(const HeapWord* p) const {
-  return _hr->block_size(p);
-}
-
-inline size_t G1BlockOffsetTablePart::block_size(const HeapWord* p, HeapWord* const pb) const {
-  return _hr->block_size(p, pb);
-}
-
-inline HeapWord* G1BlockOffsetTablePart::block_at_or_preceding(const void* addr) const {
-#ifdef ASSERT
-  if (!_hr->is_continues_humongous()) {
-    // For non-ContinuesHumongous regions, the first obj always starts from bottom.
-    u_char offset = _bot->offset_array(_bot->index_for(_hr->bottom()));
-    assert(offset == 0, "Found offset %u instead of 0 for region %u %s",
-           offset, _hr->hrm_index(), _hr->get_short_type_str());
-  }
-#endif
-
-  size_t index = _bot->index_for(addr);
-
-  uint offset = _bot->offset_array(index);  // Extend u_char to uint.
-  while (offset >= BOTConstants::card_size_in_words()) {
-    // The excess of the offset from N_words indicates a power of Base
-    // to go back by.
-    size_t n_cards_back = BOTConstants::entry_to_cards_back(offset);
-    index -= n_cards_back;
-    offset = _bot->offset_array(index);
-  }
-  assert(offset < BOTConstants::card_size_in_words(), "offset too large");
-
-  HeapWord* q = _bot->address_for_index(index);
-  return q - offset;
-}
-
-inline HeapWord* G1BlockOffsetTablePart::forward_to_block_containing_addr(HeapWord* q, HeapWord* n,
-                                                                          const void* addr,
-                                                                          HeapWord* const pb) const {
-  while (n <= addr) {
-    // When addr is not covered by the block starting at q we need to
-    // step forward until we find the correct block. With the BOT
-    // being precise, we should never have to step through more than
-    // a single card.
-    assert(_bot->index_for(n) == _bot->index_for(addr),
-           "BOT not precise. Index for n: " SIZE_FORMAT " must be equal to the index for addr: " SIZE_FORMAT,
-           _bot->index_for(n), _bot->index_for(addr));
-    q = n;
-    assert(cast_to_oop(q)->klass_or_null() != nullptr,
-        "start of block must be an initialized object");
-    n += block_size(q, pb);
-  }
-  assert(q <= addr, "wrong order for q and addr");
-  assert(addr < n, "wrong order for addr and n");
-  return q;
-}
-
 #endif // SHARE_GC_G1_G1BLOCKOFFSETTABLE_INLINE_HPP

src/hotspot/share/gc/g1/g1CollectedHeap.cpp

@@ -2346,7 +2346,7 @@ HeapWord* G1CollectedHeap::block_start(const void* addr) const {
   if (addr >= hr->top()) {
     return nullptr;
   }
-  return hr->block_start(addr, hr->parsable_bottom_acquire());
+  return hr->block_start(addr);
 }
 
 bool G1CollectedHeap::block_is_obj(const HeapWord* addr) const {

src/hotspot/share/gc/g1/heapRegion.hpp

@@ -144,10 +144,19 @@ private:
   // This version synchronizes with other calls to par_allocate_impl().
   inline HeapWord* par_allocate_impl(size_t min_word_size, size_t desired_word_size, size_t* actual_word_size);
 
+  // Return the address of the beginning of the block that contains "addr".
+  // "q" is a block boundary that is <= "addr"; "n" is the address of the
+  // next block (or the end of the HeapRegion.)
+  inline HeapWord* forward_to_block_containing_addr(HeapWord* q, HeapWord* n,
+                                                    const void* addr,
+                                                    HeapWord* pb) const;
+
   static bool obj_is_filler(oop obj);
 
 public:
-  HeapWord* block_start(const void* addr, HeapWord* const pb);
+  // Returns the address of the block reaching into or starting at addr.
+  HeapWord* block_start(const void* addr) const;
+  HeapWord* block_start(const void* addr, HeapWord* const pb) const;
 
   void object_iterate(ObjectClosure* blk);
 

src/hotspot/share/gc/g1/heapRegion.inline.hpp

@@ -82,8 +82,35 @@ inline HeapWord* HeapRegion::par_allocate_impl(size_t min_word_size,
   } while (true);
 }
 
-inline HeapWord* HeapRegion::block_start(const void* addr, HeapWord* const pb) {
+inline HeapWord* HeapRegion::forward_to_block_containing_addr(HeapWord* q, HeapWord* n,
-  return _bot_part.block_start(addr, pb);
+                                                              const void* addr,
+                                                              HeapWord* pb) const {
+  while (n <= addr) {
+    // When addr is not covered by the block starting at q we need to
+    // step forward until we find the correct block. With the BOT
+    // being precise, we should never have to step through more than
+    // a single card.
+    assert(!G1BlockOffsetTablePart::is_crossing_card_boundary(n, (HeapWord*)addr), "must be");
+    q = n;
+    assert(cast_to_oop(q)->klass_or_null() != nullptr,
+        "start of block must be an initialized object");
+    n += block_size(q, pb);
+  }
+  assert(q <= addr, "wrong order for q and addr");
+  assert(addr < n, "wrong order for addr and n");
+  return q;
+}
+
+inline HeapWord* HeapRegion::block_start(const void* addr) const {
+  return block_start(addr, parsable_bottom_acquire());
+}
+
+inline HeapWord* HeapRegion::block_start(const void* addr, HeapWord* const pb) const {
+  HeapWord* q = _bot_part.block_start_reaching_into_card(addr);
+  // The returned address is the block that reaches into the card of addr. Walk
+  // the heap to get to the block reaching into addr.
+  HeapWord* n = q + block_size(q, pb);
+  return forward_to_block_containing_addr(q, n, addr, pb);
 }
 
 inline bool HeapRegion::obj_in_unparsable_area(oop obj, HeapWord* const pb) {