8229839: Break circular dependency between oop.inline.hpp and markWord.inline.hpp
authorstefank
Mon, 19 Aug 2019 12:40:27 +0200
changeset 57812 9bb28ccc6106
parent 57811 947252a54b98
child 57813 80fad5f39a42
8229839: Break circular dependency between oop.inline.hpp and markWord.inline.hpp Reviewed-by: pliden, tonyp
src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp
src/hotspot/share/gc/cms/promotionInfo.cpp
src/hotspot/share/gc/g1/g1FullGCCompactionPoint.cpp
src/hotspot/share/gc/g1/g1FullGCMarker.inline.hpp
src/hotspot/share/gc/g1/g1FullGCOopClosures.inline.hpp
src/hotspot/share/gc/serial/markSweep.inline.hpp
src/hotspot/share/gc/shared/preservedMarks.inline.hpp
src/hotspot/share/oops/markWord.hpp
src/hotspot/share/oops/markWord.inline.hpp
src/hotspot/share/oops/oop.hpp
src/hotspot/share/oops/oop.inline.hpp
src/hotspot/share/prims/jvmtiTagMap.cpp
--- a/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/gc/cms/concurrentMarkSweepGeneration.cpp	Mon Aug 19 12:40:27 2019 +0200
@@ -8008,14 +8008,14 @@
 // Single threaded
 void CMSCollector::preserve_mark_if_necessary(oop p) {
   markWord m = p->mark_raw();
-  if (m.must_be_preserved(p)) {
+  if (p->mark_must_be_preserved(m)) {
     preserve_mark_work(p, m);
   }
 }
 
 void CMSCollector::par_preserve_mark_if_necessary(oop p) {
   markWord m = p->mark_raw();
-  if (m.must_be_preserved(p)) {
+  if (p->mark_must_be_preserved(m)) {
     MutexLocker x(ParGCRareEvent_lock, Mutex::_no_safepoint_check_flag);
     // Even though we read the mark word without holding
     // the lock, we are assured that it will not change
--- a/src/hotspot/share/gc/cms/promotionInfo.cpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/gc/cms/promotionInfo.cpp	Mon Aug 19 12:40:27 2019 +0200
@@ -28,7 +28,7 @@
 #include "gc/shared/genOopClosures.hpp"
 #include "oops/compressedOops.inline.hpp"
 #include "oops/markWord.inline.hpp"
-#include "oops/oop.hpp"
+#include "oops/oop.inline.hpp"
 
 /////////////////////////////////////////////////////////////////////////
 //// PromotionInfo
--- a/src/hotspot/share/gc/g1/g1FullGCCompactionPoint.cpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1FullGCCompactionPoint.cpp	Mon Aug 19 12:40:27 2019 +0200
@@ -116,11 +116,11 @@
     } else {
       // Make sure object has the correct mark-word set or that it will be
       // fixed when restoring the preserved marks.
-      assert(object->mark_raw() == markWord::prototype_for_object(object) || // Correct mark
-             object->mark_raw().must_be_preserved(object) || // Will be restored by PreservedMarksSet
+      assert(object->mark_raw() == markWord::prototype_for_klass(object->klass()) || // Correct mark
+             object->mark_must_be_preserved() || // Will be restored by PreservedMarksSet
              (UseBiasedLocking && object->has_bias_pattern_raw()), // Will be restored by BiasedLocking
              "should have correct prototype obj: " PTR_FORMAT " mark: " PTR_FORMAT " prototype: " PTR_FORMAT,
-             p2i(object), object->mark_raw().value(), markWord::prototype_for_object(object).value());
+             p2i(object), object->mark_raw().value(), markWord::prototype_for_klass(object->klass()).value());
     }
     assert(object->forwardee() == NULL, "should be forwarded to NULL");
   }
--- a/src/hotspot/share/gc/g1/g1FullGCMarker.inline.hpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1FullGCMarker.inline.hpp	Mon Aug 19 12:40:27 2019 +0200
@@ -51,7 +51,7 @@
 
   // Marked by us, preserve if needed.
   markWord mark = obj->mark_raw();
-  if (mark.must_be_preserved(obj) &&
+  if (obj->mark_must_be_preserved(mark) &&
       !G1ArchiveAllocator::is_open_archive_object(obj)) {
     preserved_stack()->push(obj, mark);
   }
--- a/src/hotspot/share/gc/g1/g1FullGCOopClosures.inline.hpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/gc/g1/g1FullGCOopClosures.inline.hpp	Mon Aug 19 12:40:27 2019 +0200
@@ -77,11 +77,11 @@
   oop forwardee = obj->forwardee();
   if (forwardee == NULL) {
     // Not forwarded, return current reference.
-    assert(obj->mark_raw() == markWord::prototype_for_object(obj) || // Correct mark
-           obj->mark_raw().must_be_preserved(obj) || // Will be restored by PreservedMarksSet
+    assert(obj->mark_raw() == markWord::prototype_for_klass(obj->klass()) || // Correct mark
+           obj->mark_must_be_preserved() || // Will be restored by PreservedMarksSet
            (UseBiasedLocking && obj->has_bias_pattern_raw()), // Will be restored by BiasedLocking
            "Must have correct prototype or be preserved, obj: " PTR_FORMAT ", mark: " PTR_FORMAT ", prototype: " PTR_FORMAT,
-           p2i(obj), obj->mark_raw().value(), markWord::prototype_for_object(obj).value());
+           p2i(obj), obj->mark_raw().value(), markWord::prototype_for_klass(obj->klass()).value());
     return;
   }
 
--- a/src/hotspot/share/gc/serial/markSweep.inline.hpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/gc/serial/markSweep.inline.hpp	Mon Aug 19 12:40:27 2019 +0200
@@ -40,7 +40,7 @@
   markWord mark = obj->mark_raw();
   obj->set_mark_raw(markWord::prototype().set_marked());
 
-  if (mark.must_be_preserved(obj)) {
+  if (obj->mark_must_be_preserved(mark)) {
     preserve_mark(obj, mark);
   }
 }
--- a/src/hotspot/share/gc/shared/preservedMarks.inline.hpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/gc/shared/preservedMarks.inline.hpp	Mon Aug 19 12:40:27 2019 +0200
@@ -31,7 +31,7 @@
 #include "utilities/stack.inline.hpp"
 
 inline bool PreservedMarks::should_preserve_mark(oop obj, markWord m) const {
-  return m.must_be_preserved_for_promotion_failure(obj);
+  return obj->mark_must_be_preserved_for_promotion_failure(m);
 }
 
 inline void PreservedMarks::push(oop obj, markWord m) {
--- a/src/hotspot/share/oops/markWord.hpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/oops/markWord.hpp	Mon Aug 19 12:40:27 2019 +0200
@@ -252,8 +252,8 @@
   static markWord INFLATING() { return zero(); }    // inflate-in-progress
 
   // Should this header be preserved during GC?
-  inline bool must_be_preserved(oop obj_containing_mark) const;
-  inline bool must_be_preserved_with_bias(oop obj_containing_mark) const;
+  template <typename KlassProxy>
+  inline bool must_be_preserved(KlassProxy klass) const;
 
   // Should this header (including its age bits) be preserved in the
   // case of a promotion failure during scavenge?
@@ -272,15 +272,14 @@
   // observation is that promotion failures are quite rare and
   // reducing the number of mark words preserved during them isn't a
   // high priority.
-  inline bool must_be_preserved_for_promotion_failure(oop obj_containing_mark) const;
-  inline bool must_be_preserved_with_bias_for_promotion_failure(oop obj_containing_mark) const;
+  template <typename KlassProxy>
+  inline bool must_be_preserved_for_promotion_failure(KlassProxy klass) const;
 
   // Should this header be preserved during a scavenge where CMS is
   // the old generation?
   // (This is basically the same body as must_be_preserved_for_promotion_failure(),
   // but takes the Klass* as argument instead)
   inline bool must_be_preserved_for_cms_scavenge(Klass* klass_of_obj_containing_mark) const;
-  inline bool must_be_preserved_with_bias_for_cms_scavenge(Klass* klass_of_obj_containing_mark) const;
 
   // WARNING: The following routines are used EXCLUSIVELY by
   // synchronization functions. They are not really gc safe.
@@ -372,7 +371,7 @@
   }
 
   // Helper function for restoration of unmarked mark oops during GC
-  static inline markWord prototype_for_object(oop obj);
+  static inline markWord prototype_for_klass(const Klass* klass);
 
   // Debugging
   void print_on(outputStream* st) const;
--- a/src/hotspot/share/oops/markWord.inline.hpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/oops/markWord.inline.hpp	Mon Aug 19 12:40:27 2019 +0200
@@ -27,88 +27,57 @@
 
 #include "oops/klass.hpp"
 #include "oops/markWord.hpp"
-#include "oops/oop.inline.hpp"
 #include "runtime/globals.hpp"
 
-// Should this header be preserved during GC (when biased locking is enabled)?
-inline bool markWord::must_be_preserved_with_bias(oop obj_containing_mark) const {
-  assert(UseBiasedLocking, "unexpected");
-  if (has_bias_pattern()) {
-    // Will reset bias at end of collection
-    // Mark words of biased and currently locked objects are preserved separately
-    return false;
-  }
-  markWord prototype_header = prototype_for_object(obj_containing_mark);
-  if (prototype_header.has_bias_pattern()) {
-    // Individual instance which has its bias revoked; must return
-    // true for correctness
-    return true;
+// Should this header be preserved during GC?
+template <typename KlassProxy>
+inline bool markWord::must_be_preserved(KlassProxy klass) const {
+  if (UseBiasedLocking) {
+    if (has_bias_pattern()) {
+      // Will reset bias at end of collection
+      // Mark words of biased and currently locked objects are preserved separately
+      return false;
+    }
+    markWord prototype_header = prototype_for_klass(klass);
+    if (prototype_header.has_bias_pattern()) {
+      // Individual instance which has its bias revoked; must return
+      // true for correctness
+      return true;
+    }
   }
   return (!is_unlocked() || !has_no_hash());
 }
 
-// Should this header be preserved during GC?
-inline bool markWord::must_be_preserved(oop obj_containing_mark) const {
-  if (!UseBiasedLocking)
-    return (!is_unlocked() || !has_no_hash());
-  return must_be_preserved_with_bias(obj_containing_mark);
-}
-
-// Should this header be preserved in the case of a promotion failure
-// during scavenge (when biased locking is enabled)?
-inline bool markWord::must_be_preserved_with_bias_for_promotion_failure(oop obj_containing_mark) const {
-  assert(UseBiasedLocking, "unexpected");
-  // We don't explicitly save off the mark words of biased and
-  // currently-locked objects during scavenges, so if during a
-  // promotion failure we encounter either a biased mark word or a
-  // klass which still has a biasable prototype header, we have to
-  // preserve the mark word. This results in oversaving, but promotion
-  // failures are rare, and this avoids adding more complex logic to
-  // the scavengers to call new variants of
-  // BiasedLocking::preserve_marks() / restore_marks() in the middle
-  // of a scavenge when a promotion failure has first been detected.
-  if (has_bias_pattern() ||
-      prototype_for_object(obj_containing_mark).has_bias_pattern()) {
-    return true;
+// Should this header be preserved in the case of a promotion failure during scavenge?
+template <typename KlassProxy>
+inline bool markWord::must_be_preserved_for_promotion_failure(KlassProxy klass) const {
+  if (UseBiasedLocking) {
+    // We don't explicitly save off the mark words of biased and
+    // currently-locked objects during scavenges, so if during a
+    // promotion failure we encounter either a biased mark word or a
+    // klass which still has a biasable prototype header, we have to
+    // preserve the mark word. This results in oversaving, but promotion
+    // failures are rare, and this avoids adding more complex logic to
+    // the scavengers to call new variants of
+    // BiasedLocking::preserve_marks() / restore_marks() in the middle
+    // of a scavenge when a promotion failure has first been detected.
+    if (has_bias_pattern() || prototype_for_klass(klass).has_bias_pattern()) {
+      return true;
+    }
   }
   return (!is_unlocked() || !has_no_hash());
 }
 
-// Should this header be preserved in the case of a promotion failure
-// during scavenge?
-inline bool markWord::must_be_preserved_for_promotion_failure(oop obj_containing_mark) const {
-  if (!UseBiasedLocking)
-    return (!is_unlocked() || !has_no_hash());
-  return must_be_preserved_with_bias_for_promotion_failure(obj_containing_mark);
+// Same as must_be_preserved_for_promotion_failure().
+inline bool markWord::must_be_preserved_for_cms_scavenge(Klass* klass_of_obj_containing_mark) const {
+  return must_be_preserved_for_promotion_failure(klass_of_obj_containing_mark);
 }
 
-
-// Same as must_be_preserved_with_bias_for_promotion_failure() except that
-// it takes a Klass* argument, instead of the object of which this is the mark word.
-inline bool markWord::must_be_preserved_with_bias_for_cms_scavenge(Klass* klass_of_obj_containing_mark) const {
-  assert(UseBiasedLocking, "unexpected");
-  // CMS scavenges preserve mark words in similar fashion to promotion failures; see above
-  if (has_bias_pattern() ||
-      klass_of_obj_containing_mark->prototype_header().has_bias_pattern()) {
-    return true;
-  }
-  return (!is_unlocked() || !has_no_hash());
-}
+inline markWord markWord::prototype_for_klass(const Klass* klass) {
+  markWord prototype_header = klass->prototype_header();
+  assert(prototype_header == prototype() || prototype_header.has_bias_pattern(), "corrupt prototype header");
 
-// Same as must_be_preserved_for_promotion_failure() except that
-// it takes a Klass* argument, instead of the object of which this is the mark word.
-inline bool markWord::must_be_preserved_for_cms_scavenge(Klass* klass_of_obj_containing_mark) const {
-  if (!UseBiasedLocking)
-    return (!is_unlocked() || !has_no_hash());
-  return must_be_preserved_with_bias_for_cms_scavenge(klass_of_obj_containing_mark);
-}
-
-inline markWord markWord::prototype_for_object(oop obj) {
-#ifdef ASSERT
-  markWord prototype_header = obj->klass()->prototype_header();
-  assert(prototype_header == prototype() || prototype_header.has_bias_pattern(), "corrupt prototype header");
-#endif
-  return obj->klass()->prototype_header();
+  return prototype_header;
 }
 
 #endif // SHARE_OOPS_MARKWORD_INLINE_HPP
--- a/src/hotspot/share/oops/oop.hpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/oops/oop.hpp	Mon Aug 19 12:40:27 2019 +0200
@@ -313,6 +313,11 @@
   inline markWord displaced_mark_raw() const;
   inline void     set_displaced_mark_raw(markWord m);
 
+  // Checks if the mark word needs to be preserved
+  inline bool mark_must_be_preserved() const;
+  inline bool mark_must_be_preserved(markWord m) const;
+  inline bool mark_must_be_preserved_for_promotion_failure(markWord m) const;
+
   static bool has_klass_gap();
 
   // for code generation
--- a/src/hotspot/share/oops/oop.inline.hpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/oops/oop.inline.hpp	Mon Aug 19 12:40:27 2019 +0200
@@ -82,11 +82,11 @@
 }
 
 void oopDesc::init_mark() {
-  set_mark(markWord::prototype_for_object(this));
+  set_mark(markWord::prototype_for_klass(klass()));
 }
 
 void oopDesc::init_mark_raw() {
-  set_mark_raw(markWord::prototype_for_object(this));
+  set_mark_raw(markWord::prototype_for_klass(klass()));
 }
 
 Klass* oopDesc::klass() const {
@@ -483,4 +483,34 @@
   mark_raw().set_displaced_mark_helper(m);
 }
 
+// Supports deferred calling of obj->klass().
+class DeferredObjectToKlass {
+  const oopDesc* _obj;
+
+public:
+  DeferredObjectToKlass(const oopDesc* obj) : _obj(obj) {}
+
+  // Implicitly convertible to const Klass*.
+  operator const Klass*() const {
+    return _obj->klass();
+  }
+};
+
+bool oopDesc::mark_must_be_preserved() const {
+  return mark_must_be_preserved(mark_raw());
+}
+
+bool oopDesc::mark_must_be_preserved(markWord m) const {
+  // There's a circular dependency between oop.inline.hpp and
+  // markWord.inline.hpp because markWord::must_be_preserved wants to call
+  // oopDesc::klass(). This could be solved by calling klass() here. However,
+  // not all paths inside must_be_preserved calls klass(). Defer the call until
+  // the klass is actually needed.
+  return m.must_be_preserved(DeferredObjectToKlass(this));
+}
+
+bool oopDesc::mark_must_be_preserved_for_promotion_failure(markWord m) const {
+  return m.must_be_preserved_for_promotion_failure(DeferredObjectToKlass(this));
+}
+
 #endif // SHARE_OOPS_OOP_INLINE_HPP
--- a/src/hotspot/share/prims/jvmtiTagMap.cpp	Mon Aug 19 11:30:03 2019 +0200
+++ b/src/hotspot/share/prims/jvmtiTagMap.cpp	Mon Aug 19 12:40:27 2019 +0200
@@ -1712,7 +1712,7 @@
   // object's mark word
   markWord mark = o->mark();
 
-  if (mark.must_be_preserved(o)) {
+  if (o->mark_must_be_preserved(mark)) {
     _saved_mark_stack->push(mark);
     _saved_oop_stack->push(o);
   }