# HG changeset patch # User kbarrett # Date 1479864287 18000 # Node ID 3d026d2bda87987e8e2811eca36fbbb0f3f09fe4 # Parent db3b97029d3ddd4c9c56e674f902864f22751e43 8258933: G1 needs klass_or_null_acquire Summary: Use acquire where needed, split refinment humongous and non to simplify. Reviewed-by: tschatzl, ehelin, aph diff -r db3b97029d3d -r 3d026d2bda87 src/share/vm/gc_implementation/g1/g1BlockOffsetTable.cpp --- a/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.cpp Mon Sep 19 13:12:26 2016 -0400 +++ b/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.cpp Tue Nov 22 20:24:47 2016 -0500 @@ -264,7 +264,7 @@ while (n <= next_boundary) { q = n; oop obj = oop(q); - if (obj->klass_or_null() == NULL) return q; + if (obj->klass_or_null_acquire() == NULL) return q; n += block_size(q); } assert(q <= next_boundary && n > next_boundary, "Consequence of loop"); diff -r db3b97029d3d -r 3d026d2bda87 src/share/vm/gc_implementation/g1/g1BlockOffsetTable.inline.hpp --- a/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.inline.hpp Mon Sep 19 13:12:26 2016 -0400 +++ b/src/share/vm/gc_implementation/g1/g1BlockOffsetTable.inline.hpp Tue Nov 22 20:24:47 2016 -0500 @@ -166,7 +166,7 @@ while (n <= addr) { q = n; oop obj = oop(q); - if (obj->klass_or_null() == NULL) return q; + if (obj->klass_or_null_acquire() == NULL) return q; n += block_size(q); } assert(q <= n, "wrong order for q and addr"); @@ -177,7 +177,7 @@ inline HeapWord* G1BlockOffsetArray::forward_to_block_containing_addr(HeapWord* q, const void* addr) { - if (oop(q)->klass_or_null() == NULL) return q; + if (oop(q)->klass_or_null_acquire() == NULL) return q; HeapWord* n = q + block_size(q); // In the normal case, where the query "addr" is a card boundary, and the // offset table chunks are the same size as cards, the block starting at diff -r db3b97029d3d -r 3d026d2bda87 src/share/vm/gc_implementation/g1/g1RemSet.cpp --- a/src/share/vm/gc_implementation/g1/g1RemSet.cpp Mon Sep 19 13:12:26 2016 -0400 +++ b/src/share/vm/gc_implementation/g1/g1RemSet.cpp Tue Nov 22 20:24:47 2016 -0500 @@ -575,9 +575,10 @@ card_ptr); // If unable to process the card then we encountered an unparsable - // part of the heap (e.g. a partially allocated object). Redirty - // and re-enqueue: if we put off the card until a GC pause, then the - // allocation will have completed. + // part of the heap (e.g. a partially allocated object) while + // processing a stale card. Despite the card being stale, redirty + // and re-enqueue, because we've already cleaned the card. Without + // this we could incorrectly discard a non-stale card. if (!card_processed) { assert(!_g1->is_gc_active(), "Unparsable heap during GC"); // The card might have gotten re-dirtied and re-enqueued while we diff -r db3b97029d3d -r 3d026d2bda87 src/share/vm/gc_implementation/g1/heapRegion.cpp --- a/src/share/vm/gc_implementation/g1/heapRegion.cpp Mon Sep 19 13:12:26 2016 -0400 +++ b/src/share/vm/gc_implementation/g1/heapRegion.cpp Tue Nov 22 20:24:47 2016 -0500 @@ -407,6 +407,49 @@ return NULL; } +// Humongous objects are allocated directly in the old-gen. Need +// special handling for concurrent processing encountering an +// in-progress allocation. +static bool do_oops_on_card_in_humongous(MemRegion mr, + FilterOutOfRegionClosure* cl, + HeapRegion* hr, + G1CollectedHeap* g1h) { + assert(hr->isHumongous(), "precondition"); + HeapRegion* sr = hr->humongous_start_region(); + oop obj = oop(sr->bottom()); + + // If concurrent and klass_or_null is NULL, then space has been + // allocated but the object has not yet been published by setting + // the klass. That can only happen if the card is stale. However, + // we've already set the card clean, so we must return failure, + // since the allocating thread could have performed a write to the + // card that might be missed otherwise. + if (!g1h->is_gc_active() && (obj->klass_or_null_acquire() == NULL)) { + return false; + } + + // Only filler objects follow a humongous object in the containing + // regions, and we can ignore those. So only process the one + // humongous object. + if (!g1h->is_obj_dead(obj, sr)) { + if (obj->is_objArray() || (sr->bottom() < mr.start())) { + // objArrays are always marked precisely, so limit processing + // with mr. Non-objArrays might be precisely marked, and since + // it's humongous it's worthwhile avoiding full processing. + // However, the card could be stale and only cover filler + // objects. That should be rare, so not worth checking for; + // instead let it fall out from the bounded iteration. + obj->oop_iterate(cl, mr); + } else { + // If obj is not an objArray and mr contains the start of the + // obj, then this could be an imprecise mark, and we need to + // process the entire object. + obj->oop_iterate(cl); + } + } + return true; +} + bool HeapRegion::oops_on_card_seq_iterate_careful(MemRegion mr, FilterOutOfRegionClosure* cl, jbyte* card_ptr) { @@ -424,7 +467,6 @@ if (mr.is_empty()) { return true; } - // Otherwise, find the obj that extends onto mr.start(). // The intersection of the incoming mr (for the card) and the // allocated part of the region is non-empty. This implies that @@ -442,54 +484,52 @@ // We must complete this write before we do any of the reads below. OrderAccess::storeload(); + // Special handling for humongous regions. + if (isHumongous()) { + return do_oops_on_card_in_humongous(mr, cl, this, g1h); + } + + // During GC we limit mr by scan_top. So we never get here with an + // mr covering objects allocated during GC. Non-humongous objects + // are only allocated in the old-gen during GC. So the parts of the + // heap that may be examined here are always parsable; there's no + // need to use klass_or_null here to detect in-progress allocations. + // Cache the boundaries of the memory region in some const locals HeapWord* const start = mr.start(); HeapWord* const end = mr.end(); - // Update BOT as needed while finding start of (potential) object. + // Find the obj that extends onto mr.start(). + // Update BOT as needed while finding start of (possibly dead) + // object containing the start of the region. HeapWord* cur = block_start(start); - assert(cur <= start, "Postcondition"); - - oop obj; - HeapWord* next = cur; - do { - cur = next; - obj = oop(cur); - if (obj->klass_or_null() == NULL) { - // Ran into an unparseable point. - assert(!g1h->is_gc_active(), - err_msg("Unparsable heap during GC at " PTR_FORMAT, p2i(cur))); - return false; - } - // Otherwise... - next = cur + block_size(cur); - } while (next <= start); - - // If we finish the above loop...We have a parseable object that - // begins on or before the start of the memory region, and ends - // inside or spans the entire region. - assert(cur <= start, "Loop postcondition"); - assert(obj->klass_or_null() != NULL, "Loop postcondition"); +#ifdef ASSERT + { + assert(cur <= start, + err_msg("cur: " PTR_FORMAT ", start: " PTR_FORMAT, p2i(cur), p2i(start))); + HeapWord* next = cur + block_size(cur); + assert(start < next, + err_msg("start: " PTR_FORMAT ", next: " PTR_FORMAT, p2i(start), p2i(next))); + } +#endif do { - obj = oop(cur); - assert((cur + block_size(cur)) > (HeapWord*)obj, "Loop invariant"); - if (obj->klass_or_null() == NULL) { - // Ran into an unparseable point. - assert(!g1h->is_gc_active(), - err_msg("Unparsable heap during GC at " PTR_FORMAT, p2i(cur))); - return false; - } + oop obj = oop(cur); + assert(obj->is_oop(true), err_msg("Not an oop at " PTR_FORMAT, p2i(cur))); + assert(obj->klass_or_null() != NULL, + err_msg("Unparsable heap at " PTR_FORMAT, p2i(cur))); - // Advance the current pointer. "obj" still points to the object to iterate. - cur = cur + block_size(cur); - - if (!g1h->is_obj_dead(obj)) { - // Non-objArrays are sometimes marked imprecise at the object start. We - // always need to iterate over them in full. - // We only iterate over object arrays in full if they are completely contained - // in the memory region. + if (g1h->is_obj_dead(obj, this)) { + // Carefully step over dead object. + cur += block_size(cur); + } else { + // Step over live object, and process its references. + cur += obj->size(); + // Non-objArrays are usually marked imprecise at the object + // start, in which case we need to iterate over them in full. + // objArrays are precisely marked, but can still be iterated + // over in full if completely covered. if (!obj->is_objArray() || (((HeapWord*)obj) >= start && cur <= end)) { obj->oop_iterate(cl); } else { diff -r db3b97029d3d -r 3d026d2bda87 src/share/vm/gc_implementation/g1/heapRegion.hpp --- a/src/share/vm/gc_implementation/g1/heapRegion.hpp Mon Sep 19 13:12:26 2016 -0400 +++ b/src/share/vm/gc_implementation/g1/heapRegion.hpp Tue Nov 22 20:24:47 2016 -0500 @@ -723,7 +723,7 @@ // mr: the memory region covered by the card. // card_ptr: if we decide that the card is not young and we iterate // over it, we'll clean the card before we start the iteration. - // Returns true if card was successfully processed, false if an + // Returns true if the card was successfully processed, false if an // unparsable part of the heap was encountered, which should only // happen when invoked concurrently with the mutator. bool oops_on_card_seq_iterate_careful(MemRegion mr,