8065358: Refactor G1s usage of save_marks and reduce related races

Wed, 25 Mar 2015 11:03:16 +0100

author
mgerdin
date
Wed, 25 Mar 2015 11:03:16 +0100
changeset 7647
80ac3ee51955
parent 7646
5743a702da65
child 7648
f97f21d8d58c

8065358: Refactor G1s usage of save_marks and reduce related races
Summary: Stop using save_marks in G1 related code and make setting the replacement field less racy.
Reviewed-by: brutisso, tschatzl

src/share/vm/gc_implementation/g1/g1Allocator.cpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/g1/g1RemSet.cpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/g1/heapRegion.cpp file | annotate | diff | comparison | revisions
src/share/vm/gc_implementation/g1/heapRegion.hpp file | annotate | diff | comparison | revisions
     1.1 --- a/src/share/vm/gc_implementation/g1/g1Allocator.cpp	Tue Mar 24 10:04:10 2015 +0000
     1.2 +++ b/src/share/vm/gc_implementation/g1/g1Allocator.cpp	Wed Mar 25 11:03:16 2015 +0100
     1.3 @@ -59,7 +59,7 @@
     1.4        !(retained_region->top() == retained_region->end()) &&
     1.5        !retained_region->is_empty() &&
     1.6        !retained_region->isHumongous()) {
     1.7 -    retained_region->record_top_and_timestamp();
     1.8 +    retained_region->record_timestamp();
     1.9      // The retained region was added to the old region set when it was
    1.10      // retired. We have to remove it now, since we don't allow regions
    1.11      // we allocate to in the region sets. We'll re-add it later, when
    1.12 @@ -94,6 +94,9 @@
    1.13    // want either way so no reason to check explicitly for either
    1.14    // condition.
    1.15    _retained_old_gc_alloc_region = old_gc_alloc_region(context)->release();
    1.16 +  if (_retained_old_gc_alloc_region != NULL) {
    1.17 +    _retained_old_gc_alloc_region->record_retained_region();
    1.18 +  }
    1.19  
    1.20    if (ResizePLAB) {
    1.21      _g1h->_survivor_plab_stats.adjust_desired_plab_sz(no_of_gc_workers);
     2.1 --- a/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Tue Mar 24 10:04:10 2015 +0000
     2.2 +++ b/src/share/vm/gc_implementation/g1/g1CollectedHeap.cpp	Wed Mar 25 11:03:16 2015 +0100
     2.3 @@ -6808,7 +6808,7 @@
     2.4        // We really only need to do this for old regions given that we
     2.5        // should never scan survivors. But it doesn't hurt to do it
     2.6        // for survivors too.
     2.7 -      new_alloc_region->record_top_and_timestamp();
     2.8 +      new_alloc_region->record_timestamp();
     2.9        if (survivor) {
    2.10          new_alloc_region->set_survivor();
    2.11          _hr_printer.alloc(new_alloc_region, G1HRPrinter::Survivor);
     3.1 --- a/src/share/vm/gc_implementation/g1/g1RemSet.cpp	Tue Mar 24 10:04:10 2015 +0000
     3.2 +++ b/src/share/vm/gc_implementation/g1/g1RemSet.cpp	Wed Mar 25 11:03:16 2015 +0100
     3.3 @@ -147,11 +147,9 @@
     3.4  
     3.5      // Set the "from" region in the closure.
     3.6      _oc->set_region(r);
     3.7 -    HeapWord* card_start = _bot_shared->address_for_index(index);
     3.8 -    HeapWord* card_end = card_start + G1BlockOffsetSharedArray::N_words;
     3.9 -    Space *sp = SharedHeap::heap()->space_containing(card_start);
    3.10 -    MemRegion sm_region = sp->used_region_at_save_marks();
    3.11 -    MemRegion mr = sm_region.intersection(MemRegion(card_start,card_end));
    3.12 +    MemRegion card_region(_bot_shared->address_for_index(index), G1BlockOffsetSharedArray::N_words);
    3.13 +    MemRegion pre_gc_allocated(r->bottom(), r->scan_top());
    3.14 +    MemRegion mr = pre_gc_allocated.intersection(card_region);
    3.15      if (!mr.is_empty() && !_ct_bs->is_card_claimed(index)) {
    3.16        // We make the card as "claimed" lazily (so races are possible
    3.17        // but they're benign), which reduces the number of duplicate
     4.1 --- a/src/share/vm/gc_implementation/g1/heapRegion.cpp	Tue Mar 24 10:04:10 2015 +0000
     4.2 +++ b/src/share/vm/gc_implementation/g1/heapRegion.cpp	Wed Mar 25 11:03:16 2015 +0100
     4.3 @@ -338,7 +338,7 @@
     4.4    _orig_end = mr.end();
     4.5    hr_clear(false /*par*/, false /*clear_space*/);
     4.6    set_top(bottom());
     4.7 -  record_top_and_timestamp();
     4.8 +  record_timestamp();
     4.9  }
    4.10  
    4.11  CompactibleSpace* HeapRegion::next_compaction_space() const {
    4.12 @@ -426,9 +426,9 @@
    4.13  
    4.14    // If we're within a stop-world GC, then we might look at a card in a
    4.15    // GC alloc region that extends onto a GC LAB, which may not be
    4.16 -  // parseable.  Stop such at the "saved_mark" of the region.
    4.17 +  // parseable.  Stop such at the "scan_top" of the region.
    4.18    if (g1h->is_gc_active()) {
    4.19 -    mr = mr.intersection(used_region_at_save_marks());
    4.20 +    mr = mr.intersection(MemRegion(bottom(), scan_top()));
    4.21    } else {
    4.22      mr = mr.intersection(used_region());
    4.23    }
    4.24 @@ -980,7 +980,7 @@
    4.25  
    4.26  void G1OffsetTableContigSpace::clear(bool mangle_space) {
    4.27    set_top(bottom());
    4.28 -  set_saved_mark_word(bottom());
    4.29 +  _scan_top = bottom();
    4.30    CompactibleSpace::clear(mangle_space);
    4.31    reset_bot();
    4.32  }
    4.33 @@ -1012,41 +1012,42 @@
    4.34    return _offsets.threshold();
    4.35  }
    4.36  
    4.37 -HeapWord* G1OffsetTableContigSpace::saved_mark_word() const {
    4.38 +HeapWord* G1OffsetTableContigSpace::scan_top() const {
    4.39    G1CollectedHeap* g1h = G1CollectedHeap::heap();
    4.40 -  assert( _gc_time_stamp <= g1h->get_gc_time_stamp(), "invariant" );
    4.41    HeapWord* local_top = top();
    4.42    OrderAccess::loadload();
    4.43 -  if (_gc_time_stamp < g1h->get_gc_time_stamp()) {
    4.44 +  const unsigned local_time_stamp = _gc_time_stamp;
    4.45 +  assert(local_time_stamp <= g1h->get_gc_time_stamp(), "invariant");
    4.46 +  if (local_time_stamp < g1h->get_gc_time_stamp()) {
    4.47      return local_top;
    4.48    } else {
    4.49 -    return Space::saved_mark_word();
    4.50 +    return _scan_top;
    4.51    }
    4.52  }
    4.53  
    4.54 -void G1OffsetTableContigSpace::record_top_and_timestamp() {
    4.55 +void G1OffsetTableContigSpace::record_timestamp() {
    4.56    G1CollectedHeap* g1h = G1CollectedHeap::heap();
    4.57    unsigned curr_gc_time_stamp = g1h->get_gc_time_stamp();
    4.58  
    4.59    if (_gc_time_stamp < curr_gc_time_stamp) {
    4.60 -    // The order of these is important, as another thread might be
    4.61 -    // about to start scanning this region. If it does so after
    4.62 -    // set_saved_mark and before _gc_time_stamp = ..., then the latter
    4.63 -    // will be false, and it will pick up top() as the high water mark
    4.64 -    // of region. If it does so after _gc_time_stamp = ..., then it
    4.65 -    // will pick up the right saved_mark_word() as the high water mark
    4.66 -    // of the region. Either way, the behaviour will be correct.
    4.67 -    Space::set_saved_mark_word(top());
    4.68 -    OrderAccess::storestore();
    4.69 +    // Setting the time stamp here tells concurrent readers to look at
    4.70 +    // scan_top to know the maximum allowed address to look at.
    4.71 +
    4.72 +    // scan_top should be bottom for all regions except for the
    4.73 +    // retained old alloc region which should have scan_top == top
    4.74 +    HeapWord* st = _scan_top;
    4.75 +    guarantee(st == _bottom || st == _top, "invariant");
    4.76 +
    4.77      _gc_time_stamp = curr_gc_time_stamp;
    4.78 -    // No need to do another barrier to flush the writes above. If
    4.79 -    // this is called in parallel with other threads trying to
    4.80 -    // allocate into the region, the caller should call this while
    4.81 -    // holding a lock and when the lock is released the writes will be
    4.82 -    // flushed.
    4.83    }
    4.84  }
    4.85  
    4.86 +void G1OffsetTableContigSpace::record_retained_region() {
    4.87 +  // scan_top is the maximum address where it's safe for the next gc to
    4.88 +  // scan this region.
    4.89 +  _scan_top = top();
    4.90 +}
    4.91 +
    4.92  void G1OffsetTableContigSpace::safe_object_iterate(ObjectClosure* blk) {
    4.93    object_iterate(blk);
    4.94  }
    4.95 @@ -1080,6 +1081,8 @@
    4.96  void G1OffsetTableContigSpace::initialize(MemRegion mr, bool clear_space, bool mangle_space) {
    4.97    CompactibleSpace::initialize(mr, clear_space, mangle_space);
    4.98    _top = bottom();
    4.99 +  _scan_top = bottom();
   4.100 +  set_saved_mark_word(NULL);
   4.101    reset_bot();
   4.102  }
   4.103  
     5.1 --- a/src/share/vm/gc_implementation/g1/heapRegion.hpp	Tue Mar 24 10:04:10 2015 +0000
     5.2 +++ b/src/share/vm/gc_implementation/g1/heapRegion.hpp	Wed Mar 25 11:03:16 2015 +0100
     5.3 @@ -101,28 +101,25 @@
     5.4  // OffsetTableContigSpace.  If the two versions of BlockOffsetTable could
     5.5  // be reconciled, then G1OffsetTableContigSpace could go away.
     5.6  
     5.7 -// The idea behind time stamps is the following. Doing a save_marks on
     5.8 -// all regions at every GC pause is time consuming (if I remember
     5.9 -// well, 10ms or so). So, we would like to do that only for regions
    5.10 -// that are GC alloc regions. To achieve this, we use time
    5.11 -// stamps. For every evacuation pause, G1CollectedHeap generates a
    5.12 -// unique time stamp (essentially a counter that gets
    5.13 -// incremented). Every time we want to call save_marks on a region,
    5.14 -// we set the saved_mark_word to top and also copy the current GC
    5.15 -// time stamp to the time stamp field of the space. Reading the
    5.16 -// saved_mark_word involves checking the time stamp of the
    5.17 -// region. If it is the same as the current GC time stamp, then we
    5.18 -// can safely read the saved_mark_word field, as it is valid. If the
    5.19 -// time stamp of the region is not the same as the current GC time
    5.20 -// stamp, then we instead read top, as the saved_mark_word field is
    5.21 -// invalid. Time stamps (on the regions and also on the
    5.22 -// G1CollectedHeap) are reset at every cleanup (we iterate over
    5.23 -// the regions anyway) and at the end of a Full GC. The current scheme
    5.24 -// that uses sequential unsigned ints will fail only if we have 4b
    5.25 +// The idea behind time stamps is the following. We want to keep track of
    5.26 +// the highest address where it's safe to scan objects for each region.
    5.27 +// This is only relevant for current GC alloc regions so we keep a time stamp
    5.28 +// per region to determine if the region has been allocated during the current
    5.29 +// GC or not. If the time stamp is current we report a scan_top value which
    5.30 +// was saved at the end of the previous GC for retained alloc regions and which is
    5.31 +// equal to the bottom for all other regions.
    5.32 +// There is a race between card scanners and allocating gc workers where we must ensure
    5.33 +// that card scanners do not read the memory allocated by the gc workers.
    5.34 +// In order to enforce that, we must not return a value of _top which is more recent than the
    5.35 +// time stamp. This is due to the fact that a region may become a gc alloc region at
    5.36 +// some point after we've read the timestamp value as being < the current time stamp.
    5.37 +// The time stamps are re-initialized to zero at cleanup and at Full GCs.
    5.38 +// The current scheme that uses sequential unsigned ints will fail only if we have 4b
    5.39  // evacuation pauses between two cleanups, which is _highly_ unlikely.
    5.40  class G1OffsetTableContigSpace: public CompactibleSpace {
    5.41    friend class VMStructs;
    5.42    HeapWord* _top;
    5.43 +  HeapWord* volatile _scan_top;
    5.44   protected:
    5.45    G1BlockOffsetArrayContigSpace _offsets;
    5.46    Mutex _par_alloc_lock;
    5.47 @@ -166,10 +163,11 @@
    5.48    void set_bottom(HeapWord* value);
    5.49    void set_end(HeapWord* value);
    5.50  
    5.51 -  virtual HeapWord* saved_mark_word() const;
    5.52 -  void record_top_and_timestamp();
    5.53 +  HeapWord* scan_top() const;
    5.54 +  void record_timestamp();
    5.55    void reset_gc_time_stamp() { _gc_time_stamp = 0; }
    5.56    unsigned get_gc_time_stamp() { return _gc_time_stamp; }
    5.57 +  void record_retained_region();
    5.58  
    5.59    // See the comment above in the declaration of _pre_dummy_top for an
    5.60    // explanation of what it is.
    5.61 @@ -193,6 +191,8 @@
    5.62    virtual HeapWord* allocate(size_t word_size);
    5.63    HeapWord* par_allocate(size_t word_size);
    5.64  
    5.65 +  HeapWord* saved_mark_word() const { ShouldNotReachHere(); return NULL; }
    5.66 +
    5.67    // MarkSweep support phase3
    5.68    virtual HeapWord* initialize_threshold();
    5.69    virtual HeapWord* cross_threshold(HeapWord* start, HeapWord* end);

mercurial