8153170: Card Live Data does not correctly handle eager reclaim
Summary: The card live data of regions eagerly reclaimed during remark and cleanup pause could be wrong, not considering that these regions were eagerly reclaimed and empty.
Reviewed-by: drwhite, kbarrett
--- a/hotspot/src/share/vm/gc/g1/g1CardLiveData.cpp Mon Apr 18 16:51:14 2016 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1CardLiveData.cpp Mon Apr 18 16:54:04 2016 +0200
@@ -28,6 +28,7 @@
#include "gc/g1/g1CardLiveData.inline.hpp"
#include "gc/g1/suspendibleThreadSet.hpp"
#include "gc/shared/workgroup.hpp"
+#include "logging/log.hpp"
#include "memory/universe.hpp"
#include "runtime/atomic.inline.hpp"
#include "runtime/globals.hpp"
@@ -38,6 +39,7 @@
G1CardLiveData::G1CardLiveData() :
_max_capacity(0),
_cards_per_region(0),
+ _gc_timestamp_at_create(0),
_live_regions(NULL),
_live_regions_size_in_bits(0),
_live_cards(NULL),
@@ -127,6 +129,13 @@
// lots of work most of the time.
BitMap::idx_t _last_marked_bit_idx;
+ void clear_card_bitmap_range(HeapWord* start, HeapWord* end) {
+ BitMap::idx_t start_idx = card_live_bitmap_index_for(start);
+ BitMap::idx_t end_idx = card_live_bitmap_index_for((HeapWord*)align_ptr_up(end, CardTableModRefBS::card_size));
+
+ _card_bm.clear_range(start_idx, end_idx);
+ }
+
// Mark the card liveness bitmap for the object spanning from start to end.
void mark_card_bitmap_range(HeapWord* start, HeapWord* end) {
BitMap::idx_t start_idx = card_live_bitmap_index_for(start);
@@ -169,6 +178,10 @@
_region_bm.par_set_bit(hr->hrm_index());
}
+ void reset_live_data(HeapRegion* hr) {
+ clear_card_bitmap_range(hr->next_top_at_mark_start(), hr->end());
+ }
+
// Mark the range of bits covered by allocations done since the last marking
// in the given heap region, i.e. from NTAMS to top of the given region.
// Returns if there has been some allocation in this region since the last marking.
@@ -305,6 +318,8 @@
};
void G1CardLiveData::create(WorkGang* workers, G1CMBitMap* mark_bitmap) {
+ _gc_timestamp_at_create = G1CollectedHeap::heap()->get_gc_time_stamp();
+
uint n_workers = workers->active_workers();
G1CreateCardLiveDataTask cl(mark_bitmap,
@@ -322,14 +337,24 @@
class G1FinalizeCardLiveDataClosure: public HeapRegionClosure {
private:
G1CardLiveDataHelper _helper;
+
+ uint _gc_timestamp_at_create;
+
+ bool has_been_reclaimed(HeapRegion* hr) const {
+ return hr->get_gc_time_stamp() > _gc_timestamp_at_create;
+ }
public:
G1FinalizeCardLiveDataClosure(G1CollectedHeap* g1h,
G1CMBitMap* bitmap,
G1CardLiveData* live_data) :
HeapRegionClosure(),
- _helper(live_data, g1h->reserved_region().start()) { }
+ _helper(live_data, g1h->reserved_region().start()),
+ _gc_timestamp_at_create(live_data->gc_timestamp_at_create()) { }
bool doHeapRegion(HeapRegion* hr) {
+ if (has_been_reclaimed(hr)) {
+ _helper.reset_live_data(hr);
+ }
bool allocated_since_marking = _helper.mark_allocated_since_marking(hr);
if (allocated_since_marking || hr->next_marked_bytes() > 0) {
_helper.set_bit_for_region(hr);
@@ -459,27 +484,26 @@
// Verify the marked bytes for this region.
if (exp_marked_bytes != act_marked_bytes) {
+ log_error(gc)("Expected marked bytes " SIZE_FORMAT " != actual marked bytes " SIZE_FORMAT " in region %u", exp_marked_bytes, act_marked_bytes, hr->hrm_index());
failures += 1;
} else if (exp_marked_bytes > HeapRegion::GrainBytes) {
+ log_error(gc)("Expected marked bytes " SIZE_FORMAT " larger than possible " SIZE_FORMAT " in region %u", exp_marked_bytes, HeapRegion::GrainBytes, hr->hrm_index());
failures += 1;
}
// Verify the bit, for this region, in the actual and expected
// (which was just calculated) region bit maps.
- // We're not OK if the bit in the calculated expected region
- // bitmap is set and the bit in the actual region bitmap is not.
uint index = hr->hrm_index();
bool expected = _exp_live_data->is_region_live(index);
bool actual = _act_live_data->is_region_live(index);
- if (expected && !actual) {
+ if (expected != actual) {
+ log_error(gc)("Expected liveness %d not equal actual %d in region %u", expected, actual, hr->hrm_index());
failures += 1;
}
// Verify that the card bit maps for the cards spanned by the current
- // region match. We have an error if we have a set bit in the expected
- // bit map and the corresponding bit in the actual bitmap is not set.
-
+ // region match.
BitMap::idx_t start_idx = _helper.card_live_bitmap_index_for(hr->bottom());
BitMap::idx_t end_idx = _helper.card_live_bitmap_index_for(hr->top());
@@ -487,7 +511,8 @@
expected = _exp_live_data->is_card_live_at(i);
actual = _act_live_data->is_card_live_at(i);
- if (expected && !actual) {
+ if (expected != actual) {
+ log_error(gc)("Expected card liveness %d not equal actual card liveness %d at card " SIZE_FORMAT " in region %u", expected, actual, i, hr->hrm_index());
failures += 1;
}
}
--- a/hotspot/src/share/vm/gc/g1/g1CardLiveData.hpp Mon Apr 18 16:51:14 2016 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1CardLiveData.hpp Mon Apr 18 16:54:04 2016 +0200
@@ -46,6 +46,17 @@
size_t _max_capacity;
size_t _cards_per_region;
+ // Regions may be reclaimed while concurrently creating live data (e.g. due to humongous
+ // eager reclaim). This results in wrong live data for these regions at the end.
+ // So we need to somehow detect these regions, and during live data finalization completely
+ // recreate their information.
+ // This _gc_timestamp_at_create tracks the global timestamp when live data creation
+ // has started. Any regions with a higher time stamp have been cleared after that
+ // point in time, and need re-finalization.
+ // Unsynchronized access to this variable is okay, since this value is only set during a
+ // concurrent phase, and read only at the Cleanup safepoint. I.e. there is always
+ // full memory synchronization inbetween.
+ uint _gc_timestamp_at_create;
// The per-card liveness bitmap.
bm_word_t* _live_cards;
size_t _live_cards_size_in_bits;
@@ -69,6 +80,8 @@
size_t live_region_bitmap_size_in_bits() const;
size_t live_card_bitmap_size_in_bits() const;
public:
+ uint gc_timestamp_at_create() const { return _gc_timestamp_at_create; }
+
inline bool is_region_live(uint region) const;
inline void remove_nonlive_cards(uint region, BitMap* bm);
--- a/hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp Mon Apr 18 16:51:14 2016 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1CollectedHeap.hpp Mon Apr 18 16:54:04 2016 +0200
@@ -245,7 +245,7 @@
// If not, we can skip a few steps.
bool _has_humongous_reclaim_candidates;
- volatile unsigned _gc_time_stamp;
+ volatile uint _gc_time_stamp;
G1HRPrinter _hr_printer;
@@ -999,7 +999,7 @@
// Try to minimize the remembered set.
void scrub_rem_set();
- unsigned get_gc_time_stamp() {
+ uint get_gc_time_stamp() {
return _gc_time_stamp;
}
--- a/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp Mon Apr 18 16:51:14 2016 +0200
+++ b/hotspot/src/share/vm/gc/g1/g1ConcurrentMark.cpp Mon Apr 18 16:54:04 2016 +0200
@@ -1144,8 +1144,6 @@
if (hr->is_archive()) {
return false;
}
- // We use a claim value of zero here because all regions
- // were claimed with value 1 in the FinalCount task.
_g1->reset_gc_time_stamps(hr);
hr->note_end_of_marking();
--- a/hotspot/src/share/vm/gc/g1/heapRegion.cpp Mon Apr 18 16:51:14 2016 +0200
+++ b/hotspot/src/share/vm/gc/g1/heapRegion.cpp Mon Apr 18 16:54:04 2016 +0200
@@ -187,6 +187,7 @@
zero_marked_bytes();
init_top_at_mark_start();
+ _gc_time_stamp = G1CollectedHeap::heap()->get_gc_time_stamp();
if (clear_space) clear(SpaceDecorator::Mangle);
}
@@ -1044,7 +1045,7 @@
void G1ContiguousSpace::record_timestamp() {
G1CollectedHeap* g1h = G1CollectedHeap::heap();
- unsigned curr_gc_time_stamp = g1h->get_gc_time_stamp();
+ uint curr_gc_time_stamp = g1h->get_gc_time_stamp();
if (_gc_time_stamp < curr_gc_time_stamp) {
// Setting the time stamp here tells concurrent readers to look at
--- a/hotspot/src/share/vm/gc/g1/heapRegion.hpp Mon Apr 18 16:51:14 2016 +0200
+++ b/hotspot/src/share/vm/gc/g1/heapRegion.hpp Mon Apr 18 16:54:04 2016 +0200
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2001, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2016, 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
@@ -124,7 +124,7 @@
protected:
G1BlockOffsetTablePart _bot_part;
Mutex _par_alloc_lock;
- volatile unsigned _gc_time_stamp;
+ volatile uint _gc_time_stamp;
// When we need to retire an allocation region, while other threads
// are also concurrently trying to allocate into it, we typically
// allocate a dummy object at the end of the region to ensure that
@@ -174,7 +174,7 @@
HeapWord* scan_top() const;
void record_timestamp();
void reset_gc_time_stamp() { _gc_time_stamp = 0; }
- unsigned get_gc_time_stamp() { return _gc_time_stamp; }
+ uint get_gc_time_stamp() { return _gc_time_stamp; }
void record_retained_region();
// See the comment above in the declaration of _pre_dummy_top for an