changeset 53585:b55c5c0ee24f

8213623: ZGC: Let heap iteration walk all roots Reviewed-by: eosterlund, kbarrett
author pliden
date Tue, 20 Nov 2018 08:17:24 +0100
parents 1a395165c09b
children 4ed308e0e89b
files src/hotspot/share/gc/z/zHeapIterator.cpp src/hotspot/share/gc/z/zRootsIterator.hpp
diffstat 2 files changed, 34 insertions(+), 18 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/gc/z/zHeapIterator.cpp	Tue Nov 20 10:50:54 2018 +0530
+++ b/src/hotspot/share/gc/z/zHeapIterator.cpp	Tue Nov 20 08:17:24 2018 +0100
@@ -59,11 +59,9 @@
       _iter(iter) {}
 
   virtual void do_oop(oop* p) {
-    // Load barrier needed here for the same reason we
-    // need fixup_partial_loads() in ZHeap::mark_end().
-    // This barrier is also needed here in case we're
-    // treating the JVMTI weak tag map as strong roots.
-    const oop obj = ZBarrier::load_barrier_on_oop_field(p);
+    // Load barrier needed here, even on non-concurrent strong roots,
+    // for the same reason we need fixup_partial_loads() in ZHeap::mark_end().
+    const oop obj = NativeAccess<AS_NO_KEEPALIVE>::oop_load(p);
     _iter->push(obj);
   }
 
@@ -80,9 +78,9 @@
 
   oop load_oop(oop* p) const {
     if (_visit_referents) {
-      return HeapAccess<ON_UNKNOWN_OOP_REF>::oop_load_at(_base, _base->field_offset(p));
+      return HeapAccess<ON_UNKNOWN_OOP_REF | AS_NO_KEEPALIVE>::oop_load_at(_base, _base->field_offset(p));
     } else {
-      return HeapAccess<>::oop_load(p);
+      return HeapAccess<AS_NO_KEEPALIVE>::oop_load(p);
     }
   }
 
@@ -164,18 +162,36 @@
 }
 
 void ZHeapIterator::objects_do(ObjectClosure* cl) {
-  // Push roots onto stack
+  // Note that the heap iterator visits all reachable objects, including
+  // objects that might be unreachable from the application, such as a
+  // not yet cleared JNIWeakGloablRef. However, also note that visiting
+  // the JVMTI tag map is a requirement to make sure we visit all tagged
+  // objects, even those that might now have become phantom reachable.
+  // If we didn't do this the application would have expected to see
+  // ObjectFree events for phantom reachable objects in the tag map.
+
+  ZHeapIteratorRootOopClosure root_cl(this);
+
+  // Push strong roots onto stack
   {
-    // Note that we also visit the JVMTI weak tag map as if they were
-    // strong roots to make sure we visit all tagged objects, even
-    // those that might now have become unreachable. If we didn't do
-    // this the user would have expected to see ObjectFree events for
-    // unreachable objects in the tag map.
     ZRootsIterator roots;
-    ZConcurrentRootsIterator concurrent_roots(false /* marking */);
-    ZHeapIteratorRootOopClosure root_cl(this);
-    roots.oops_do(&root_cl, true /* visit_jvmti_weak_export */);
-    concurrent_roots.oops_do(&root_cl);
+    roots.oops_do(&root_cl);
+  }
+
+  {
+    ZConcurrentRootsIterator roots;
+    roots.oops_do(&root_cl);
+  }
+
+  // Push weak roots onto stack
+  {
+    ZWeakRootsIterator roots;
+    roots.oops_do(&root_cl);
+  }
+
+  {
+    ZConcurrentWeakRootsIterator roots;
+    roots.oops_do(&root_cl);
   }
 
   // Drain stack
--- a/src/hotspot/share/gc/z/zRootsIterator.hpp	Tue Nov 20 10:50:54 2018 +0530
+++ b/src/hotspot/share/gc/z/zRootsIterator.hpp	Tue Nov 20 08:17:24 2018 +0100
@@ -124,7 +124,7 @@
   ZParallelOopsDo<ZConcurrentRootsIterator, &ZConcurrentRootsIterator::do_class_loader_data_graph> _class_loader_data_graph;
 
 public:
-  ZConcurrentRootsIterator(bool marking);
+  ZConcurrentRootsIterator(bool marking = false);
   ~ZConcurrentRootsIterator();
 
   void oops_do(ZRootsIteratorClosure* cl);