8016325: JVM hangs verifying system dictionary
Summary: Minimize redundant verifications of Klasses.
Reviewed-by: hseigel, jmasa
--- a/hotspot/src/cpu/sparc/vm/frame_sparc.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/cpu/sparc/vm/frame_sparc.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -680,7 +680,7 @@
// 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
--- a/hotspot/src/cpu/x86/vm/frame_x86.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/cpu/x86/vm/frame_x86.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -587,7 +587,7 @@
// 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
--- a/hotspot/src/share/vm/ci/ciObjectFactory.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/ci/ciObjectFactory.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -265,8 +265,6 @@
ciMetadata* ciObjectFactory::get_metadata(Metadata* key) {
ASSERT_IN_VM;
- assert(key == NULL || key->is_metadata(), "must be");
-
#ifdef ASSERT
if (CIObjectFactoryVerify) {
Metadata* last = NULL;
--- a/hotspot/src/share/vm/classfile/dictionary.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/classfile/dictionary.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -555,7 +555,7 @@
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++;
}
--- a/hotspot/src/share/vm/code/debugInfo.hpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/code/debugInfo.hpp Mon Jun 24 18:55:46 2013 -0400
@@ -274,7 +274,7 @@
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();
--- a/hotspot/src/share/vm/code/dependencies.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/code/dependencies.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -655,8 +655,8 @@
} 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;
}
--- a/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/gc_implementation/g1/heapRegion.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -798,7 +798,7 @@
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;
--- a/hotspot/src/share/vm/memory/allocation.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/memory/allocation.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -71,13 +71,6 @@
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);
--- a/hotspot/src/share/vm/memory/allocation.hpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/memory/allocation.hpp Mon Jun 24 18:55:46 2013 -0400
@@ -264,7 +264,6 @@
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
--- a/hotspot/src/share/vm/memory/heapInspection.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/memory/heapInspection.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -157,7 +157,6 @@
}
uint KlassInfoTable::hash(const Klass* p) {
- assert(p->is_metadata(), "all klasses are metadata");
return (uint)(((uintptr_t)p - (uintptr_t)_ref) >> 2);
}
--- a/hotspot/src/share/vm/oops/arrayKlass.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/arrayKlass.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -221,8 +221,8 @@
// 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");
--- a/hotspot/src/share/vm/oops/arrayKlass.hpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/arrayKlass.hpp Mon Jun 24 18:55:46 2013 -0400
@@ -152,7 +152,7 @@
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);
};
--- a/hotspot/src/share/vm/oops/compiledICHolder.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/compiledICHolder.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -48,8 +48,6 @@
// 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");
}
--- a/hotspot/src/share/vm/oops/constMethod.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/constMethod.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -440,7 +440,6 @@
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.
--- a/hotspot/src/share/vm/oops/constantPool.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/constantPool.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -2095,12 +2095,10 @@
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 @@
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");
}
}
--- a/hotspot/src/share/vm/oops/instanceKlass.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/instanceKlass.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -3088,27 +3088,26 @@
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 @@
// 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 @@
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 @@
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 @@
// 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");
}
}
--- a/hotspot/src/share/vm/oops/instanceKlass.hpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/instanceKlass.hpp Mon Jun 24 18:55:46 2013 -0400
@@ -1050,7 +1050,7 @@
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);
};
--- a/hotspot/src/share/vm/oops/klass.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/klass.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -377,7 +377,6 @@
}
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 @@
// 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::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");
}
--- a/hotspot/src/share/vm/oops/klass.hpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/klass.hpp Mon Jun 24 18:55:46 2013 -0400
@@ -703,8 +703,8 @@
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);
--- a/hotspot/src/share/vm/oops/method.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/method.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -1969,14 +1969,9 @@
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");
}
--- a/hotspot/src/share/vm/oops/objArrayKlass.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/objArrayKlass.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -676,11 +676,9 @@
// 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");
--- a/hotspot/src/share/vm/oops/objArrayKlass.hpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/oops/objArrayKlass.hpp Mon Jun 24 18:55:46 2013 -0400
@@ -151,7 +151,7 @@
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);
};
--- a/hotspot/src/share/vm/runtime/frame.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/runtime/frame.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -387,7 +387,6 @@
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;
}
--- a/hotspot/src/share/vm/shark/sharkBuilder.cpp Sun Jun 23 22:08:28 2013 -0700
+++ b/hotspot/src/share/vm/shark/sharkBuilder.cpp Mon Jun 24 18:55:46 2013 -0400
@@ -471,7 +471,7 @@
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)),