diff --git a/src/hotspot/share/gc/shared/markBitMap.hpp b/src/hotspot/share/gc/shared/markBitMap.hpp index a55a8220910ae..237b289489244 100644 --- a/src/hotspot/share/gc/shared/markBitMap.hpp +++ b/src/hotspot/share/gc/shared/markBitMap.hpp @@ -79,11 +79,19 @@ class MarkBitMap { } // Return the address corresponding to the next marked bit at or after - // "addr", and before "limit", if "limit" is non-null. If there is no - // such bit, returns "limit" if that is non-null, or else "endWord()". + // "addr", and before "limit", which are required to be non-null. In other + // words, we look for a marked address in the range [addr, limit). If there is no + // such bit, returns "limit". inline HeapWord* get_next_marked_addr(const HeapWord* addr, HeapWord* limit) const; + // Return the address corresponding to the last marked bit before + // "addr" but at or after "limit", which are required to be non-null. + // In other words, we look for a marked address in the range [limit, addr). + // If there is no such bit, returns "addr". + inline HeapWord* get_last_marked_addr(HeapWord* limit, + const HeapWord* addr) const; + void print_on(outputStream* st, const char* prefix) const; // Write marks. diff --git a/src/hotspot/share/gc/shared/markBitMap.inline.hpp b/src/hotspot/share/gc/shared/markBitMap.inline.hpp index cf6cf02cf320c..3dfe4ce0142b3 100644 --- a/src/hotspot/share/gc/shared/markBitMap.inline.hpp +++ b/src/hotspot/share/gc/shared/markBitMap.inline.hpp @@ -36,13 +36,26 @@ inline HeapWord* MarkBitMap::get_next_marked_addr(const HeapWord* const addr, HeapWord* const limit) const { assert(limit != nullptr, "limit must not be null"); - // Round addr up to a possible object boundary to be safe. + assert(addr != nullptr, "addr must not be null"); + // Round up to bitmap's alignment boundary to start looking size_t const addr_offset = addr_to_offset(align_up(addr, HeapWordSize << _shifter)); size_t const limit_offset = addr_to_offset(limit); size_t const nextOffset = _bm.find_first_set_bit(addr_offset, limit_offset); return offset_to_addr(nextOffset); } +inline HeapWord* MarkBitMap::get_last_marked_addr(HeapWord* const limit, + const HeapWord* const addr) const { + assert(limit != nullptr, "limit must not be null"); + assert(addr != nullptr, "addr must not be null"); + size_t const limit_offset = addr_to_offset(limit); + // Round down to bitmap's alignment boundary to start looking back + size_t const addr_offset = addr_to_offset(align_down(addr, HeapWordSize << _shifter)); + size_t const lastOffset = _bm.find_last_set_bit(limit_offset, addr_offset); + return offset_to_addr(lastOffset); +} + + inline void MarkBitMap::mark(HeapWord* addr) { check_mark(addr); _bm.set_bit(addr_to_offset(addr)); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.cpp b/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.cpp index 34e6af41b427c..401ecdc1476e4 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.cpp @@ -58,6 +58,25 @@ bool ShenandoahMarkBitMap::is_bitmap_clear_range(const HeapWord* start, const He } +HeapWord* ShenandoahMarkBitMap::get_last_marked_addr(const HeapWord* limit, + const HeapWord* addr) const { +#ifdef ASSERT + ShenandoahHeap* heap = ShenandoahHeap::heap(); + ShenandoahHeapRegion* r = heap->heap_region_containing(addr); + ShenandoahMarkingContext* ctx = heap->marking_context(); + HeapWord* tams = ctx->top_at_mark_start(r); + assert(limit != nullptr, "limit must not be null"); + assert(limit >= r->bottom(), "limit must be more than bottom"); + assert(addr <= tams, "addr must be less than TAMS"); +#endif + + // Round addr up to a possible object boundary to be safe. + size_t const addr_offset = address_to_index(align_down(addr, HeapWordSize << LogMinObjAlignment)); + size_t const limit_offset = address_to_index(limit); + size_t const nextOffset = get_last_one_offset(limit_offset, addr_offset); + return index_to_address(nextOffset); +} + HeapWord* ShenandoahMarkBitMap::get_next_marked_addr(const HeapWord* addr, const HeapWord* limit) const { #ifdef ASSERT diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.hpp b/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.hpp index 56daf4c595671..faa7abb531090 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.hpp @@ -119,7 +119,16 @@ class ShenandoahMarkBitMap { template inline idx_t get_next_bit_impl(idx_t l_index, idx_t r_index) const; + // Helper for get_last_{zero,one}_bit variants. + // - flip designates whether searching for 1s or 0s. Must be one of + // find_{zeros,ones}_flip. + // - aligned_left is true if l_index is a priori on a bm_word_t boundary. + template + inline idx_t get_last_bit_impl(idx_t l_index, idx_t r_index) const; + + inline idx_t get_next_one_offset (idx_t l_index, idx_t r_index) const; + inline idx_t get_last_one_offset (idx_t l_index, idx_t r_index) const; void clear_large_range (idx_t beg, idx_t end); @@ -162,12 +171,16 @@ class ShenandoahMarkBitMap { bool is_bitmap_clear_range(const HeapWord* start, const HeapWord* end) const; - // Return the address corresponding to the next marked bit at or after - // "addr", and before "limit", if "limit" is non-null. If there is no - // such bit, returns "limit" if that is non-null, or else "endWord()". + // Return the first marked address in the range [addr, limit), or limit + // if none found. HeapWord* get_next_marked_addr(const HeapWord* addr, const HeapWord* limit) const; + // Return the last marked address in the range (limit, addr], or limit + // if none found. + HeapWord* get_last_marked_addr(const HeapWord* limit, + const HeapWord* addr) const; + bm_word_t inverted_bit_mask_for_range(idx_t beg, idx_t end) const; void clear_range_within_word (idx_t beg, idx_t end); void clear_range (idx_t beg, idx_t end); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.inline.hpp index f0a9752b614c4..44c716c501312 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.inline.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkBitMap.inline.hpp @@ -171,10 +171,77 @@ inline ShenandoahMarkBitMap::idx_t ShenandoahMarkBitMap::get_next_bit_impl(idx_t return r_index; } +template +inline ShenandoahMarkBitMap::idx_t ShenandoahMarkBitMap::get_last_bit_impl(idx_t l_index, idx_t r_index) const { + STATIC_ASSERT(flip == find_ones_flip || flip == find_zeros_flip); + verify_range(l_index, r_index); + assert(!aligned_left || is_aligned(l_index, BitsPerWord), "l_index not aligned"); + + // The first word often contains an interesting bit, either due to + // density or because of features of the calling algorithm. So it's + // important to examine that first word with a minimum of fuss, + // minimizing setup time for later words that will be wasted if the + // first word is indeed interesting. + + // The benefit from aligned_left being true is relatively small. + // It saves an operation in the setup for the word search loop. + // It also eliminates the range check on the final result. + // However, callers often have a comparison with l_index, and + // inlining often allows the two comparisons to be combined; it is + // important when !aligned_left that return paths either return + // l_index or a value dominating a comparison with l_index. + // aligned_left is still helpful when the caller doesn't have a + // range check because features of the calling algorithm guarantee + // an interesting bit will be present. + + if (l_index < r_index) { + // Get the word containing r_index, and shift out low bits. + idx_t index = to_words_align_down(r_index); + bm_word_t cword = (map(index) ^ flip) >> bit_in_word(r_index); + if ((cword & 1) != 0) { + // The first bit is similarly often interesting. When it matters + // (density or features of the calling algorithm make it likely + // the first bit is set), going straight to the next clause compares + // poorly with doing this check first; count_trailing_zeros can be + // relatively expensive, plus there is the additional range check. + // But when the first bit isn't set, the cost of having tested for + // it is relatively small compared to the rest of the search. + return r_index; + } else if (cword != 0) { + // Flipped and shifted first word is non-zero. + idx_t result = r_index + count_trailing_zeros(cword); + if (aligned_left || (result > l_index)) return result; + // Result is beyond range bound; return l_index. + } else { + // Flipped and shifted first word is zero. Word search through + // aligned up r_index for a non-zero flipped word. + idx_t limit = aligned_left + ? to_words_align_down(l_index) // Minuscule savings when aligned. + : to_words_align_up(l_index); + while (--index > limit) { + cword = map(index) ^ flip; + if (cword != 0) { + idx_t result = bit_index(index) + count_trailing_zeros(cword); + if (aligned_left || (result > l_index)) return result; + // Result is beyond range bound; return r_index. + assert((index - 1) == limit, "invariant"); + break; + } + } + // No bits in range; return l_index. + } + } + return l_index; +} + inline ShenandoahMarkBitMap::idx_t ShenandoahMarkBitMap::get_next_one_offset(idx_t l_offset, idx_t r_offset) const { return get_next_bit_impl(l_offset, r_offset); } +inline ShenandoahMarkBitMap::idx_t ShenandoahMarkBitMap::get_last_one_offset(idx_t l_offset, idx_t r_offset) const { + return get_last_bit_impl(l_offset, r_offset); +} + // Returns a bit mask for a range of bits [beg, end) within a single word. Each // bit in the mask is 0 if the bit is in the range, 1 if not in the range. The // returned mask can be used directly to clear the range, or inverted to set the diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.hpp b/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.hpp index 8a52042e513ef..1710f34bbd77f 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.hpp @@ -67,7 +67,10 @@ class ShenandoahMarkingContext : public CHeapObj { inline bool is_marked_or_old(oop obj) const; inline bool is_marked_strong_or_old(oop obj) const; + // get the first marked address in the range [addr,linit), or limit if none found inline HeapWord* get_next_marked_addr(const HeapWord* addr, const HeapWord* limit) const; + // get the last marked address in the range (limit, addr), or limit if none found + inline HeapWord* get_last_marked_addr(const HeapWord* limit, const HeapWord* addr) const; inline bool allocated_after_mark_start(const oop obj) const; inline bool allocated_after_mark_start(const HeapWord* addr) const; diff --git a/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.inline.hpp index e3ba774283c18..db7ee03046fb5 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.inline.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.inline.hpp @@ -72,6 +72,10 @@ inline HeapWord* ShenandoahMarkingContext::get_next_marked_addr(const HeapWord* return _mark_bit_map.get_next_marked_addr(start, limit); } +inline HeapWord* ShenandoahMarkingContext::get_last_marked_addr(const HeapWord* limit, const HeapWord* start) const { + return _mark_bit_map.get_last_marked_addr(limit, start); +} + inline bool ShenandoahMarkingContext::allocated_after_mark_start(oop obj) const { const HeapWord* addr = cast_from_oop(obj); return allocated_after_mark_start(addr); diff --git a/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp b/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp index 23c705348c409..b378584982805 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.cpp @@ -125,9 +125,8 @@ void ShenandoahDirectCardMarkRememberedSet::mark_read_table_as_clean() { // No lock required because arguments align with card boundaries. void ShenandoahCardCluster::reset_object_range(HeapWord* from, HeapWord* to) { - assert(((((unsigned long long) from) & (CardTable::card_size() - 1)) == 0) && - ((((unsigned long long) to) & (CardTable::card_size() - 1)) == 0), - "reset_object_range bounds must align with card boundaries"); + assert(CardTable::is_card_aligned(from) && CardTable::is_card_aligned(to), + "Must align with card boundaries"); size_t card_at_start = _rs->card_index_for_addr(from); size_t num_cards = (to - from) / CardTable::card_size_in_words(); @@ -222,22 +221,67 @@ size_t ShenandoahCardCluster::get_last_start(size_t card_index) const { return _object_starts[card_index].offsets.last; } -// Given a card_index, return the starting address of the first block in the heap -// that straddles into this card. If this card is co-initial with an object, then -// this would return the first address of the range that this card covers, which is -// where the card's first object also begins. -HeapWord* ShenandoahCardCluster::block_start(const size_t card_index) const { +// Given a card_index, return the starting address of the first object in the heap +// that intersects with this card. This must be a valid, parsable object, and must +// be the first such object that intersects with this card. The object may start before, +// at, or after the start of the card, and may end in or after the card. If no +// such object exists, a null value is returned. +// Expects to be called for a card in an region affiliated with the old generation in +// generational heap, otherwise behavior is undefined. +// If not null, ctx holds the complete marking context of the old generation. If null, +// we expect that the marking context isn't available and the crossing maps are valid. +// Note that crossing maps may be invalid following class unloading and before dead +// or unloaded objects have been coalesced and filled (updating the crossing maps). +HeapWord* ShenandoahCardCluster::first_object_start(const size_t card_index, const ShenandoahMarkingContext* const ctx) const { HeapWord* left = _rs->addr_for_card_index(card_index); + ShenandoahHeapRegion* region = ShenandoahHeap::heap()->heap_region_containing(left); #ifdef ASSERT assert(ShenandoahHeap::heap()->mode()->is_generational(), "Do not use in non-generational mode"); - ShenandoahHeapRegion* region = ShenandoahHeap::heap()->heap_region_containing(left); assert(region->is_old(), "Do not use for young regions"); // For HumongousRegion:s it's more efficient to jump directly to the // start region. assert(!region->is_humongous(), "Use region->humongous_start_region() instead"); #endif + + // if marking context is valid, we use the marking bit map to find the + // first marked object that intersects with this card, and if no such + // object exists, we return null + if (ctx != nullptr) { + if (ctx->is_marked(left)) { + oop obj = cast_to_oop(left); + assert(oopDesc::is_oop(obj), "Should be an object"); + return left; + } + // get the previous marked object, if any + if (region->bottom() < left) { + HeapWord* prev = ctx->get_last_marked_addr(region->bottom(), left); + if (prev != nullptr) { + oop obj = cast_to_oop(prev); + assert(oopDesc::is_oop(obj), "Should be an object"); + HeapWord* obj_end = prev + obj->size(); + if (obj_end > left) { + return prev; + } + } + } + // the prev marked object, if any, ends before left; + // find the next marked object if any on this card + assert(!ctx->is_marked(left), "Was dealt with above"); + HeapWord* right = MIN2(region->top(), ctx->top_at_mark_start(region)); + HeapWord* next = ctx->get_next_marked_addr(left, right); + if (next < right) { + oop obj = cast_to_oop(next); + assert(oopDesc::is_oop(obj), "Should be an object"); + } + return next; + } + assert(ctx == nullptr, "Should have returned above"); + + // The following code assumes that the region has only parsable objects + assert(ShenandoahGenerationalHeap::heap()->old_generation()->is_parsable(), + "The code that follows expects a parsable heap"); if (starts_object(card_index) && get_first_start(card_index) == 0) { // This card contains a co-initial object; a fortiori, it covers // also the case of a card being the first in a region. @@ -245,8 +289,6 @@ HeapWord* ShenandoahCardCluster::block_start(const size_t card_index) const { return left; } - HeapWord* p = nullptr; - oop obj = cast_to_oop(p); ssize_t cur_index = (ssize_t)card_index; assert(cur_index >= 0, "Overflow"); assert(cur_index > 0, "Should have returned above"); @@ -262,31 +304,45 @@ HeapWord* ShenandoahCardCluster::block_start(const size_t card_index) const { assert(starts_object(cur_index), "Error"); size_t offset = get_last_start(cur_index); // can avoid call via card size arithmetic below instead - p = _rs->addr_for_card_index(cur_index) + offset; + HeapWord* p = _rs->addr_for_card_index(cur_index) + offset; // Recall that we already dealt with the co-initial object case above assert(p < left, "obj should start before left"); // While it is safe to ask an object its size in the loop that // follows, the (ifdef'd out) loop should never be needed. - // 1. we ask this question only for regions in the old generation + // 1. we ask this question only for regions in the old generation, and those + // that are not humongous regions // 2. there is no direct allocation ever by mutators in old generation - // regions. Only GC will ever allocate in old regions, and then - // too only during promotion/evacuation phases. Thus there is no danger + // regions walked by this code. Only GC will ever allocate in old regions, + // and then too only during promotion/evacuation phases. Thus there is no danger // of races between reading from and writing to the object start array, // or of asking partially initialized objects their size (in the loop below). + // Furthermore, humongous regions (and their dirty cards) are never processed + // by this code. // 3. only GC asks this question during phases when it is not concurrently // evacuating/promoting, viz. during concurrent root scanning (before // the evacuation phase) and during concurrent update refs (after the // evacuation phase) of young collections. This is never called - // during old or global collections. + // during global collections during marking or update refs.. // 4. Every allocation under TAMS updates the object start array. - NOT_PRODUCT(obj = cast_to_oop(p);) + oop obj = cast_to_oop(p); assert(oopDesc::is_oop(obj), "Should be an object"); -#define WALK_FORWARD_IN_BLOCK_START false + assert(Klass::is_valid(obj->klass()), "Not a valid klass ptr"); +#ifdef ASSERT +#define WALK_FORWARD_IN_BLOCK_START true +#else +#define WALK_FORWARD_IN_BLOCK_START true +#endif // ASSERT while (WALK_FORWARD_IN_BLOCK_START && p + obj->size() < left) { p += obj->size(); + obj = cast_to_oop(p); + assert(oopDesc::is_oop(obj), "Should be an object"); + assert(Klass::is_valid(obj->klass()), "Not a valid klass ptr"); + // Check assumptions in previous block comment if this assert fires + guarantee(false, "Should never need forward walk in block start"); } -#undef WALK_FORWARD_IN_BLOCK_START // false - assert(p + obj->size() > left, "obj should end after left"); +#undef WALK_FORWARD_IN_BLOCK_START + guarantee(p <= left, "p should start at or before left end of card"); + guarantee(p + obj->size() > left, "obj should end after left end of card"); return p; } diff --git a/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp b/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp index 5df34159c0ff9..19940f05bcdca 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.hpp @@ -650,12 +650,18 @@ class ShenandoahCardCluster: public CHeapObj { size_t get_last_start(size_t card_index) const; - // Given a card_index, return the starting address of the first block in the heap - // that straddles into the card. If the card is co-initial with an object, then - // this would return the starting address of the heap that this card covers. - // Expects to be called for a card affiliated with the old generation in - // generational mode. - HeapWord* block_start(size_t card_index) const; + // Given a card_index, return the starting address of the first object in the heap + // that intersects with this card. This must be a valid, parsable object, and must + // be the first such object that intersects with this card. The object may start before, + // at, or after the start of the card, and may end in or after the card. If no + // such object exists, a null value is returned. + // Expects to be called for a card in an region affiliated with the old generation in + // generational heap, otherwise behavior is undefined. + // If not null, ctx holds the complete marking context of the old generation. If null, + // we expect that the marking context isn't available and the crossing maps are valid. + // Note that crossing maps may be invalid following class unloading and before dead + // or unloaded objects have been coalesced and filled (updating the crossing maps). + HeapWord* first_object_start(size_t card_index, const ShenandoahMarkingContext* const ctx) const; }; // ShenandoahScanRemembered is a concrete class representing the diff --git a/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.inline.hpp b/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.inline.hpp index 68bec5c2071bc..651349fd01169 100644 --- a/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.inline.hpp +++ b/src/hotspot/share/gc/shenandoah/shenandoahScanRemembered.inline.hpp @@ -167,14 +167,17 @@ void ShenandoahScanRemembered::process_clusters(size_t first_cluster, size_t cou assert(right <= region->top() && end_addr <= region->top(), "Busted bounds"); const MemRegion mr(left, right); - // NOTE: We'll not call block_start() repeatedly - // on a very large object if its head card is dirty. If not, - // (i.e. the head card is clean) we'll call it each time we - // process a new dirty range on the object. This is always - // the case for large object arrays, which are typically more + // NOTE: We'll not call first_object_start() repeatedly + // on a very large object, i.e. one spanning multiple cards, + // if its head card is dirty. If not, (i.e. its head card is clean) + // we'll call it each time we process a new dirty range on the object. + // This is always the case for large object arrays, which are typically more // common. - HeapWord* p = _scc->block_start(dirty_l); + assert(ctx != nullptr || heap->old_generation()->is_parsable(), "Error"); + HeapWord* p = _scc->first_object_start(dirty_l, ctx); oop obj = cast_to_oop(p); + assert(oopDesc::is_oop(obj), "Not an object"); + assert(ctx==nullptr || ctx->is_marked(obj), "Error"); // PREFIX: The object that straddles into this range of dirty cards // from the left may be subject to special treatment unless diff --git a/src/hotspot/share/oops/klass.cpp b/src/hotspot/share/oops/klass.cpp index 41ab8e325ab7c..169a07a1cacb4 100644 --- a/src/hotspot/share/oops/klass.cpp +++ b/src/hotspot/share/oops/klass.cpp @@ -1110,7 +1110,14 @@ bool Klass::is_valid(Klass* k) { if (!Metaspace::contains(k)) return false; if (!Symbol::is_valid(k->name())) return false; - return ClassLoaderDataGraph::is_valid(k->class_loader_data()); + + // YSR_DEBUGGING + if (SafepointSynchronize::is_at_safepoint()) { + return ClassLoaderDataGraph::is_valid(k->class_loader_data()); + } else { + MutexLocker x(ClassLoaderDataGraph_lock, Mutex::_no_safepoint_check_flag); + return ClassLoaderDataGraph::is_valid(k->class_loader_data()); + } } Method* Klass::method_at_vtable(int index) {