8237363: Remove automatic is in heap verification in OopIterateClosure

Reviewed-by: eosterlund, pliden

Author: stefank

src/hotspot/share/gc/g1/heapRegion.cpp

@@ -516,9 +516,6 @@ class G1VerificationClosure : public BasicOopIterateClosure {
     obj->print_on(out);
 #endif // PRODUCT
   }
-
-  // This closure provides its own oop verification code.
-  debug_only(virtual bool should_verify_oops() { return false; })
 };
 
 class VerifyLiveClosure : public G1VerificationClosure {
@@ -655,9 +652,6 @@ class G1Mux2Closure : public BasicOopIterateClosure {
   }
   virtual inline void do_oop(oop* p) { do_oop_work(p); }
   virtual inline void do_oop(narrowOop* p) { do_oop_work(p); }
-
-  // This closure provides its own oop verification code.
-  debug_only(virtual bool should_verify_oops() { return false; })
 };
 
 void HeapRegion::verify(VerifyOption vo,

src/hotspot/share/gc/parallel/psCompactionManager.inline.hpp

@@ -47,9 +47,6 @@ class PCMarkAndPushClosure: public OopClosure {
   template <typename T> void do_oop_nv(T* p)      { _compaction_manager->mark_and_push(p); }
   virtual void do_oop(oop* p)                     { do_oop_nv(p); }
   virtual void do_oop(narrowOop* p)               { do_oop_nv(p); }
-
-  // This closure provides its own oop verification code.
-  debug_only(virtual bool should_verify_oops()    { return false; })
 };
 
 class PCIterateMarkAndPushClosure: public MetadataVisitingOopIterateClosure {
@@ -64,9 +61,6 @@ class PCIterateMarkAndPushClosure: public MetadataVisitingOopIterateClosure {
 
   void do_klass_nv(Klass* k)                      { _compaction_manager->follow_klass(k); }
   void do_cld_nv(ClassLoaderData* cld)            { _compaction_manager->follow_class_loader(cld); }
-
-  // This closure provides its own oop verification code.
-  debug_only(virtual bool should_verify_oops()    { return false; })
 };
 
 inline bool ParCompactionManager::steal(int queue_num, oop& t) {

src/hotspot/share/gc/parallel/psParallelCompact.inline.hpp

@@ -134,8 +134,6 @@ class PCAdjustPointerClosure: public BasicOopIterateClosure {
   virtual void do_oop(oop* p)                { do_oop_nv(p); }
   virtual void do_oop(narrowOop* p)          { do_oop_nv(p); }
 
-  // This closure provides its own oop verification code.
-  debug_only(virtual bool should_verify_oops() { return false; })
   virtual ReferenceIterationMode reference_iteration_mode() { return DO_FIELDS; }
 private:
   ParCompactionManager* _cm;

src/hotspot/share/gc/parallel/psPromotionManager.inline.hpp

@@ -97,9 +97,6 @@ class PSPushContentsClosure: public BasicOopIterateClosure {
 
   virtual void do_oop(oop* p)       { do_oop_nv(p); }
   virtual void do_oop(narrowOop* p) { do_oop_nv(p); }
-
-  // Don't use the oop verification code in the oop_oop_iterate framework.
-  debug_only(virtual bool should_verify_oops() { return false; })
 };
 
 //

src/hotspot/share/gc/serial/defNewGeneration.inline.hpp

@@ -45,7 +45,7 @@ inline void DefNewGeneration::KeepAliveClosure::do_oop_work(T* p) {
   }
 #endif // ASSERT
 
-  Devirtualizer::do_oop_no_verify(_cl, p);
+  Devirtualizer::do_oop(_cl, p);
 
   // Card marking is trickier for weak refs.
   // This oop is a 'next' field which was filled in while we
@@ -77,7 +77,7 @@ inline void DefNewGeneration::FastKeepAliveClosure::do_oop_work(T* p) {
   }
 #endif // ASSERT
 
-  Devirtualizer::do_oop_no_verify(_cl, p);
+  Devirtualizer::do_oop(_cl, p);
 
   // Optimized for Defnew generation if it's the youngest generation:
   // we set a younger_gen card if we have an older->youngest

src/hotspot/share/gc/serial/markSweep.hpp

@@ -191,9 +191,6 @@ class AdjustPointerClosure: public BasicOopIterateClosure {
   virtual void do_oop(oop* p);
   virtual void do_oop(narrowOop* p);
   virtual ReferenceIterationMode reference_iteration_mode() { return DO_FIELDS; }
-
-  // This closure provides its own oop verification code.
-  debug_only(virtual bool should_verify_oops() { return false; })
 };
 
 class PreservedMark {

src/hotspot/share/gc/z/zHeapIterator.cpp

@@ -160,12 +160,6 @@ class ZHeapIteratorOopClosure : public ClaimMetadataVisitingOopIterateClosure {
   virtual void do_oop(narrowOop* p) {
     ShouldNotReachHere();
   }
-
-#ifdef ASSERT
-  virtual bool should_verify_oops() {
-    return false;
-  }
-#endif
 };
 
 ZHeapIterator::ZHeapIterator(uint nworkers, bool visit_weaks) :

src/hotspot/share/gc/z/zOopClosures.hpp

@@ -31,12 +31,6 @@ class ZLoadBarrierOopClosure : public BasicOopIterateClosure {
 public:
   virtual void do_oop(oop* p);
   virtual void do_oop(narrowOop* p);
-
-#ifdef ASSERT
-  virtual bool should_verify_oops() {
-    return false;
-  }
-#endif
 };
 
 class ZNMethodOopClosure : public OopClosure {
@@ -52,12 +46,6 @@ class ZMarkBarrierOopClosure : public ClaimMetadataVisitingOopIterateClosure {
 
   virtual void do_oop(oop* p);
   virtual void do_oop(narrowOop* p);
-
-#ifdef ASSERT
-  virtual bool should_verify_oops() {
-    return false;
-  }
-#endif
 };
 
 class ZPhantomIsAliveObjectClosure : public BoolObjectClosure {

src/hotspot/share/gc/z/zVerify.cpp

@@ -214,13 +214,6 @@ class ZVerifyOopClosure : public ClaimMetadataVisitingOopIterateClosure, public
   virtual ReferenceIterationMode reference_iteration_mode() {
     return _verify_weaks ? DO_FIELDS : DO_FIELDS_EXCEPT_REFERENT;
   }
-
-#ifdef ASSERT
-  // Verification handled by the closure itself
-  virtual bool should_verify_oops() {
-    return false;
-  }
-#endif
 };
 
 template <typename RootsIterator>

src/hotspot/share/jfr/leakprofiler/chains/bfsClosure.hpp

@@ -64,7 +64,6 @@ class BFSClosure : public BasicOopIterateClosure {
 
  public:
   virtual ReferenceIterationMode reference_iteration_mode() { return DO_FIELDS_EXCEPT_REFERENT; }
-  virtual bool should_verify_oops() { return false; }
 
   BFSClosure(EdgeQueue* edge_queue, EdgeStore* edge_store, BitSet* mark_bits);
   void process();

src/hotspot/share/jfr/leakprofiler/chains/dfsClosure.hpp

@@ -54,7 +54,6 @@ class DFSClosure : public BasicOopIterateClosure {
 
  public:
   virtual ReferenceIterationMode reference_iteration_mode() { return DO_FIELDS_EXCEPT_REFERENT; }
-  virtual bool should_verify_oops() { return false; }
 
   static void find_leaks_from_edge(EdgeStore* edge_store, BitSet* mark_bits, const Edge* start_edge);
   static void find_leaks_from_root_set(EdgeStore* edge_store, BitSet* mark_bits);

src/hotspot/share/memory/filemap.cpp

@@ -1737,7 +1737,7 @@ address FileMapInfo::decode_start_address(FileMapRegion* spc, bool with_current_
   size_t offset = spc->mapping_offset();
   narrowOop n = CompressedOops::narrow_oop_cast(offset);
   if (with_current_oop_encoding_mode) {
-    return cast_from_oop<address>(CompressedOops::decode_not_null(n));
+    return cast_from_oop<address>(CompressedOops::decode_raw_not_null(n));
   } else {
     return cast_from_oop<address>(HeapShared::decode_from_archive(n));
   }

src/hotspot/share/memory/heapShared.cpp

@@ -1150,9 +1150,6 @@ class FindEmbeddedNonNullPointers: public BasicOopIterateClosure {
   FindEmbeddedNonNullPointers(narrowOop* start, BitMap* oopmap)
     : _start(start), _oopmap(oopmap), _num_total_oops(0),  _num_null_oops(0) {}
 
-  virtual bool should_verify_oops(void) {
-    return false;
-  }
   virtual void do_oop(narrowOop* p) {
     _num_total_oops ++;
     narrowOop v = *p;

src/hotspot/share/memory/iterator.hpp

@@ -102,14 +102,6 @@ class OopIterateClosure : public OopClosure {
   virtual bool do_metadata() = 0;
   virtual void do_klass(Klass* k) = 0;
   virtual void do_cld(ClassLoaderData* cld) = 0;
-
-#ifdef ASSERT
-  // Default verification of each visited oop field.
-  template <typename T> void verify(T* p);
-
-  // Can be used by subclasses to turn off the default verification of oop fields.
-  virtual bool should_verify_oops() { return true; }
-#endif
 };
 
 // An OopIterateClosure that can be used when there's no need to visit the Metadata.
@@ -357,7 +349,6 @@ class CompareClosure : public Closure {
 // a concrete implementation, otherwise a virtual call is taken.
 class Devirtualizer {
  public:
-  template <typename OopClosureType, typename T> static void do_oop_no_verify(OopClosureType* closure, T* p);
   template <typename OopClosureType, typename T> static void do_oop(OopClosureType* closure, T* p);
   template <typename OopClosureType>             static void do_klass(OopClosureType* closure, Klass* k);
   template <typename OopClosureType>             static void do_cld(OopClosureType* closure, ClassLoaderData* cld);

src/hotspot/share/memory/iterator.inline.hpp

@@ -27,7 +27,6 @@
 
 #include "classfile/classLoaderData.hpp"
 #include "memory/iterator.hpp"
-#include "memory/universe.hpp"
 #include "oops/access.inline.hpp"
 #include "oops/compressedOops.inline.hpp"
 #include "oops/klass.hpp"
@@ -52,22 +51,6 @@ inline void ClaimMetadataVisitingOopIterateClosure::do_klass(Klass* k) {
   ClaimMetadataVisitingOopIterateClosure::do_cld(cld);
 }
 
-#ifdef ASSERT
-// This verification is applied to all visited oops.
-// The closures can turn is off by overriding should_verify_oops().
-template <typename T>
-void OopIterateClosure::verify(T* p) {
-  if (should_verify_oops()) {
-    T heap_oop = RawAccess<>::oop_load(p);
-    if (!CompressedOops::is_null(heap_oop)) {
-      oop o = CompressedOops::decode_not_null(heap_oop);
-      assert(Universe::heap()->is_in(o),
-             "should be in closed *p " PTR_FORMAT " " PTR_FORMAT, p2i(p), p2i(o));
-    }
-  }
-}
-#endif
-
 // Implementation of the non-virtual do_oop dispatch.
 //
 // The same implementation is used for do_metadata, do_klass, and do_cld.
@@ -123,16 +106,9 @@ call_do_oop(void (Receiver::*)(T*), void (Base::*)(T*), OopClosureType* closure,
   closure->OopClosureType::do_oop(p);
 }
 
-template <typename OopClosureType, typename T>
-inline void Devirtualizer::do_oop_no_verify(OopClosureType* closure, T* p) {
-  call_do_oop<T>(&OopClosureType::do_oop, &OopClosure::do_oop, closure, p);
-}
-
 template <typename OopClosureType, typename T>
 inline void Devirtualizer::do_oop(OopClosureType* closure, T* p) {
-  debug_only(closure->verify(p));
-
-  do_oop_no_verify(closure, p);
+  call_do_oop<T>(&OopClosureType::do_oop, &OopClosure::do_oop, closure, p);
 }
 
 // Implementation of the non-virtual do_metadata dispatch.

src/hotspot/share/oops/compressedOops.hpp

@@ -119,17 +119,18 @@ class CompressedOops : public AllStatic {
   static bool is_null(oop v)       { return v == NULL; }
   static bool is_null(narrowOop v) { return v == narrowOop::null; }
 
+  static inline oop decode_raw_not_null(narrowOop v);
   static inline oop decode_raw(narrowOop v);
   static inline oop decode_not_null(narrowOop v);
   static inline oop decode(narrowOop v);
   static inline narrowOop encode_not_null(oop v);
   static inline narrowOop encode(oop v);
 
   // No conversions needed for these overloads
-  static oop decode_not_null(oop v)             { return v; }
-  static oop decode(oop v)                      { return v; }
-  static narrowOop encode_not_null(narrowOop v) { return v; }
-  static narrowOop encode(narrowOop v)          { return v; }
+  static inline oop decode_not_null(oop v);
+  static inline oop decode(oop v);
+  static inline narrowOop encode_not_null(narrowOop v);
+  static inline narrowOop encode(narrowOop v);
 
   static inline uint32_t narrow_oop_value(oop o);
   static inline uint32_t narrow_oop_value(narrowOop o);

src/hotspot/share/oops/compressedOops.inline.hpp

@@ -41,6 +41,11 @@
 // offset from the heap base.  Saving the check for null can save instructions
 // in inner GC loops so these are separated.
 
+inline oop CompressedOops::decode_raw_not_null(narrowOop v) {
+  assert(!is_null(v), "narrow oop value can never be zero");
+  return decode_raw(v);
+}
+
 inline oop CompressedOops::decode_raw(narrowOop v) {
   return (oop)(void*)((uintptr_t)base() + ((uintptr_t)v << shift()));
 }
@@ -49,6 +54,7 @@ inline oop CompressedOops::decode_not_null(narrowOop v) {
   assert(!is_null(v), "narrow oop value can never be zero");
   oop result = decode_raw(v);
   assert(is_object_aligned(result), "address not aligned: " INTPTR_FORMAT, p2i((void*) result));
+  assert(Universe::heap()->is_in(result), "object not in heap " PTR_FORMAT, p2i((void*) result));
   return result;
 }
 
@@ -63,14 +69,32 @@ inline narrowOop CompressedOops::encode_not_null(oop v) {
   uint64_t  pd = (uint64_t)(pointer_delta((void*)v, (void*)base(), 1));
   assert(OopEncodingHeapMax > pd, "change encoding max if new encoding");
   narrowOop result = narrow_oop_cast(pd >> shift());
-  assert(decode(result) == v, "reversibility");
+  assert(decode_raw(result) == v, "reversibility");
   return result;
 }
 
 inline narrowOop CompressedOops::encode(oop v) {
   return is_null(v) ? narrowOop::null : encode_not_null(v);
 }
 
+inline oop CompressedOops::decode_not_null(oop v) {
+  assert(Universe::heap()->is_in(v), "object not in heap " PTR_FORMAT, p2i((void*) v));
+  return v;
+}
+
+inline oop CompressedOops::decode(oop v) {
+  assert(Universe::heap()->is_in_or_null(v), "object not in heap " PTR_FORMAT, p2i((void*) v));
+  return v;
+}
+
+inline narrowOop CompressedOops::encode_not_null(narrowOop v) {
+  return v;
+}
+
+inline narrowOop CompressedOops::encode(narrowOop v) {
+  return v;
+}
+
 inline uint32_t CompressedOops::narrow_oop_value(oop o) {
   return narrow_oop_value(encode(o));
 }