changeset 10934:40a2defe3b0c

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
author tschatzl
date Mon, 18 Apr 2016 16:54:04 +0200
parents b4982d6f20e6
children 33d34e159776
files src/share/vm/gc/g1/g1CardLiveData.cpp src/share/vm/gc/g1/g1CardLiveData.hpp src/share/vm/gc/g1/g1CollectedHeap.hpp src/share/vm/gc/g1/g1ConcurrentMark.cpp src/share/vm/gc/g1/heapRegion.cpp src/share/vm/gc/g1/heapRegion.hpp
diffstat 6 files changed, 53 insertions(+), 16 deletions(-) [+]
line wrap: on
line diff
--- a/src/share/vm/gc/g1/g1CardLiveData.cpp	Mon Apr 18 16:51:14 2016 +0200
+++ b/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/src/share/vm/gc/g1/g1CardLiveData.hpp	Mon Apr 18 16:51:14 2016 +0200
+++ b/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/src/share/vm/gc/g1/g1CollectedHeap.hpp	Mon Apr 18 16:51:14 2016 +0200
+++ b/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/src/share/vm/gc/g1/g1ConcurrentMark.cpp	Mon Apr 18 16:51:14 2016 +0200
+++ b/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/src/share/vm/gc/g1/heapRegion.cpp	Mon Apr 18 16:51:14 2016 +0200
+++ b/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/src/share/vm/gc/g1/heapRegion.hpp	Mon Apr 18 16:51:14 2016 +0200
+++ b/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