< prev index next >

src/share/vm/gc_implementation/g1/satbQueue.cpp

Print this page
rev 8129 : [mq]: fix2

@@ -1,7 +1,7 @@
 /*
- * Copyright (c) 2001, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2001, 2015, 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
  * under the terms of the GNU General Public License version 2 only, as
  * published by the Free Software Foundation.

@@ -31,34 +31,70 @@
 #include "runtime/mutexLocker.hpp"
 #include "runtime/thread.hpp"
 #include "runtime/vmThread.hpp"
 
 void ObjPtrQueue::flush() {
-  // The buffer might contain refs into the CSet. We have to filter it
-  // first before we flush it, otherwise we might end up with an
-  // enqueued buffer with refs into the CSet which breaks our invariants.
+  // Filter now to possibly save work later.  If filtering empties the
+  // buffer then flush_impl can deallocate the buffer.
   filter();
   flush_impl();
 }
 
-// This method removes entries from an SATB buffer that will not be
-// useful to the concurrent marking threads. An entry is removed if it
-// satisfies one of the following conditions:
-//
-// * it points to an object outside the G1 heap (G1's concurrent
-//     marking only visits objects inside the G1 heap),
-// * it points to an object that has been allocated since marking
-//     started (according to SATB those objects do not need to be
-//     visited during marking), or
-// * it points to an object that has already been marked (no need to
-//     process it again).
-//
-// The rest of the entries will be retained and are compacted towards
-// the top of the buffer. Note that, because we do not allow old
-// regions in the CSet during marking, all objects on the CSet regions
-// are young (eden or survivors) and therefore implicitly live. So any
-// references into the CSet will be removed during filtering.
+// Return true if a SATB buffer entry refers to an object that
+// requires marking.
+//
+// The entry must point into the G1 heap.  In particular, it must not
+// be a NULL pointer.  NULL pointers are pre-filtered and never
+// inserted into a SATB buffer.
+//
+// An entry that is below the NTAMS pointer for the containing heap
+// region requires marking. Such an entry must point to a valid object.
+//
+// An entry that is at least the NTAMS pointer for the containing heap
+// region might be any of the following, none of which should be marked.
+//
+// * A reference to an object allocated since marking started.
+//   According to SATB, such objects are implicitly kept live and do
+//   not need to be dealt with via SATB buffer processing.
+//
+// * A reference to a young generation object. Young objects are
+//   handled separately and are not marked by concurrent marking.
+//
+// * A stale reference to a young generation object. If a young
+//   generation object reference is recorded and not filtered out
+//   before being moved by a young collection, the reference becomes
+//   stale.
+//
+// * A stale reference to an eagerly reclaimed humongous object.  If a
+//   humongous object is recorded and then reclaimed, the reference
+//   becomes stale.
+//
+// The stale reference cases are implicitly handled by the NTAMS
+// comparison. Because of the possibility of stale references, buffer
+// processing must be somewhat circumspect and not assume entries
+// in an unfiltered buffer refer to valid objects.
+
+inline bool requires_marking(const HeapWord* entry, G1CollectedHeap* heap) {
+  // Includes rejection of NULL pointers.
+  assert(heap->is_in_reserved(entry),
+         err_msg("Non-heap pointer in SATB buffer: " PTR_FORMAT, p2i(entry)));
+
+  HeapRegion* region = heap->heap_region_containing_raw(entry);
+  assert(region != NULL, err_msg("No region for " PTR_FORMAT, p2i(entry)));
+  if (entry >= region->next_top_at_mark_start()) return false;
+
+  // Can obj really have it's mark word set?  It's not in young gen...
+  assert(((oop)entry)->is_oop(true /* ignore mark word */),
+         err_msg("Invalid oop in SATB buffer: " PTR_FORMAT, p2i(entry)));
+
+  return true;
+}
+
+// This method removes entries from a SATB buffer that will not be
+// useful to the concurrent marking threads.  Entries are retained if
+// they require marking and are not already marked. Retained entries
+// are compacted toward the top of the buffer.
 
 void ObjPtrQueue::filter() {
   G1CollectedHeap* g1h = G1CollectedHeap::heap();
   void** buf = _buf;
   size_t sz = _sz;

@@ -76,30 +112,29 @@
 
   while (i > _index) {
     assert(i > 0, "we should have at least one more entry to process");
     i -= oopSize;
     debug_only(entries += 1;)
-    oop* p = (oop*) &buf[byte_index_to_index((int) i)];
-    oop obj = *p;
+    HeapWord** p = (HeapWord**) &buf[byte_index_to_index((int) i)];
+    HeapWord* entry = *p;
     // NULL the entry so that unused parts of the buffer contain NULLs
     // at the end. If we are going to retain it we will copy it to its
     // final place. If we have retained all entries we have visited so
     // far, we'll just end up copying it to the same place.
     *p = NULL;
 
-    bool retain = g1h->is_obj_ill(obj);
-    if (retain) {
+    if (requires_marking(entry, g1h) && !g1h->isMarkedNext((oop)entry)) {
       assert(new_index > 0, "we should not have already filled up the buffer");
       new_index -= oopSize;
       assert(new_index >= i,
              "new_index should never be below i, as we always compact 'up'");
-      oop* new_p = (oop*) &buf[byte_index_to_index((int) new_index)];
+      HeapWord** new_p = (HeapWord**) &buf[byte_index_to_index((int) new_index)];
       assert(new_p >= p, "the destination location should never be below "
              "the source as we always compact 'up'");
       assert(*new_p == NULL,
              "we should have already cleared the destination location");
-      *new_p = obj;
+      *new_p = entry;
       debug_only(retained += 1;)
     }
   }
 
 #ifdef ASSERT

@@ -182,21 +217,10 @@
                          "index: "SIZE_FORMAT" sz: "SIZE_FORMAT,
                          name, p2i(buf), index, sz);
 }
 #endif // PRODUCT
 
-#ifdef ASSERT
-void ObjPtrQueue::verify_oops_in_buffer() {
-  if (_buf == NULL) return;
-  for (size_t i = _index; i < _sz; i += oopSize) {
-    oop obj = (oop)_buf[byte_index_to_index((int)i)];
-    assert(obj != NULL && obj->is_oop(true /* ignore mark word */),
-           "Not an oop");
-  }
-}
-#endif
-
 #ifdef _MSC_VER // the use of 'this' below gets a warning, make it go away
 #pragma warning( disable:4355 ) // 'this' : used in base member initializer list
 #endif // _MSC_VER
 
 SATBMarkQueueSet::SATBMarkQueueSet() :

@@ -210,11 +234,10 @@
   _shared_satb_queue.set_lock(lock);
   _closures = NEW_C_HEAP_ARRAY(ObjectClosure*, ParallelGCThreads, mtGC);
 }
 
 void SATBMarkQueueSet::handle_zero_index_for_thread(JavaThread* t) {
-  DEBUG_ONLY(t->satb_mark_queue().verify_oops_in_buffer();)
   t->satb_mark_queue().handle_zero_index();
 }
 
 #ifdef ASSERT
 void SATBMarkQueueSet::dump_active_states(bool expected_active) {
< prev index next >