hotspot/src/share/vm/gc_implementation/g1/g1RemSet.cpp
changeset 17327 4bd0581aa231
parent 17108 cf72dcf9a8f2
child 17854 d65bc1546091
--- a/hotspot/src/share/vm/gc_implementation/g1/g1RemSet.cpp	Thu May 09 12:23:43 2013 +0200
+++ b/hotspot/src/share/vm/gc_implementation/g1/g1RemSet.cpp	Thu May 09 11:16:39 2013 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2001, 2012, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2013, 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
@@ -29,6 +29,7 @@
 #include "gc_implementation/g1/g1BlockOffsetTable.inline.hpp"
 #include "gc_implementation/g1/g1CollectedHeap.inline.hpp"
 #include "gc_implementation/g1/g1CollectorPolicy.hpp"
+#include "gc_implementation/g1/g1HotCardCache.hpp"
 #include "gc_implementation/g1/g1GCPhaseTimes.hpp"
 #include "gc_implementation/g1/g1OopClosures.inline.hpp"
 #include "gc_implementation/g1/g1RemSet.inline.hpp"
@@ -247,7 +248,7 @@
     assert(SafepointSynchronize::is_at_safepoint(), "not during an evacuation pause");
     assert(worker_i < (int) (ParallelGCThreads == 0 ? 1 : ParallelGCThreads), "should be a GC worker");
 
-    if (_g1rs->concurrentRefineOneCard(card_ptr, worker_i, true)) {
+    if (_g1rs->refine_card(card_ptr, worker_i, true)) {
       // 'card_ptr' contains references that point into the collection
       // set. We need to record the card in the DCQS
       // (G1CollectedHeap::into_cset_dirty_card_queue_set())
@@ -288,9 +289,6 @@
 #if CARD_REPEAT_HISTO
   ct_freq_update_histo_and_reset();
 #endif
-  if (worker_i == 0) {
-    _cg1r->clear_and_record_card_counts();
-  }
 
   // We cache the value of 'oc' closure into the appropriate slot in the
   // _cset_rs_update_cl for this worker
@@ -396,7 +394,7 @@
     //   RSet updating,
     // * the post-write barrier shouldn't be logging updates to young
     //   regions (but there is a situation where this can happen - see
-    //   the comment in G1RemSet::concurrentRefineOneCard below -
+    //   the comment in G1RemSet::refine_card() below -
     //   that should not be applicable here), and
     // * during actual RSet updating, the filtering of cards in young
     //   regions in HeapRegion::oops_on_card_seq_iterate_careful is
@@ -502,8 +500,6 @@
                                        claim_val);
 }
 
-
-
 G1TriggerClosure::G1TriggerClosure() :
   _triggered(false) { }
 
@@ -524,13 +520,91 @@
   _record_refs_into_cset(record_refs_into_cset),
   _push_ref_cl(push_ref_cl), _worker_i(worker_i) { }
 
-bool G1RemSet::concurrentRefineOneCard_impl(jbyte* card_ptr, int worker_i,
-                                                   bool check_for_refs_into_cset) {
+// Returns true if the given card contains references that point
+// into the collection set, if we're checking for such references;
+// false otherwise.
+
+bool G1RemSet::refine_card(jbyte* card_ptr, int worker_i,
+                           bool check_for_refs_into_cset) {
+
+  // If the card is no longer dirty, nothing to do.
+  if (*card_ptr != CardTableModRefBS::dirty_card_val()) {
+    // No need to return that this card contains refs that point
+    // into the collection set.
+    return false;
+  }
+
   // Construct the region representing the card.
   HeapWord* start = _ct_bs->addr_for(card_ptr);
   // And find the region containing it.
   HeapRegion* r = _g1->heap_region_containing(start);
-  assert(r != NULL, "unexpected null");
+  if (r == NULL) {
+    // Again no need to return that this card contains refs that
+    // point into the collection set.
+    return false;  // Not in the G1 heap (might be in perm, for example.)
+  }
+
+  // Why do we have to check here whether a card is on a young region,
+  // given that we dirty young regions and, as a result, the
+  // post-barrier is supposed to filter them out and never to enqueue
+  // them? When we allocate a new region as the "allocation region" we
+  // actually dirty its cards after we release the lock, since card
+  // dirtying while holding the lock was a performance bottleneck. So,
+  // as a result, it is possible for other threads to actually
+  // allocate objects in the region (after the acquire the lock)
+  // before all the cards on the region are dirtied. This is unlikely,
+  // and it doesn't happen often, but it can happen. So, the extra
+  // check below filters out those cards.
+  if (r->is_young()) {
+    return false;
+  }
+
+  // While we are processing RSet buffers during the collection, we
+  // actually don't want to scan any cards on the collection set,
+  // since we don't want to update remebered sets with entries that
+  // point into the collection set, given that live objects from the
+  // collection set are about to move and such entries will be stale
+  // very soon. This change also deals with a reliability issue which
+  // involves scanning a card in the collection set and coming across
+  // an array that was being chunked and looking malformed. Note,
+  // however, that if evacuation fails, we have to scan any objects
+  // that were not moved and create any missing entries.
+  if (r->in_collection_set()) {
+    return false;
+  }
+
+  // The result from the hot card cache insert call is either:
+  //   * pointer to the current card
+  //     (implying that the current card is not 'hot'),
+  //   * null
+  //     (meaning we had inserted the card ptr into the "hot" card cache,
+  //     which had some headroom),
+  //   * a pointer to a "hot" card that was evicted from the "hot" cache.
+  //
+
+  G1HotCardCache* hot_card_cache = _cg1r->hot_card_cache();
+  if (hot_card_cache->use_cache()) {
+    assert(!check_for_refs_into_cset, "sanity");
+    assert(!SafepointSynchronize::is_at_safepoint(), "sanity");
+
+    card_ptr = hot_card_cache->insert(card_ptr);
+    if (card_ptr == NULL) {
+      // There was no eviction. Nothing to do.
+      return false;
+    }
+
+    start = _ct_bs->addr_for(card_ptr);
+    r = _g1->heap_region_containing(start);
+    if (r == NULL) {
+      // Not in the G1 heap
+      return false;
+    }
+
+    // Checking whether the region we got back from the cache
+    // is young here is inappropriate. The region could have been
+    // freed, reallocated and tagged as young while in the cache.
+    // Hence we could see its young type change at any time.
+  }
 
   // Don't use addr_for(card_ptr + 1) which can ask for
   // a card beyond the heap.  This is not safe without a perm
@@ -610,140 +684,17 @@
     _conc_refine_cards++;
   }
 
-  return trigger_cl.triggered();
-}
-
-bool G1RemSet::concurrentRefineOneCard(jbyte* card_ptr, int worker_i,
-                                              bool check_for_refs_into_cset) {
-  // If the card is no longer dirty, nothing to do.
-  if (*card_ptr != CardTableModRefBS::dirty_card_val()) {
-    // No need to return that this card contains refs that point
-    // into the collection set.
-    return false;
-  }
-
-  // Construct the region representing the card.
-  HeapWord* start = _ct_bs->addr_for(card_ptr);
-  // And find the region containing it.
-  HeapRegion* r = _g1->heap_region_containing(start);
-  if (r == NULL) {
-    // Again no need to return that this card contains refs that
-    // point into the collection set.
-    return false;  // Not in the G1 heap (might be in perm, for example.)
-  }
-  // Why do we have to check here whether a card is on a young region,
-  // given that we dirty young regions and, as a result, the
-  // post-barrier is supposed to filter them out and never to enqueue
-  // them? When we allocate a new region as the "allocation region" we
-  // actually dirty its cards after we release the lock, since card
-  // dirtying while holding the lock was a performance bottleneck. So,
-  // as a result, it is possible for other threads to actually
-  // allocate objects in the region (after the acquire the lock)
-  // before all the cards on the region are dirtied. This is unlikely,
-  // and it doesn't happen often, but it can happen. So, the extra
-  // check below filters out those cards.
-  if (r->is_young()) {
-    return false;
-  }
-  // While we are processing RSet buffers during the collection, we
-  // actually don't want to scan any cards on the collection set,
-  // since we don't want to update remebered sets with entries that
-  // point into the collection set, given that live objects from the
-  // collection set are about to move and such entries will be stale
-  // very soon. This change also deals with a reliability issue which
-  // involves scanning a card in the collection set and coming across
-  // an array that was being chunked and looking malformed. Note,
-  // however, that if evacuation fails, we have to scan any objects
-  // that were not moved and create any missing entries.
-  if (r->in_collection_set()) {
-    return false;
-  }
+  // This gets set to true if the card being refined has
+  // references that point into the collection set.
+  bool has_refs_into_cset = trigger_cl.triggered();
 
-  // Should we defer processing the card?
-  //
-  // Previously the result from the insert_cache call would be
-  // either card_ptr (implying that card_ptr was currently "cold"),
-  // null (meaning we had inserted the card ptr into the "hot"
-  // cache, which had some headroom), or a "hot" card ptr
-  // extracted from the "hot" cache.
-  //
-  // Now that the _card_counts cache in the ConcurrentG1Refine
-  // instance is an evicting hash table, the result we get back
-  // could be from evicting the card ptr in an already occupied
-  // bucket (in which case we have replaced the card ptr in the
-  // bucket with card_ptr and "defer" is set to false). To avoid
-  // having a data structure (updates to which would need a lock)
-  // to hold these unprocessed dirty cards, we need to immediately
-  // process card_ptr. The actions needed to be taken on return
-  // from cache_insert are summarized in the following table:
-  //
-  // res      defer   action
-  // --------------------------------------------------------------
-  // null     false   card evicted from _card_counts & replaced with
-  //                  card_ptr; evicted ptr added to hot cache.
-  //                  No need to process res; immediately process card_ptr
-  //
-  // null     true    card not evicted from _card_counts; card_ptr added
-  //                  to hot cache.
-  //                  Nothing to do.
-  //
-  // non-null false   card evicted from _card_counts & replaced with
-  //                  card_ptr; evicted ptr is currently "cold" or
-  //                  caused an eviction from the hot cache.
-  //                  Immediately process res; process card_ptr.
-  //
-  // non-null true    card not evicted from _card_counts; card_ptr is
-  //                  currently cold, or caused an eviction from hot
-  //                  cache.
-  //                  Immediately process res; no need to process card_ptr.
-
-
-  jbyte* res = card_ptr;
-  bool defer = false;
+  // We should only be detecting that the card contains references
+  // that point into the collection set if the current thread is
+  // a GC worker thread.
+  assert(!has_refs_into_cset || SafepointSynchronize::is_at_safepoint(),
+           "invalid result at non safepoint");
 
-  // This gets set to true if the card being refined has references
-  // that point into the collection set.
-  bool oops_into_cset = false;
-
-  if (_cg1r->use_cache()) {
-    jbyte* res = _cg1r->cache_insert(card_ptr, &defer);
-    if (res != NULL && (res != card_ptr || defer)) {
-      start = _ct_bs->addr_for(res);
-      r = _g1->heap_region_containing(start);
-      if (r != NULL) {
-        // Checking whether the region we got back from the cache
-        // is young here is inappropriate. The region could have been
-        // freed, reallocated and tagged as young while in the cache.
-        // Hence we could see its young type change at any time.
-        //
-        // Process card pointer we get back from the hot card cache. This
-        // will check whether the region containing the card is young
-        // _after_ checking that the region has been allocated from.
-        oops_into_cset = concurrentRefineOneCard_impl(res, worker_i,
-                                                      false /* check_for_refs_into_cset */);
-        // The above call to concurrentRefineOneCard_impl is only
-        // performed if the hot card cache is enabled. This cache is
-        // disabled during an evacuation pause - which is the only
-        // time when we need know if the card contains references
-        // that point into the collection set. Also when the hot card
-        // cache is enabled, this code is executed by the concurrent
-        // refine threads - rather than the GC worker threads - and
-        // concurrentRefineOneCard_impl will return false.
-        assert(!oops_into_cset, "should not see true here");
-      }
-    }
-  }
-
-  if (!defer) {
-    oops_into_cset =
-      concurrentRefineOneCard_impl(card_ptr, worker_i, check_for_refs_into_cset);
-    // We should only be detecting that the card contains references
-    // that point into the collection set if the current thread is
-    // a GC worker thread.
-    assert(!oops_into_cset || SafepointSynchronize::is_at_safepoint(),
-           "invalid result at non safepoint");
-  }
-  return oops_into_cset;
+  return has_refs_into_cset;
 }
 
 class HRRSStatsIter: public HeapRegionClosure {
@@ -846,13 +797,16 @@
       DirtyCardQueueSet& dcqs = JavaThread::dirty_card_queue_set();
       dcqs.concatenate_logs();
     }
-    bool cg1r_use_cache = _cg1r->use_cache();
-    _cg1r->set_use_cache(false);
+
+    G1HotCardCache* hot_card_cache = _cg1r->hot_card_cache();
+    bool use_hot_card_cache = hot_card_cache->use_cache();
+    hot_card_cache->set_use_cache(false);
+
     DirtyCardQueue into_cset_dcq(&_g1->into_cset_dirty_card_queue_set());
     updateRS(&into_cset_dcq, 0);
     _g1->into_cset_dirty_card_queue_set().clear();
-    _cg1r->set_use_cache(cg1r_use_cache);
 
+    hot_card_cache->set_use_cache(use_hot_card_cache);
     assert(JavaThread::dirty_card_queue_set().completed_buffers_num() == 0, "All should be consumed");
   }
 }