8016325: JVM hangs verifying system dictionary

Summary: Minimize redundant verifications of Klasses.
Reviewed-by: hseigel, jmasa

src/cpu/sparc/vm/frame_sparc.cpp

@@ -680,7 +680,7 @@ bool frame::is_interpreted_frame_valid(JavaThread* thread) const {
 
   // validate ConstantPoolCache*
   ConstantPoolCache* cp = *interpreter_frame_cache_addr();
-  if (cp == NULL || !cp->is_metadata()) return false;
+  if (cp == NULL || !cp->is_metaspace_object()) return false;
 
   // validate locals
 

src/cpu/x86/vm/frame_x86.cpp

@@ -587,7 +587,7 @@ bool frame::is_interpreted_frame_valid(JavaThread* thread) const {
 
   // validate ConstantPoolCache*
   ConstantPoolCache* cp = *interpreter_frame_cache_addr();
-  if (cp == NULL || !cp->is_metadata()) return false;
+  if (cp == NULL || !cp->is_metaspace_object()) return false;
 
   // validate locals
 

src/share/vm/ci/ciObjectFactory.cpp

@@ -265,8 +265,6 @@ ciObject* ciObjectFactory::get(oop key) {
 ciMetadata* ciObjectFactory::get_metadata(Metadata* key) {
   ASSERT_IN_VM;
 
-  assert(key == NULL || key->is_metadata(), "must be");
-
 #ifdef ASSERT
   if (CIObjectFactoryVerify) {
     Metadata* last = NULL;

src/share/vm/classfile/dictionary.cpp

@@ -555,7 +555,7 @@ void Dictionary::verify() {
                 loader_data->class_loader() == NULL ||
                 loader_data->class_loader()->is_instance(),
                 "checking type of class_loader");
-      e->verify();
+      e->verify(/*check_dictionary*/false);
       probe->verify_protection_domain_set();
       element_count++;
     }

src/share/vm/code/debugInfo.hpp

@@ -274,7 +274,7 @@ class DebugInfoReadStream : public CompressedReadStream {
   Method* read_method() {
     Method* o = (Method*)(code()->metadata_at(read_int()));
     assert(o == NULL ||
-           o->is_metadata(), "meta data only");
+           o->is_metaspace_object(), "meta data only");
     return o;
   }
   ScopeValue* read_object_value();

src/share/vm/code/dependencies.cpp

@@ -655,8 +655,8 @@ inline Metadata* Dependencies::DepStream::recorded_metadata_at(int i) {
   } else {
     o = _deps->oop_recorder()->metadata_at(i);
   }
-  assert(o == NULL || o->is_metadata(),
-         err_msg("Should be perm " PTR_FORMAT, o));
+  assert(o == NULL || o->is_metaspace_object(),
+         err_msg("Should be metadata " PTR_FORMAT, o));
   return o;
 }
 

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

@@ -798,7 +798,7 @@ void HeapRegion::verify(VerifyOption vo,
     if (!g1->is_obj_dead_cond(obj, this, vo)) {
       if (obj->is_oop()) {
         Klass* klass = obj->klass();
-        if (!klass->is_metadata()) {
+        if (!klass->is_metaspace_object()) {
           gclog_or_tty->print_cr("klass "PTR_FORMAT" of object "PTR_FORMAT" "
                                  "not metadata", klass, obj);
           *failures = true;

src/share/vm/memory/allocation.cpp

@@ -71,13 +71,6 @@ bool MetaspaceObj::is_shared() const {
   return MetaspaceShared::is_in_shared_space(this);
 }
 
-bool MetaspaceObj::is_metadata() const {
-  // GC Verify checks use this in guarantees.
-  // TODO: either replace them with is_metaspace_object() or remove them.
-  // is_metaspace_object() is slower than this test.  This test doesn't
-  // seem very useful for metaspace objects anymore though.
-  return !Universe::heap()->is_in_reserved(this);
-}
 
 bool MetaspaceObj::is_metaspace_object() const {
   return Metaspace::contains((void*)this);

src/share/vm/memory/allocation.hpp

@@ -264,7 +264,6 @@ class ClassLoaderData;
 
 class MetaspaceObj {
  public:
-  bool is_metadata() const;
   bool is_metaspace_object() const;  // more specific test but slower
   bool is_shared() const;
   void print_address_on(outputStream* st) const;  // nonvirtual address printing

src/share/vm/memory/heapInspection.cpp

@@ -157,7 +157,6 @@ KlassInfoTable::~KlassInfoTable() {
 }
 
 uint KlassInfoTable::hash(const Klass* p) {
-  assert(p->is_metadata(), "all klasses are metadata");
   return (uint)(((uintptr_t)p - (uintptr_t)_ref) >> 2);
 }
 

src/share/vm/oops/arrayKlass.cpp

@@ -221,8 +221,8 @@ void ArrayKlass::oop_print_on(oop obj, outputStream* st) {
 
 // Verification
 
-void ArrayKlass::verify_on(outputStream* st) {
-  Klass::verify_on(st);
+void ArrayKlass::verify_on(outputStream* st, bool check_dictionary) {
+  Klass::verify_on(st, check_dictionary);
 
   if (component_mirror() != NULL) {
     guarantee(component_mirror()->klass() != NULL, "should have a class");

src/share/vm/oops/arrayKlass.hpp

@@ -152,7 +152,7 @@ class ArrayKlass: public Klass {
   void oop_print_on(oop obj, outputStream* st);
 
   // Verification
-  void verify_on(outputStream* st);
+  void verify_on(outputStream* st, bool check_dictionary);
 
   void oop_verify_on(oop obj, outputStream* st);
 };

src/share/vm/oops/compiledICHolder.cpp

@@ -48,8 +48,6 @@ void CompiledICHolder::print_value_on(outputStream* st) const {
 // Verification
 
 void CompiledICHolder::verify_on(outputStream* st) {
-  guarantee(holder_method()->is_metadata(),   "should be in metaspace");
   guarantee(holder_method()->is_method(), "should be method");
-  guarantee(holder_klass()->is_metadata(),    "should be in metaspace");
   guarantee(holder_klass()->is_klass(),   "should be klass");
 }

src/share/vm/oops/constMethod.cpp

@@ -440,7 +440,6 @@ void ConstMethod::collect_statistics(KlassSizeStats *sz) const {
 
 void ConstMethod::verify_on(outputStream* st) {
   guarantee(is_constMethod(), "object must be constMethod");
-  guarantee(is_metadata(), err_msg("Should be metadata " PTR_FORMAT, this));
 
   // Verification can occur during oop construction before the method or
   // other fields have been initialized.

src/share/vm/oops/constantPool.cpp

@@ -2095,12 +2095,10 @@ void ConstantPool::verify_on(outputStream* st) {
     CPSlot entry = slot_at(i);
     if (tag.is_klass()) {
       if (entry.is_resolved()) {
-        guarantee(entry.get_klass()->is_metadata(), "should be metadata");
         guarantee(entry.get_klass()->is_klass(),    "should be klass");
       }
     } else if (tag.is_unresolved_klass()) {
       if (entry.is_resolved()) {
-        guarantee(entry.get_klass()->is_metadata(), "should be metadata");
         guarantee(entry.get_klass()->is_klass(),    "should be klass");
       }
     } else if (tag.is_symbol()) {
@@ -2112,13 +2110,11 @@ void ConstantPool::verify_on(outputStream* st) {
   if (cache() != NULL) {
     // Note: cache() can be NULL before a class is completely setup or
     // in temporary constant pools used during constant pool merging
-    guarantee(cache()->is_metadata(),          "should be metadata");
     guarantee(cache()->is_constantPoolCache(), "should be constant pool cache");
   }
   if (pool_holder() != NULL) {
     // Note: pool_holder() can be NULL in temporary constant pools
     // used during constant pool merging
-    guarantee(pool_holder()->is_metadata(), "should be metadata");
     guarantee(pool_holder()->is_klass(),    "should be klass");
   }
 }

src/share/vm/oops/instanceKlass.cpp

@@ -3088,27 +3088,26 @@ class VerifyFieldClosure: public OopClosure {
   virtual void do_oop(narrowOop* p) { VerifyFieldClosure::do_oop_work(p); }
 };
 
-void InstanceKlass::verify_on(outputStream* st) {
-  Klass::verify_on(st);
-  Thread *thread = Thread::current();
-
+void InstanceKlass::verify_on(outputStream* st, bool check_dictionary) {
 #ifndef PRODUCT
-  // Avoid redundant verifies
+  // Avoid redundant verifies, this really should be in product.
   if (_verify_count == Universe::verify_count()) return;
   _verify_count = Universe::verify_count();
 #endif
-  // Verify that klass is present in SystemDictionary
-  if (is_loaded() && !is_anonymous()) {
+
+  // Verify Klass
+  Klass::verify_on(st, check_dictionary);
+
+  // Verify that klass is present in SystemDictionary if not already
+  // verifying the SystemDictionary.
+  if (is_loaded() && !is_anonymous() && check_dictionary) {
     Symbol* h_name = name();
     SystemDictionary::verify_obj_klass_present(h_name, class_loader_data());
   }
 
-  // Verify static fields
-  VerifyFieldClosure blk;
-
   // Verify vtables
   if (is_linked()) {
-    ResourceMark rm(thread);
+    ResourceMark rm;
     // $$$ This used to be done only for m/s collections.  Doing it
     // always seemed a valid generalization.  (DLD -- 6/00)
     vtable()->verify(st);
@@ -3116,7 +3115,6 @@ void InstanceKlass::verify_on(outputStream* st) {
 
   // Verify first subklass
   if (subklass_oop() != NULL) {
-    guarantee(subklass_oop()->is_metadata(), "should be in metaspace");
     guarantee(subklass_oop()->is_klass(), "should be klass");
   }
 
@@ -3128,7 +3126,6 @@ void InstanceKlass::verify_on(outputStream* st) {
       fatal(err_msg("subclass points to itself " PTR_FORMAT, sib));
     }
 
-    guarantee(sib->is_metadata(), "should be in metaspace");
     guarantee(sib->is_klass(), "should be klass");
     guarantee(sib->super() == super, "siblings should have same superklass");
   }
@@ -3164,7 +3161,6 @@ void InstanceKlass::verify_on(outputStream* st) {
   if (methods() != NULL) {
     Array<Method*>* methods = this->methods();
     for (int j = 0; j < methods->length(); j++) {
-      guarantee(methods->at(j)->is_metadata(), "should be in metaspace");
       guarantee(methods->at(j)->is_method(), "non-method in methods array");
     }
     for (int j = 0; j < methods->length() - 1; j++) {
@@ -3202,16 +3198,13 @@ void InstanceKlass::verify_on(outputStream* st) {
 
   // Verify other fields
   if (array_klasses() != NULL) {
-    guarantee(array_klasses()->is_metadata(), "should be in metaspace");
     guarantee(array_klasses()->is_klass(), "should be klass");
   }
   if (constants() != NULL) {
-    guarantee(constants()->is_metadata(), "should be in metaspace");
     guarantee(constants()->is_constantPool(), "should be constant pool");
   }
   const Klass* host = host_klass();
   if (host != NULL) {
-    guarantee(host->is_metadata(), "should be in metaspace");
     guarantee(host->is_klass(), "should be klass");
   }
 }

src/share/vm/oops/instanceKlass.hpp

@@ -1050,7 +1050,7 @@ public:
   const char* internal_name() const;
 
   // Verification
-  void verify_on(outputStream* st);
+  void verify_on(outputStream* st, bool check_dictionary);
 
   void oop_verify_on(oop obj, outputStream* st);
 };

src/share/vm/oops/klass.cpp

@@ -377,7 +377,6 @@ void Klass::append_to_sibling_list() {
 }
 
 bool Klass::is_loader_alive(BoolObjectClosure* is_alive) {
-  assert(is_metadata(), "p is not meta-data");
   assert(ClassLoaderDataGraph::contains((address)this), "is in the metaspace");
 
 #ifdef ASSERT
@@ -648,27 +647,24 @@ void Klass::collect_statistics(KlassSizeStats *sz) const {
 
 // Verification
 
-void Klass::verify_on(outputStream* st) {
-  guarantee(!Universe::heap()->is_in_reserved(this), "Shouldn't be");
-  guarantee(this->is_metadata(), "should be in metaspace");
+void Klass::verify_on(outputStream* st, bool check_dictionary) {
 
+  // This can be expensive, but it is worth checking that this klass is actually
+  // in the CLD graph but not in production.
   assert(ClassLoaderDataGraph::contains((address)this), "Should be");
 
   guarantee(this->is_klass(),"should be klass");
 
   if (super() != NULL) {
-    guarantee(super()->is_metadata(), "should be in metaspace");
     guarantee(super()->is_klass(), "should be klass");
   }
   if (secondary_super_cache() != NULL) {
     Klass* ko = secondary_super_cache();
-    guarantee(ko->is_metadata(), "should be in metaspace");
     guarantee(ko->is_klass(), "should be klass");
   }
   for ( uint i = 0; i < primary_super_limit(); i++ ) {
     Klass* ko = _primary_supers[i];
     if (ko != NULL) {
-      guarantee(ko->is_metadata(), "should be in metaspace");
       guarantee(ko->is_klass(), "should be klass");
     }
   }
@@ -680,7 +676,6 @@ void Klass::verify_on(outputStream* st) {
 
 void Klass::oop_verify_on(oop obj, outputStream* st) {
   guarantee(obj->is_oop(),  "should be oop");
-  guarantee(obj->klass()->is_metadata(), "should not be in Java heap");
   guarantee(obj->klass()->is_klass(), "klass field is not a klass");
 }
 

src/share/vm/oops/klass.hpp

@@ -703,8 +703,8 @@ class Klass : public Metadata {
   virtual const char* internal_name() const = 0;
 
   // Verification
-  virtual void verify_on(outputStream* st);
-  void verify() { verify_on(tty); }
+  virtual void verify_on(outputStream* st, bool check_dictionary);
+  void verify(bool check_dictionary = true) { verify_on(tty, check_dictionary); }
 
 #ifndef PRODUCT
   void verify_vtable_index(int index);

src/share/vm/oops/method.cpp

@@ -1969,14 +1969,9 @@ void Method::collect_statistics(KlassSizeStats *sz) const {
 
 void Method::verify_on(outputStream* st) {
   guarantee(is_method(), "object must be method");
-  guarantee(is_metadata(),  "should be metadata");
   guarantee(constants()->is_constantPool(), "should be constant pool");
-  guarantee(constants()->is_metadata(), "should be metadata");
   guarantee(constMethod()->is_constMethod(), "should be ConstMethod*");
-  guarantee(constMethod()->is_metadata(), "should be metadata");
   MethodData* md = method_data();
-  guarantee(md == NULL ||
-      md->is_metadata(), "should be metadata");
   guarantee(md == NULL ||
       md->is_methodData(), "should be method data");
 }

src/share/vm/oops/objArrayKlass.cpp

@@ -676,11 +676,9 @@ const char* ObjArrayKlass::internal_name() const {
 
 // Verification
 
-void ObjArrayKlass::verify_on(outputStream* st) {
-  ArrayKlass::verify_on(st);
-  guarantee(element_klass()->is_metadata(), "should be in metaspace");
+void ObjArrayKlass::verify_on(outputStream* st, bool check_dictionary) {
+  ArrayKlass::verify_on(st, check_dictionary);
   guarantee(element_klass()->is_klass(), "should be klass");
-  guarantee(bottom_klass()->is_metadata(), "should be in metaspace");
   guarantee(bottom_klass()->is_klass(), "should be klass");
   Klass* bk = bottom_klass();
   guarantee(bk->oop_is_instance() || bk->oop_is_typeArray(),  "invalid bottom klass");

src/share/vm/oops/objArrayKlass.hpp

@@ -151,7 +151,7 @@ class ObjArrayKlass : public ArrayKlass {
   const char* internal_name() const;
 
   // Verification
-  void verify_on(outputStream* st);
+  void verify_on(outputStream* st, bool check_dictionary);
 
   void oop_verify_on(oop obj, outputStream* st);
 };

src/share/vm/runtime/frame.cpp

@@ -387,7 +387,6 @@ void frame::interpreter_frame_set_locals(intptr_t* locs)  {
 Method* frame::interpreter_frame_method() const {
   assert(is_interpreted_frame(), "interpreted frame expected");
   Method* m = *interpreter_frame_method_addr();
-  assert(m->is_metadata(), "bad Method* in interpreter frame");
   assert(m->is_method(), "not a Method*");
   return m;
 }

src/share/vm/shark/sharkBuilder.cpp

@@ -471,7 +471,7 @@ Value* SharkBuilder::CreateInlineOop(jobject object, const char* name) {
 
 Value* SharkBuilder::CreateInlineMetadata(Metadata* metadata, llvm::PointerType* type, const char* name) {
   assert(metadata != NULL, "inlined metadata must not be NULL");
-  assert(metadata->is_metadata(), "sanity check");
+  assert(metadata->is_metaspace_object(), "sanity check");
   return CreateLoad(
     CreateIntToPtr(
       code_buffer_address(code_buffer()->inline_Metadata(metadata)),