# HG changeset patch # User drwhite # Date 1467053030 14400 # Mon Jun 27 14:43:50 2016 -0400 # Node ID dfd44e705f41492ce1a42b23ab2d14ec6982abad # Parent ba08710f3b6cc85e1a16f2a1d65d74bbefd7d9c5 imported patch webrev.01 diff --git a/src/share/vm/classfile/javaClasses.cpp b/src/share/vm/classfile/javaClasses.cpp --- a/src/share/vm/classfile/javaClasses.cpp +++ b/src/share/vm/classfile/javaClasses.cpp @@ -871,12 +871,16 @@ int java_lang_Class::oop_size(oop java_class) { assert(_oop_size_offset != 0, "must be set"); - return java_class->int_field(_oop_size_offset); + int size = java_class->int_field(_oop_size_offset); + assert(size > 0, "size not set or stomped on: %d", size); + return size; } void java_lang_Class::set_oop_size(oop java_class, int size) { assert(_oop_size_offset != 0, "must be set"); + assert(size > 0, "total object size must be positive: %d", size); java_class->int_field_put(_oop_size_offset, size); } + int java_lang_Class::static_oop_field_count(oop java_class) { assert(_static_oop_field_count_offset != 0, "must be set"); return java_class->int_field(_static_oop_field_count_offset); diff --git a/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp b/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp --- a/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp +++ b/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp @@ -7365,9 +7365,16 @@ if (oop(addr)->klass_or_null() != NULL) { // Ignore mark word because we are running concurrent with mutators assert(oop(addr)->is_oop(true), "live block should be an oop"); - assert(size == + Klass* k = oop(addr)->klass(); + + // It's not always safe to ask for the size of a j.l.Class object, + // if the Class's oop_size hasn't been set yet. + if (!k->is_instance_klass() || + !InstanceKlass::cast(k)->is_mirror_instance_klass()) { + assert(size == CompactibleFreeListSpace::adjustObjectSize(oop(addr)->size()), "P-mark and computed size do not agree"); + } } #endif diff --git a/src/share/vm/oops/oop.inline.hpp b/src/share/vm/oops/oop.inline.hpp --- a/src/share/vm/oops/oop.inline.hpp +++ b/src/share/vm/oops/oop.inline.hpp @@ -258,8 +258,8 @@ } } - assert(s % MinObjAlignment == 0, "alignment check"); - assert(s > 0, "Bad size calculated"); + assert(s % MinObjAlignment == 0, "alignment check of %d", s); + assert(s > 0, "Bad size calculated: %d", s); return s; } # HG changeset patch # User drwhite # Date 1467053030 14400 # Mon Jun 27 14:43:50 2016 -0400 # Node ID 60a7c8bd5b78bc435819efb891900f1612ffa5e5 # Parent dfd44e705f41492ce1a42b23ab2d14ec6982abad imported patch webrev.02 diff --git a/src/share/vm/classfile/javaClasses.cpp b/src/share/vm/classfile/javaClasses.cpp --- a/src/share/vm/classfile/javaClasses.cpp +++ b/src/share/vm/classfile/javaClasses.cpp @@ -869,17 +869,13 @@ java_lang_Class::set_module(k->java_mirror(), module()); } +// Note, oop_size is set in CollectedHeap::class_allocate() int java_lang_Class::oop_size(oop java_class) { assert(_oop_size_offset != 0, "must be set"); int size = java_class->int_field(_oop_size_offset); assert(size > 0, "size not set or stomped on: %d", size); return size; } -void java_lang_Class::set_oop_size(oop java_class, int size) { - assert(_oop_size_offset != 0, "must be set"); - assert(size > 0, "total object size must be positive: %d", size); - java_class->int_field_put(_oop_size_offset, size); -} int java_lang_Class::static_oop_field_count(oop java_class) { assert(_static_oop_field_count_offset != 0, "must be set"); diff --git a/src/share/vm/classfile/javaClasses.hpp b/src/share/vm/classfile/javaClasses.hpp --- a/src/share/vm/classfile/javaClasses.hpp +++ b/src/share/vm/classfile/javaClasses.hpp @@ -271,11 +271,10 @@ static oop module(oop java_class); static int oop_size(oop java_class); - static void set_oop_size(oop java_class, int size); + static int oop_size_offset() { return _oop_size_offset; } static int static_oop_field_count(oop java_class); static void set_static_oop_field_count(oop java_class, int size); - static GrowableArray* fixup_mirror_list() { return _fixup_mirror_list; } diff --git a/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp b/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp --- a/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp +++ b/src/share/vm/gc/cms/concurrentMarkSweepGeneration.cpp @@ -7365,16 +7365,9 @@ if (oop(addr)->klass_or_null() != NULL) { // Ignore mark word because we are running concurrent with mutators assert(oop(addr)->is_oop(true), "live block should be an oop"); - Klass* k = oop(addr)->klass(); - - // It's not always safe to ask for the size of a j.l.Class object, - // if the Class's oop_size hasn't been set yet. - if (!k->is_instance_klass() || - !InstanceKlass::cast(k)->is_mirror_instance_klass()) { - assert(size == + assert(size == CompactibleFreeListSpace::adjustObjectSize(oop(addr)->size()), "P-mark and computed size do not agree"); - } } #endif diff --git a/src/share/vm/gc/shared/collectedHeap.hpp b/src/share/vm/gc/shared/collectedHeap.hpp --- a/src/share/vm/gc/shared/collectedHeap.hpp +++ b/src/share/vm/gc/shared/collectedHeap.hpp @@ -159,6 +159,8 @@ inline static void post_allocation_setup_array(KlassHandle klass, HeapWord* obj, int length); + inline static void post_allocation_setup_class(KlassHandle klass, HeapWord* obj, int size, int size_field_offset); + // Clears an allocated object. inline static void init_obj(HeapWord* obj, size_t size); @@ -300,6 +302,7 @@ inline static oop obj_allocate(KlassHandle klass, int size, TRAPS); inline static oop array_allocate(KlassHandle klass, int size, int length, TRAPS); inline static oop array_allocate_nozero(KlassHandle klass, int size, int length, TRAPS); + inline static oop class_allocate(KlassHandle klass, int size, int size_field_offset, TRAPS); inline static void post_allocation_install_obj_klass(KlassHandle klass, oop obj); diff --git a/src/share/vm/gc/shared/collectedHeap.inline.hpp b/src/share/vm/gc/shared/collectedHeap.inline.hpp --- a/src/share/vm/gc/shared/collectedHeap.inline.hpp +++ b/src/share/vm/gc/shared/collectedHeap.inline.hpp @@ -96,6 +96,22 @@ post_allocation_notify(klass, (oop)obj, size); } +void CollectedHeap::post_allocation_setup_class(KlassHandle klass, + HeapWord* obj, + int size, + int size_field_offset) { + post_allocation_setup_no_klass_install(klass, obj); + + // set the j.l.Class instance's oop_size field BEFORE setting the header: + ((oop)obj)->int_field_put(size_field_offset, size); + + post_allocation_install_obj_klass(klass, oop(obj)); // set the header + assert(Universe::is_bootstrapping() || + !((oop)obj)->is_array(), "must not be an array"); + // notify jvmti and dtrace + post_allocation_notify(klass, (oop)obj, size); +} + void CollectedHeap::post_allocation_setup_array(KlassHandle klass, HeapWord* obj, int length) { @@ -207,6 +223,18 @@ return (oop)obj; } +// Instances of j.l.Class have an oop_size field that must be set before the +// the header is set in order to parse the instances's size correctly. +oop CollectedHeap::class_allocate(KlassHandle klass, int size, int size_field_offset, TRAPS) { + debug_only(check_for_valid_allocation_state()); + assert(!Universe::heap()->is_gc_active(), "Allocation during gc not allowed"); + assert(size >= 0, "int won't convert to size_t"); + HeapWord* obj = common_mem_allocate_init(klass, size, CHECK_NULL); + post_allocation_setup_class(klass, obj, size, size_field_offset); // set oop_size + NOT_PRODUCT(Universe::heap()->check_for_bad_heap_word_value(obj, size)); + return (oop)obj; +} + oop CollectedHeap::array_allocate(KlassHandle klass, int size, int length, diff --git a/src/share/vm/oops/instanceMirrorKlass.cpp b/src/share/vm/oops/instanceMirrorKlass.cpp --- a/src/share/vm/oops/instanceMirrorKlass.cpp +++ b/src/share/vm/oops/instanceMirrorKlass.cpp @@ -50,12 +50,13 @@ // Query before forming handle. int size = instance_size(k); KlassHandle h_k(THREAD, this); - instanceOop i = (instanceOop)CollectedHeap::obj_allocate(h_k, size, CHECK_NULL); - - // Since mirrors can be variable sized because of the static fields, store - // the size in the mirror itself. - java_lang_Class::set_oop_size(i, size); - + int field_offset = java_lang_Class::oop_size_offset(); + + assert(field_offset != 0, "must be set"); + assert(size > 0, "total object size must be positive: %d", size); + + instanceOop i = (instanceOop)CollectedHeap::class_allocate(h_k, size, field_offset, CHECK_NULL); + // oop_size is set by class_allocate() before the header is set. return i; } # HG changeset patch # User drwhite # Date 1467128510 14400 # Tue Jun 28 11:41:50 2016 -0400 # Node ID 816b338be76f3ae63c649639793e7cd41a5bbc30 # Parent 60a7c8bd5b78bc435819efb891900f1612ffa5e5 [mq]: webrev.03 diff --git a/src/share/vm/gc/shared/collectedHeap.inline.hpp b/src/share/vm/gc/shared/collectedHeap.inline.hpp --- a/src/share/vm/gc/shared/collectedHeap.inline.hpp +++ b/src/share/vm/gc/shared/collectedHeap.inline.hpp @@ -100,16 +100,18 @@ HeapWord* obj, int size, int size_field_offset) { - post_allocation_setup_no_klass_install(klass, obj); - - // set the j.l.Class instance's oop_size field BEFORE setting the header: - ((oop)obj)->int_field_put(size_field_offset, size); - - post_allocation_install_obj_klass(klass, oop(obj)); // set the header + // Set oop_size field before setting the _klass field + // in post_allocation_setup_common() because the klass field + // indicates that the object is parsable by concurrent GC. + oop new_obj = (oop)obj; + assert(size > 0, "oop_size must be positive."); + assert(size_field_offset > 0, "size_field_offset must be initialized."); + new_obj->int_field_put(size_field_offset, size); + post_allocation_setup_common(klass, obj); assert(Universe::is_bootstrapping() || - !((oop)obj)->is_array(), "must not be an array"); + !new_obj->is_array(), "must not be an array"); // notify jvmti and dtrace - post_allocation_notify(klass, (oop)obj, size); + post_allocation_notify(klass, new_obj, size); } void CollectedHeap::post_allocation_setup_array(KlassHandle klass, diff --git a/src/share/vm/oops/instanceMirrorKlass.cpp b/src/share/vm/oops/instanceMirrorKlass.cpp --- a/src/share/vm/oops/instanceMirrorKlass.cpp +++ b/src/share/vm/oops/instanceMirrorKlass.cpp @@ -51,13 +51,11 @@ int size = instance_size(k); KlassHandle h_k(THREAD, this); int field_offset = java_lang_Class::oop_size_offset(); - + assert(field_offset != 0, "must be set"); assert(size > 0, "total object size must be positive: %d", size); - - instanceOop i = (instanceOop)CollectedHeap::class_allocate(h_k, size, field_offset, CHECK_NULL); - // oop_size is set by class_allocate() before the header is set. - return i; + + return (instanceOop)CollectedHeap::class_allocate(h_k, size, field_offset, CHECK_NULL); } int InstanceMirrorKlass::oop_size(oop obj) const { diff --git a/src/share/vm/oops/oop.inline.hpp b/src/share/vm/oops/oop.inline.hpp --- a/src/share/vm/oops/oop.inline.hpp +++ b/src/share/vm/oops/oop.inline.hpp @@ -258,8 +258,8 @@ } } - assert(s % MinObjAlignment == 0, "alignment check of %d", s); - assert(s > 0, "Bad size calculated: %d", s); + assert(s % MinObjAlignment == 0, "Oop size is not properly aligned: %d", s); + assert(s > 0, "Oop size must be greater than zero, not %d", s); return s; }