8212753: Improve oopDesc::forward_to_atomic
authortschatzl
Wed, 24 Oct 2018 16:22:34 +0200
changeset 52271 1587306fe23f
parent 52270 487bd00f4ea8
child 52272 be166557c8dc
child 52273 97b761e247b3
8212753: Improve oopDesc::forward_to_atomic Summary: Avoid multiple unnecessary reloads of the mark oop in oopDesc::forward_to_atomic Reviewed-by: kbarrett, mdoerr
src/hotspot/share/gc/cms/parNewGeneration.cpp
src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
src/hotspot/share/oops/oop.hpp
src/hotspot/share/oops/oop.inline.hpp
--- a/src/hotspot/share/gc/cms/parNewGeneration.cpp	Wed Oct 24 14:59:21 2018 +0200
+++ b/src/hotspot/share/gc/cms/parNewGeneration.cpp	Wed Oct 24 16:22:34 2018 +0200
@@ -1118,7 +1118,7 @@
 
     // Attempt to install a null forwarding pointer (atomically),
     // to claim the right to install the real forwarding pointer.
-    forward_ptr = old->forward_to_atomic(ClaimedForwardPtr);
+    forward_ptr = old->forward_to_atomic(ClaimedForwardPtr, m);
     if (forward_ptr != NULL) {
       // someone else beat us to it.
         return real_forwardee(old);
@@ -1144,7 +1144,7 @@
     // Is in to-space; do copying ourselves.
     Copy::aligned_disjoint_words((HeapWord*)old, (HeapWord*)new_obj, sz);
     assert(CMSHeap::heap()->is_in_reserved(new_obj), "illegal forwarding pointer value.");
-    forward_ptr = old->forward_to_atomic(new_obj);
+    forward_ptr = old->forward_to_atomic(new_obj, m);
     // Restore the mark word copied above.
     new_obj->set_mark_raw(m);
     // Increment age if obj still in new generation
--- a/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp	Wed Oct 24 14:59:21 2018 +0200
+++ b/src/hotspot/share/gc/g1/g1ParScanThreadState.cpp	Wed Oct 24 16:22:34 2018 +0200
@@ -265,7 +265,7 @@
   Prefetch::write(obj_ptr, PrefetchCopyIntervalInBytes);
 
   const oop obj = oop(obj_ptr);
-  const oop forward_ptr = old->forward_to_atomic(obj, memory_order_relaxed);
+  const oop forward_ptr = old->forward_to_atomic(obj, old_mark, memory_order_relaxed);
   if (forward_ptr == NULL) {
     Copy::aligned_disjoint_words((HeapWord*) old, obj_ptr, word_sz);
 
@@ -355,7 +355,7 @@
 oop G1ParScanThreadState::handle_evacuation_failure_par(oop old, markOop m) {
   assert(_g1h->is_in_cset(old), "Object " PTR_FORMAT " should be in the CSet", p2i(old));
 
-  oop forward_ptr = old->forward_to_atomic(old, memory_order_relaxed);
+  oop forward_ptr = old->forward_to_atomic(old, m, memory_order_relaxed);
   if (forward_ptr == NULL) {
     // Forward-to-self succeeded. We are the "owner" of the object.
     HeapRegion* r = _g1h->heap_region_containing(old);
--- a/src/hotspot/share/oops/oop.hpp	Wed Oct 24 14:59:21 2018 +0200
+++ b/src/hotspot/share/oops/oop.hpp	Wed Oct 24 16:22:34 2018 +0200
@@ -273,7 +273,7 @@
   // Exactly one thread succeeds in inserting the forwarding pointer, and
   // this call returns "NULL" for that thread; any other thread has the
   // value of the forwarding pointer returned and does not modify "this".
-  inline oop forward_to_atomic(oop p, atomic_memory_order order = memory_order_conservative);
+  inline oop forward_to_atomic(oop p, markOop compare, atomic_memory_order order = memory_order_conservative);
 
   inline oop forwardee() const;
   inline oop forwardee_acquire() const;
--- a/src/hotspot/share/oops/oop.inline.hpp	Wed Oct 24 14:59:21 2018 +0200
+++ b/src/hotspot/share/oops/oop.inline.hpp	Wed Oct 24 16:22:34 2018 +0200
@@ -370,26 +370,21 @@
   return cas_set_mark_raw(m, compare, order) == compare;
 }
 
-oop oopDesc::forward_to_atomic(oop p, atomic_memory_order order) {
-  markOop oldMark = mark_raw();
-  markOop forwardPtrMark = markOopDesc::encode_pointer_as_mark(p);
-  markOop curMark;
-
-  assert(forwardPtrMark->decode_pointer() == p, "encoding must be reversable");
-  assert(sizeof(markOop) == sizeof(intptr_t), "CAS below requires this.");
-
-  while (!oldMark->is_marked()) {
-    curMark = cas_set_mark_raw(forwardPtrMark, oldMark, order);
-    assert(is_forwarded(), "object should have been forwarded");
-    if (curMark == oldMark) {
-      return NULL;
-    }
-    // If the CAS was unsuccessful then curMark->is_marked()
-    // should return true as another thread has CAS'd in another
-    // forwarding pointer.
-    oldMark = curMark;
+oop oopDesc::forward_to_atomic(oop p, markOop compare, atomic_memory_order order) {
+  // CMS forwards some non-heap value into the mark oop to reserve oops during
+  // promotion, so the next two asserts do not hold.
+  assert(UseConcMarkSweepGC || check_obj_alignment(p),
+         "forwarding to something not aligned");
+  assert(UseConcMarkSweepGC || Universe::heap()->is_in_reserved(p),
+         "forwarding to something not in heap");
+  markOop m = markOopDesc::encode_pointer_as_mark(p);
+  assert(m->decode_pointer() == p, "encoding must be reversable");
+  markOop old_mark = cas_set_mark_raw(m, compare, order);
+  if (old_mark == compare) {
+    return NULL;
+  } else {
+    return (oop)old_mark->decode_pointer();
   }
-  return forwardee();
 }
 
 // Note that the forwardee is not the same thing as the displaced_mark.