6861660: OopMapBlock count/size confusion

Reviewed-by: tonyp, iveresov

src/share/vm/classfile/classFileParser.cpp

@@ -2924,12 +2924,12 @@ instanceKlassHandle ClassFileParser::parseClassFile(symbolHandle name,
 
     // 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 @@ instanceKlassHandle ClassFileParser::parseClassFile(symbolHandle name,
       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 @@ instanceKlassHandle ClassFileParser::parseClassFile(symbolHandle name,
           // 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 @@ instanceKlassHandle ClassFileParser::parseClassFile(symbolHandle name,
 
     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 @@ instanceKlassHandle ClassFileParser::parseClassFile(symbolHandle name,
     // 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 @@ instanceKlassHandle ClassFileParser::parseClassFile(symbolHandle name,
     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 @@ instanceKlassHandle ClassFileParser::parseClassFile(symbolHandle name,
 }
 
 
-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;
+      // 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->length() * heapOopSize);
+      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();
-    OopMapBlock* super_oop_map = super->start_of_nonstatic_oop_maps();
+  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
-    while (super_oop_map_size-- > 0) {
+    OopMapBlock* super_oop_map = super->start_of_nonstatic_oop_maps();
+    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");
   }
 }
 

src/share/vm/classfile/classFileParser.hpp

@@ -125,10 +125,10 @@ class ClassFileParser VALUE_OBJ_CLASS_SPEC {
                                        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);

src/share/vm/memory/oopFactory.cpp

@@ -99,9 +99,9 @@ constantPoolCacheOop oopFactory::new_constantPoolCache(int length,
 
 
 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);
 }
 
 

src/share/vm/memory/oopFactory.hpp

@@ -90,7 +90,7 @@ class oopFactory: AllStatic {
 
   // 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:

src/share/vm/oops/instanceKlass.cpp

@@ -1396,18 +1396,18 @@ template <class T> void assert_nothing(T *p) {}
   /* 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;                                                             \
     }                                                                    \
@@ -1417,19 +1417,19 @@ template <class T> void assert_nothing(T *p) {}
 #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)                                               \
     }                                                                    \
   }                                                                      \
@@ -1443,11 +1443,11 @@ template <class T> void assert_nothing(T *p) {}
      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;                                                             \
@@ -1455,7 +1455,7 @@ template <class T> void assert_nothing(T *p) {}
   } 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;                                                             \
@@ -2216,14 +2216,14 @@ void instanceKlass::verify_class_klass_nonstatic_oop_maps(klassOop k) {
     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");
   }
 }
 

src/share/vm/oops/instanceKlass.hpp

@@ -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 @@ class FieldPrinter: public FieldClosure {
 };
 #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 @@ class instanceKlass: public Klass {
   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 @@ class instanceKlass: public Klass {
   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 @@ inline u2 instanceKlass::next_method_idnum() {
 }
 
 
-// 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;

src/share/vm/oops/instanceKlassKlass.cpp

@@ -402,9 +402,14 @@ int instanceKlassKlass::oop_update_pointers(ParCompactionManager* cm, oop obj,
 }
 #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 @@ void instanceKlassKlass::oop_print_on(oop obj, outputStream* st) {
 
   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();

src/share/vm/oops/instanceKlassKlass.hpp

@@ -39,7 +39,7 @@ class instanceKlassKlass : public klassKlass {
   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);
 

src/share/vm/oops/instanceRefKlass.cpp

@@ -400,26 +400,26 @@ void instanceRefKlass::update_nonstatic_oop_maps(klassOop k) {
   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);
   }
 }