Mercurial > hg > openjdk > hsx16
changeset 932:5a53320cd23f
6861660: OopMapBlock count/size confusion
Reviewed-by: tonyp, iveresov
| author | jcoomes |
|---|---|
| date | Tue, 11 Aug 2009 15:37:23 -0700 |
| parents | 13520bae93a6 |
| children | e13afc88afa5 |
| files | src/share/vm/classfile/classFileParser.cpp src/share/vm/classfile/classFileParser.hpp src/share/vm/memory/oopFactory.cpp src/share/vm/memory/oopFactory.hpp src/share/vm/oops/instanceKlass.cpp src/share/vm/oops/instanceKlass.hpp src/share/vm/oops/instanceKlassKlass.cpp src/share/vm/oops/instanceKlassKlass.hpp src/share/vm/oops/instanceRefKlass.cpp |
| diffstat | 9 files changed, 112 insertions(+), 84 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/vm/classfile/classFileParser.cpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/classfile/classFileParser.cpp Tue Aug 11 15:37:23 2009 -0700 @@ -2924,12 +2924,12 @@ // Prepare list of oops for oop maps generation. u2* nonstatic_oop_offsets; - u2* nonstatic_oop_length; + u2* nonstatic_oop_counts; int nonstatic_oop_map_count = 0; nonstatic_oop_offsets = NEW_RESOURCE_ARRAY_IN_THREAD( THREAD, u2, nonstatic_oop_count+1); - nonstatic_oop_length = NEW_RESOURCE_ARRAY_IN_THREAD( + nonstatic_oop_counts = NEW_RESOURCE_ARRAY_IN_THREAD( THREAD, u2, nonstatic_oop_count+1); // Add fake fields for java.lang.Class instances (also see above). @@ -2939,7 +2939,7 @@ nonstatic_oop_offsets[0] = (u2)first_nonstatic_field_offset; int fake_oop_count = (( next_nonstatic_field_offset - first_nonstatic_field_offset ) / heapOopSize); - nonstatic_oop_length [0] = (u2)fake_oop_count; + nonstatic_oop_counts [0] = (u2)fake_oop_count; nonstatic_oop_map_count = 1; nonstatic_oop_count -= fake_oop_count; first_nonstatic_oop_offset = first_nonstatic_field_offset; @@ -3119,13 +3119,13 @@ // Update oop maps if( nonstatic_oop_map_count > 0 && nonstatic_oop_offsets[nonstatic_oop_map_count - 1] == - (u2)(real_offset - nonstatic_oop_length[nonstatic_oop_map_count - 1] * heapOopSize) ) { + (u2)(real_offset - nonstatic_oop_counts[nonstatic_oop_map_count - 1] * heapOopSize) ) { // Extend current oop map - nonstatic_oop_length[nonstatic_oop_map_count - 1] += 1; + nonstatic_oop_counts[nonstatic_oop_map_count - 1] += 1; } else { // Create new oop map nonstatic_oop_offsets[nonstatic_oop_map_count] = (u2)real_offset; - nonstatic_oop_length [nonstatic_oop_map_count] = 1; + nonstatic_oop_counts [nonstatic_oop_map_count] = 1; nonstatic_oop_map_count += 1; if( first_nonstatic_oop_offset == 0 ) { // Undefined first_nonstatic_oop_offset = real_offset; @@ -3182,8 +3182,10 @@ assert(instance_size == align_object_size(align_size_up((instanceOopDesc::base_offset_in_bytes() + nonstatic_field_size*heapOopSize), wordSize) / wordSize), "consistent layout helper value"); - // Size of non-static oop map blocks (in words) allocated at end of klass - int nonstatic_oop_map_size = compute_oop_map_size(super_klass, nonstatic_oop_map_count, first_nonstatic_oop_offset); + // Number of non-static oop map blocks allocated at end of klass. + int total_oop_map_count = compute_oop_map_count(super_klass, + nonstatic_oop_map_count, + first_nonstatic_oop_offset); // Compute reference type ReferenceType rt; @@ -3196,12 +3198,13 @@ // We can now create the basic klassOop for this klass klassOop ik = oopFactory::new_instanceKlass( vtable_size, itable_size, - static_field_size, nonstatic_oop_map_size, + static_field_size, total_oop_map_count, rt, CHECK_(nullHandle)); instanceKlassHandle this_klass (THREAD, ik); - assert(this_klass->static_field_size() == static_field_size && - this_klass->nonstatic_oop_map_size() == nonstatic_oop_map_size, "sanity check"); + assert(this_klass->static_field_size() == static_field_size, "sanity"); + assert(this_klass->nonstatic_oop_map_count() == total_oop_map_count, + "sanity"); // Fill in information already parsed this_klass->set_access_flags(access_flags); @@ -3282,7 +3285,7 @@ klassItable::setup_itable_offset_table(this_klass); // Do final class setup - fill_oop_maps(this_klass, nonstatic_oop_map_count, nonstatic_oop_offsets, nonstatic_oop_length); + fill_oop_maps(this_klass, nonstatic_oop_map_count, nonstatic_oop_offsets, nonstatic_oop_counts); set_precomputed_flags(this_klass); @@ -3375,66 +3378,71 @@ } -int ClassFileParser::compute_oop_map_size(instanceKlassHandle super, int nonstatic_oop_map_count, int first_nonstatic_oop_offset) { - int map_size = super.is_null() ? 0 : super->nonstatic_oop_map_size(); +int ClassFileParser::compute_oop_map_count(instanceKlassHandle super, + int nonstatic_oop_map_count, + int first_nonstatic_oop_offset) { + int map_count = super.is_null() ? 0 : super->nonstatic_oop_map_count(); if (nonstatic_oop_map_count > 0) { // We have oops to add to map - if (map_size == 0) { - map_size = nonstatic_oop_map_count; + if (map_count == 0) { + map_count = nonstatic_oop_map_count; } else { - // Check whether we should add a new map block or whether the last one can be extended - OopMapBlock* first_map = super->start_of_nonstatic_oop_maps(); - OopMapBlock* last_map = first_map + map_size - 1; - - int next_offset = last_map->offset() + (last_map->length() * heapOopSize); + // Check whether we should add a new map block or whether the last one can + // be extended + OopMapBlock* const first_map = super->start_of_nonstatic_oop_maps(); + OopMapBlock* const last_map = first_map + map_count - 1; + + int next_offset = last_map->offset() + last_map->count() * heapOopSize; if (next_offset == first_nonstatic_oop_offset) { // There is no gap bettwen superklass's last oop field and first // local oop field, merge maps. nonstatic_oop_map_count -= 1; } else { // Superklass didn't end with a oop field, add extra maps - assert(next_offset<first_nonstatic_oop_offset, "just checking"); + assert(next_offset < first_nonstatic_oop_offset, "just checking"); } - map_size += nonstatic_oop_map_count; + map_count += nonstatic_oop_map_count; } } - return map_size; + return map_count; } void ClassFileParser::fill_oop_maps(instanceKlassHandle k, - int nonstatic_oop_map_count, - u2* nonstatic_oop_offsets, u2* nonstatic_oop_length) { + int nonstatic_oop_map_count, + u2* nonstatic_oop_offsets, + u2* nonstatic_oop_counts) { OopMapBlock* this_oop_map = k->start_of_nonstatic_oop_maps(); - OopMapBlock* last_oop_map = this_oop_map + k->nonstatic_oop_map_size(); - instanceKlass* super = k->superklass(); - if (super != NULL) { - int super_oop_map_size = super->nonstatic_oop_map_size(); + const instanceKlass* const super = k->superklass(); + const int super_count = super != NULL ? super->nonstatic_oop_map_count() : 0; + if (super_count > 0) { + // Copy maps from superklass OopMapBlock* super_oop_map = super->start_of_nonstatic_oop_maps(); - // Copy maps from superklass - while (super_oop_map_size-- > 0) { + for (int i = 0; i < super_count; ++i) { *this_oop_map++ = *super_oop_map++; } } + if (nonstatic_oop_map_count > 0) { - if (this_oop_map + nonstatic_oop_map_count > last_oop_map) { - // Calculated in compute_oop_map_size() number of oop maps is less then - // collected oop maps since there is no gap between superklass's last oop - // field and first local oop field. Extend the last oop map copied + if (super_count + nonstatic_oop_map_count > k->nonstatic_oop_map_count()) { + // The counts differ because there is no gap between superklass's last oop + // field and the first local oop field. Extend the last oop map copied // from the superklass instead of creating new one. nonstatic_oop_map_count--; nonstatic_oop_offsets++; this_oop_map--; - this_oop_map->set_length(this_oop_map->length() + *nonstatic_oop_length++); + this_oop_map->set_count(this_oop_map->count() + *nonstatic_oop_counts++); this_oop_map++; } - assert((this_oop_map + nonstatic_oop_map_count) == last_oop_map, "just checking"); + // Add new map blocks, fill them while (nonstatic_oop_map_count-- > 0) { this_oop_map->set_offset(*nonstatic_oop_offsets++); - this_oop_map->set_length(*nonstatic_oop_length++); + this_oop_map->set_count(*nonstatic_oop_counts++); this_oop_map++; } + assert(k->start_of_nonstatic_oop_maps() + k->nonstatic_oop_map_count() == + this_oop_map, "sanity"); } }
--- a/src/share/vm/classfile/classFileParser.hpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/classfile/classFileParser.hpp Tue Aug 11 15:37:23 2009 -0700 @@ -125,10 +125,10 @@ int runtime_invisible_annotations_length, TRAPS); // Final setup - int compute_oop_map_size(instanceKlassHandle super, int nonstatic_oop_count, + int compute_oop_map_count(instanceKlassHandle super, int nonstatic_oop_count, int first_nonstatic_oop_offset); void fill_oop_maps(instanceKlassHandle k, int nonstatic_oop_map_count, - u2* nonstatic_oop_offsets, u2* nonstatic_oop_length); + u2* nonstatic_oop_offsets, u2* nonstatic_oop_counts); void set_precomputed_flags(instanceKlassHandle k); objArrayHandle compute_transitive_interfaces(instanceKlassHandle super, objArrayHandle local_ifs, TRAPS);
--- a/src/share/vm/memory/oopFactory.cpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/memory/oopFactory.cpp Tue Aug 11 15:37:23 2009 -0700 @@ -99,9 +99,9 @@ klassOop oopFactory::new_instanceKlass(int vtable_len, int itable_len, int static_field_size, - int nonstatic_oop_map_size, ReferenceType rt, TRAPS) { + int nonstatic_oop_map_count, ReferenceType rt, TRAPS) { instanceKlassKlass* ikk = instanceKlassKlass::cast(Universe::instanceKlassKlassObj()); - return ikk->allocate_instance_klass(vtable_len, itable_len, static_field_size, nonstatic_oop_map_size, rt, CHECK_NULL); + return ikk->allocate_instance_klass(vtable_len, itable_len, static_field_size, nonstatic_oop_map_count, rt, CHECK_NULL); }
--- a/src/share/vm/memory/oopFactory.hpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/memory/oopFactory.hpp Tue Aug 11 15:37:23 2009 -0700 @@ -90,7 +90,7 @@ // Instance classes static klassOop new_instanceKlass(int vtable_len, int itable_len, int static_field_size, - int nonstatic_oop_map_size, ReferenceType rt, TRAPS); + int nonstatic_oop_map_count, ReferenceType rt, TRAPS); // Methods private:
--- a/src/share/vm/oops/instanceKlass.cpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/oops/instanceKlass.cpp Tue Aug 11 15:37:23 2009 -0700 @@ -1397,18 +1397,18 @@ /* Compute oopmap block range. The common case \ is nonstatic_oop_map_size == 1. */ \ OopMapBlock* map = start_of_nonstatic_oop_maps(); \ - OopMapBlock* const end_map = map + nonstatic_oop_map_size(); \ + OopMapBlock* const end_map = map + nonstatic_oop_map_count(); \ if (UseCompressedOops) { \ while (map < end_map) { \ InstanceKlass_SPECIALIZED_OOP_ITERATE(narrowOop, \ - obj->obj_field_addr<narrowOop>(map->offset()), map->length(), \ + obj->obj_field_addr<narrowOop>(map->offset()), map->count(), \ do_oop, assert_fn) \ ++map; \ } \ } else { \ while (map < end_map) { \ InstanceKlass_SPECIALIZED_OOP_ITERATE(oop, \ - obj->obj_field_addr<oop>(map->offset()), map->length(), \ + obj->obj_field_addr<oop>(map->offset()), map->count(), \ do_oop, assert_fn) \ ++map; \ } \ @@ -1418,19 +1418,19 @@ #define InstanceKlass_OOP_MAP_REVERSE_ITERATE(obj, do_oop, assert_fn) \ { \ OopMapBlock* const start_map = start_of_nonstatic_oop_maps(); \ - OopMapBlock* map = start_map + nonstatic_oop_map_size(); \ + OopMapBlock* map = start_map + nonstatic_oop_map_count(); \ if (UseCompressedOops) { \ while (start_map < map) { \ --map; \ InstanceKlass_SPECIALIZED_OOP_REVERSE_ITERATE(narrowOop, \ - obj->obj_field_addr<narrowOop>(map->offset()), map->length(), \ + obj->obj_field_addr<narrowOop>(map->offset()), map->count(), \ do_oop, assert_fn) \ } \ } else { \ while (start_map < map) { \ --map; \ InstanceKlass_SPECIALIZED_OOP_REVERSE_ITERATE(oop, \ - obj->obj_field_addr<oop>(map->offset()), map->length(), \ + obj->obj_field_addr<oop>(map->offset()), map->count(), \ do_oop, assert_fn) \ } \ } \ @@ -1444,11 +1444,11 @@ usually non-existent extra overhead of examining \ all the maps. */ \ OopMapBlock* map = start_of_nonstatic_oop_maps(); \ - OopMapBlock* const end_map = map + nonstatic_oop_map_size(); \ + OopMapBlock* const end_map = map + nonstatic_oop_map_count(); \ if (UseCompressedOops) { \ while (map < end_map) { \ InstanceKlass_SPECIALIZED_BOUNDED_OOP_ITERATE(narrowOop, \ - obj->obj_field_addr<narrowOop>(map->offset()), map->length(), \ + obj->obj_field_addr<narrowOop>(map->offset()), map->count(), \ low, high, \ do_oop, assert_fn) \ ++map; \ @@ -1456,7 +1456,7 @@ } else { \ while (map < end_map) { \ InstanceKlass_SPECIALIZED_BOUNDED_OOP_ITERATE(oop, \ - obj->obj_field_addr<oop>(map->offset()), map->length(), \ + obj->obj_field_addr<oop>(map->offset()), map->count(), \ low, high, \ do_oop, assert_fn) \ ++map; \ @@ -2217,14 +2217,14 @@ first_time = false; const int extra = java_lang_Class::number_of_fake_oop_fields; guarantee(ik->nonstatic_field_size() == extra, "just checking"); - guarantee(ik->nonstatic_oop_map_size() == 1, "just checking"); + guarantee(ik->nonstatic_oop_map_count() == 1, "just checking"); guarantee(ik->size_helper() == align_object_size(instanceOopDesc::header_size() + extra), "just checking"); // Check that the map is (2,extra) int offset = java_lang_Class::klass_offset; OopMapBlock* map = ik->start_of_nonstatic_oop_maps(); - guarantee(map->offset() == offset && map->length() == extra, "just checking"); + guarantee(map->offset() == offset && map->count() == extra, "sanity"); } }
--- a/src/share/vm/oops/instanceKlass.hpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/oops/instanceKlass.hpp Tue Aug 11 15:37:23 2009 -0700 @@ -71,7 +71,6 @@ // forward declaration for class -- see below for definition class SuperTypeClosure; -class OopMapBlock; class JNIid; class jniIdMapBase; class BreakpointInfo; @@ -99,6 +98,29 @@ }; #endif // !PRODUCT +// ValueObjs embedded in klass. Describes where oops are located in instances of +// this klass. +class OopMapBlock VALUE_OBJ_CLASS_SPEC { + public: + // Byte offset of the first oop mapped by this block. + jushort offset() const { return _offset; } + void set_offset(jushort offset) { _offset = offset; } + + // Number of oops in this block. + jushort count() const { return _count; } + void set_count(jushort count) { _count = count; } + + // sizeof(OopMapBlock) in HeapWords. + static const int size_in_words() { + return align_size_up(int(sizeof(OopMapBlock)), HeapWordSize) >> + LogHeapWordSize; + } + + private: + jushort _offset; + jushort _count; +}; + class instanceKlass: public Klass { friend class VMStructs; public: @@ -191,7 +213,7 @@ int _nonstatic_field_size; int _static_field_size; // number words used by static fields (oop and non-oop) in this klass int _static_oop_field_size;// number of static oop fields in this klass - int _nonstatic_oop_map_size;// number of nonstatic oop-map blocks allocated at end of this klass + int _nonstatic_oop_map_size;// size in words of nonstatic oop map blocks bool _is_marked_dependent; // used for marking during flushing and deoptimization bool _rewritten; // methods rewritten. bool _has_nonstatic_fields; // for sizing with UseCompressedOops @@ -424,8 +446,16 @@ void set_source_debug_extension(symbolOop n){ oop_store_without_check((oop*) &_source_debug_extension, (oop) n); } // nonstatic oop-map blocks - int nonstatic_oop_map_size() const { return _nonstatic_oop_map_size; } - void set_nonstatic_oop_map_size(int size) { _nonstatic_oop_map_size = size; } + static int nonstatic_oop_map_size(int oop_map_count) { + return oop_map_count * OopMapBlock::size_in_words(); + } + int nonstatic_oop_map_count() const { + return _nonstatic_oop_map_size / OopMapBlock::size_in_words(); + } + int nonstatic_oop_map_size() const { return _nonstatic_oop_map_size; } + void set_nonstatic_oop_map_size(int words) { + _nonstatic_oop_map_size = words; + } // RedefineClasses() support for previous versions: void add_previous_version(instanceKlassHandle ikh, BitMap *emcp_methods, @@ -839,21 +869,6 @@ } -// ValueObjs embedded in klass. Describes where oops are located in instances of this klass. - -class OopMapBlock VALUE_OBJ_CLASS_SPEC { - private: - jushort _offset; // Offset of first oop in oop-map block - jushort _length; // Length of oop-map block - public: - // Accessors - jushort offset() const { return _offset; } - void set_offset(jushort offset) { _offset = offset; } - - jushort length() const { return _length; } - void set_length(jushort length) { _length = length; } -}; - /* JNIid class for jfieldIDs only */ class JNIid: public CHeapObj { friend class VMStructs;
--- a/src/share/vm/oops/instanceKlassKlass.cpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/oops/instanceKlassKlass.cpp Tue Aug 11 15:37:23 2009 -0700 @@ -402,9 +402,14 @@ } #endif // SERIALGC -klassOop instanceKlassKlass::allocate_instance_klass(int vtable_len, int itable_len, int static_field_size, - int nonstatic_oop_map_size, ReferenceType rt, TRAPS) { +klassOop +instanceKlassKlass::allocate_instance_klass(int vtable_len, int itable_len, + int static_field_size, + int nonstatic_oop_map_count, + ReferenceType rt, TRAPS) { + const int nonstatic_oop_map_size = + instanceKlass::nonstatic_oop_map_size(nonstatic_oop_map_count); int size = instanceKlass::object_size(align_object_offset(vtable_len) + align_object_offset(itable_len) + static_field_size + nonstatic_oop_map_size); // Allocation @@ -615,9 +620,9 @@ st->print(BULLET"non-static oop maps: "); OopMapBlock* map = ik->start_of_nonstatic_oop_maps(); - OopMapBlock* end_map = map + ik->nonstatic_oop_map_size(); + OopMapBlock* end_map = map + ik->nonstatic_oop_map_count(); while (map < end_map) { - st->print("%d-%d ", map->offset(), map->offset() + heapOopSize*(map->length() - 1)); + st->print("%d-%d ", map->offset(), map->offset() + heapOopSize*(map->count() - 1)); map++; } st->cr();
--- a/src/share/vm/oops/instanceKlassKlass.hpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/oops/instanceKlassKlass.hpp Tue Aug 11 15:37:23 2009 -0700 @@ -39,7 +39,7 @@ klassOop allocate_instance_klass(int vtable_len, int itable_len, int static_field_size, - int nonstatic_oop_map_size, + int nonstatic_oop_map_count, ReferenceType rt, TRAPS);
--- a/src/share/vm/oops/instanceRefKlass.cpp Thu Jul 30 16:22:58 2009 -0400 +++ b/src/share/vm/oops/instanceRefKlass.cpp Tue Aug 11 15:37:23 2009 -0700 @@ -400,26 +400,26 @@ assert(k == SystemDictionary::reference_klass() && first_time, "Invalid update of maps"); debug_only(first_time = false); - assert(ik->nonstatic_oop_map_size() == 1, "just checking"); + assert(ik->nonstatic_oop_map_count() == 1, "just checking"); OopMapBlock* map = ik->start_of_nonstatic_oop_maps(); // Check that the current map is (2,4) - currently points at field with // offset 2 (words) and has 4 map entries. debug_only(int offset = java_lang_ref_Reference::referent_offset); - debug_only(int length = ((java_lang_ref_Reference::discovered_offset - + debug_only(int count = ((java_lang_ref_Reference::discovered_offset - java_lang_ref_Reference::referent_offset)/heapOopSize) + 1); if (UseSharedSpaces) { assert(map->offset() == java_lang_ref_Reference::queue_offset && - map->length() == 1, "just checking"); + map->count() == 1, "just checking"); } else { - assert(map->offset() == offset && map->length() == length, + assert(map->offset() == offset && map->count() == count, "just checking"); // Update map to (3,1) - point to offset of 3 (words) with 1 map entry. map->set_offset(java_lang_ref_Reference::queue_offset); - map->set_length(1); + map->set_count(1); } }