8352075: Perf regression accessing fields

Reviewed-by: coleenp, iklam, jsjolen

Author: Radim Vansa

src/hotspot/share/classfile/classFileParser.cpp

@@ -3740,6 +3740,7 @@ void ClassFileParser::apply_parsed_class_metadata(
   _cp->set_pool_holder(this_klass);
   this_klass->set_constants(_cp);
   this_klass->set_fieldinfo_stream(_fieldinfo_stream);
+  this_klass->set_fieldinfo_search_table(_fieldinfo_search_table);
   this_klass->set_fields_status(_fields_status);
   this_klass->set_methods(_methods);
   this_klass->set_inner_classes(_inner_classes);
@@ -3749,6 +3750,8 @@ void ClassFileParser::apply_parsed_class_metadata(
   this_klass->set_permitted_subclasses(_permitted_subclasses);
   this_klass->set_record_components(_record_components);
 
+  DEBUG_ONLY(FieldInfoStream::validate_search_table(_cp, _fieldinfo_stream, _fieldinfo_search_table));
+
   // Delay the setting of _local_interfaces and _transitive_interfaces until after
   // initialize_supers() in fill_instance_klass(). It is because the _local_interfaces could
   // be shared with _transitive_interfaces and _transitive_interfaces may be shared with
@@ -5056,6 +5059,7 @@ void ClassFileParser::fill_instance_klass(InstanceKlass* ik,
   // note that is not safe to use the fields in the parser from this point on
   assert(nullptr == _cp, "invariant");
   assert(nullptr == _fieldinfo_stream, "invariant");
+  assert(nullptr == _fieldinfo_search_table, "invariant");
   assert(nullptr == _fields_status, "invariant");
   assert(nullptr == _methods, "invariant");
   assert(nullptr == _inner_classes, "invariant");
@@ -5276,6 +5280,7 @@ ClassFileParser::ClassFileParser(ClassFileStream* stream,
   _super_klass(),
   _cp(nullptr),
   _fieldinfo_stream(nullptr),
+  _fieldinfo_search_table(nullptr),
   _fields_status(nullptr),
   _methods(nullptr),
   _inner_classes(nullptr),
@@ -5352,6 +5357,7 @@ void ClassFileParser::clear_class_metadata() {
   // deallocated if classfile parsing returns an error.
   _cp = nullptr;
   _fieldinfo_stream = nullptr;
+  _fieldinfo_search_table = nullptr;
   _fields_status = nullptr;
   _methods = nullptr;
   _inner_classes = nullptr;
@@ -5374,6 +5380,7 @@ ClassFileParser::~ClassFileParser() {
   if (_fieldinfo_stream != nullptr) {
     MetadataFactory::free_array<u1>(_loader_data, _fieldinfo_stream);
   }
+  MetadataFactory::free_array<u1>(_loader_data, _fieldinfo_search_table);
 
   if (_fields_status != nullptr) {
     MetadataFactory::free_array<FieldStatus>(_loader_data, _fields_status);
@@ -5774,6 +5781,7 @@ void ClassFileParser::post_process_parsed_stream(const ClassFileStream* const st
   _fieldinfo_stream =
     FieldInfoStream::create_FieldInfoStream(_temp_field_info, _java_fields_count,
                                             injected_fields_count, loader_data(), CHECK);
+  _fieldinfo_search_table = FieldInfoStream::create_search_table(_cp, _fieldinfo_stream, _loader_data, CHECK);
   _fields_status =
     MetadataFactory::new_array<FieldStatus>(_loader_data, _temp_field_info->length(),
                                             FieldStatus(0), CHECK);

src/hotspot/share/classfile/classFileParser.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -123,6 +123,7 @@ class ClassFileParser {
   const InstanceKlass* _super_klass;
   ConstantPool* _cp;
   Array<u1>* _fieldinfo_stream;
+  Array<u1>* _fieldinfo_search_table;
   Array<FieldStatus>* _fields_status;
   Array<Method*>* _methods;
   Array<u2>* _inner_classes;

src/hotspot/share/classfile/fieldLayoutBuilder.cpp

@@ -301,7 +301,7 @@ void FieldLayout::reconstruct_layout(const InstanceKlass* ik, bool& has_instance
   BasicType last_type;
   int last_offset = -1;
   while (ik != nullptr) {
-    for (AllFieldStream fs(ik->fieldinfo_stream(), ik->constants()); !fs.done(); fs.next()) {
+    for (AllFieldStream fs(ik); !fs.done(); fs.next()) {
       BasicType type = Signature::basic_type(fs.signature());
       // distinction between static and non-static fields is missing
       if (fs.access_flags().is_static()) continue;
@@ -461,7 +461,7 @@ void FieldLayout::print(outputStream* output, bool is_static, const InstanceKlas
         bool found = false;
         const InstanceKlass* ik = super;
         while (!found && ik != nullptr) {
-          for (AllFieldStream fs(ik->fieldinfo_stream(), ik->constants()); !fs.done(); fs.next()) {
+          for (AllFieldStream fs(ik); !fs.done(); fs.next()) {
             if (fs.offset() == b->offset()) {
               output->print_cr(" @%d \"%s\" %s %d/%d %s",
                   b->offset(),

src/hotspot/share/classfile/javaClasses.cpp

@@ -967,6 +967,13 @@ void java_lang_Class::fixup_mirror(Klass* k, TRAPS) {
       Array<u1>* new_fis = FieldInfoStream::create_FieldInfoStream(fields, java_fields, injected_fields, k->class_loader_data(), CHECK);
       ik->set_fieldinfo_stream(new_fis);
       MetadataFactory::free_array<u1>(k->class_loader_data(), old_stream);
+
+      Array<u1>* old_table = ik->fieldinfo_search_table();
+      Array<u1>* search_table = FieldInfoStream::create_search_table(ik->constants(), new_fis, k->class_loader_data(), CHECK);
+      ik->set_fieldinfo_search_table(search_table);
+      MetadataFactory::free_array<u1>(k->class_loader_data(), old_table);
+
+      DEBUG_ONLY(FieldInfoStream::validate_search_table(ik->constants(), new_fis, search_table));
     }
   }
 

src/hotspot/share/oops/fieldInfo.cpp

@@ -22,8 +22,11 @@
  *
  */
 
+#include "memory/resourceArea.hpp"
+#include "cds/cdsConfig.hpp"
 #include "oops/fieldInfo.inline.hpp"
 #include "runtime/atomic.hpp"
+#include "utilities/packedTable.hpp"
 
 void FieldInfo::print(outputStream* os, ConstantPool* cp) {
   os->print_cr("index=%d name_index=%d name=%s signature_index=%d signature=%s offset=%d "
@@ -37,8 +40,10 @@ void FieldInfo::print(outputStream* os, ConstantPool* cp) {
                 field_flags().as_uint(),
                 initializer_index(),
                 generic_signature_index(),
-                _field_flags.is_injected() ? lookup_symbol(generic_signature_index())->as_utf8() : cp->symbol_at(generic_signature_index())->as_utf8(),
-                contended_group());
+                _field_flags.is_generic() ? (_field_flags.is_injected() ?
+                  lookup_symbol(generic_signature_index())->as_utf8() : cp->symbol_at(generic_signature_index())->as_utf8()
+                  ) : "",
+                is_contended() ? contended_group() : 0);
 }
 
 void FieldInfo::print_from_growable_array(outputStream* os, GrowableArray<FieldInfo>* array, ConstantPool* cp) {
@@ -62,13 +67,17 @@ Array<u1>* FieldInfoStream::create_FieldInfoStream(GrowableArray<FieldInfo>* fie
   StreamSizer s;
   StreamFieldSizer sizer(&s);
 
+  assert(fields->length() == java_fields + injected_fields, "must be");
+
   sizer.consumer()->accept_uint(java_fields);
   sizer.consumer()->accept_uint(injected_fields);
   for (int i = 0; i < fields->length(); i++) {
     FieldInfo* fi = fields->adr_at(i);
     sizer.map_field_info(*fi);
   }
-  int storage_size = sizer.consumer()->position() + 1;
+  // Originally there was an extra byte with 0 terminating the reading;
+  // now we check limits instead.
+  int storage_size = sizer.consumer()->position();
   Array<u1>* const fis = MetadataFactory::new_array<u1>(loader_data, storage_size, CHECK_NULL);
 
   using StreamWriter = UNSIGNED5::Writer<Array<u1>*, int, ArrayHelper<Array<u1>*, int>>;
@@ -79,15 +88,14 @@ Array<u1>* FieldInfoStream::create_FieldInfoStream(GrowableArray<FieldInfo>* fie
   writer.consumer()->accept_uint(java_fields);
   writer.consumer()->accept_uint(injected_fields);
   for (int i = 0; i < fields->length(); i++) {
-    FieldInfo* fi = fields->adr_at(i);
-    writer.map_field_info(*fi);
+    writer.map_field_info(fields->at(i));
   }
 
 #ifdef ASSERT
   FieldInfoReader r(fis);
-  int jfc = r.next_uint();
+  int jfc, ifc;
+  r.read_field_counts(&jfc, &ifc);
   assert(jfc == java_fields, "Must be");
-  int ifc = r.next_uint();
   assert(ifc == injected_fields, "Must be");
   for (int i = 0; i < jfc + ifc; i++) {
     FieldInfo fi;
@@ -113,30 +121,221 @@ Array<u1>* FieldInfoStream::create_FieldInfoStream(GrowableArray<FieldInfo>* fie
   return fis;
 }
 
+int FieldInfoStream::compare_name_and_sig(const Symbol* n1, const Symbol* s1, const Symbol* n2, const Symbol* s2) {
+  int cmp = n1->fast_compare(n2);
+  return cmp != 0 ? cmp : s1->fast_compare(s2);
+}
+
+
+// We use both name and signature during the comparison; while JLS require unique
+// names for fields, JVMS requires only unique name + signature combination.
+struct field_pos {
+  Symbol* _name;
+  Symbol* _signature;
+  int _index;
+  int _position;
+};
+
+class FieldInfoSupplier: public PackedTableBuilder::Supplier {
+  const field_pos* _positions;
+  size_t _elements;
+
+public:
+  FieldInfoSupplier(const field_pos* positions, size_t elements): _positions(positions), _elements(elements) {}
+
+  bool next(uint32_t* key, uint32_t* value) override {
+    if (_elements == 0) {
+      return false;
+    }
+    *key = _positions->_position;
+    *value = _positions->_index;
+    ++_positions;
+    --_elements;
+    return true;
+  }
+};
+
+Array<u1>* FieldInfoStream::create_search_table(ConstantPool* cp, const Array<u1>* fis, ClassLoaderData* loader_data, TRAPS) {
+  if (CDSConfig::is_dumping_dynamic_archive()) {
+    // We cannot use search table; in case of dynamic archives it should be sorted by "requested" addresses,
+    // but Symbol* addresses are coming from _constants, which has "buffered" addresses.
+    // For background, see new comments inside allocate_node_impl in symbolTable.cpp
+    return nullptr;
+  }
+
+  FieldInfoReader r(fis);
+  int java_fields;
+  int injected_fields;
+  r.read_field_counts(&java_fields, &injected_fields);
+  assert(java_fields >= 0, "must be");
+  if (java_fields == 0 || fis->length() == 0 || static_cast<uint>(java_fields) < BinarySearchThreshold) {
+    return nullptr;
+  }
+
+  ResourceMark rm;
+  field_pos* positions = NEW_RESOURCE_ARRAY(field_pos, java_fields);
+  for (int i = 0; i < java_fields; ++i) {
+    assert(r.has_next(), "number of fields must match");
+
+    positions[i]._position = r.position();
+    FieldInfo fi;
+    r.read_field_info(fi);
+
+    positions[i]._name = fi.name(cp);
+    positions[i]._signature = fi.signature(cp);
+    positions[i]._index = i;
+  }
+  auto compare_pair = [](const void* v1, const void* v2) {
+    const field_pos* p1 = reinterpret_cast<const field_pos*>(v1);
+    const field_pos* p2 = reinterpret_cast<const field_pos*>(v2);
+    return compare_name_and_sig(p1->_name, p1->_signature, p2->_name, p2->_signature);
+  };
+  qsort(positions, java_fields, sizeof(field_pos), compare_pair);
+
+  PackedTableBuilder builder(fis->length() - 1, java_fields - 1);
+  Array<u1>* table = MetadataFactory::new_array<u1>(loader_data, java_fields * builder.element_bytes(), CHECK_NULL);
+  FieldInfoSupplier supplier(positions, java_fields);
+  builder.fill(table->data(), static_cast<size_t>(table->length()), supplier);
+  return table;
+}
+
 GrowableArray<FieldInfo>* FieldInfoStream::create_FieldInfoArray(const Array<u1>* fis, int* java_fields_count, int* injected_fields_count) {
-  int length = FieldInfoStream::num_total_fields(fis);
-  GrowableArray<FieldInfo>* array = new GrowableArray<FieldInfo>(length);
   FieldInfoReader r(fis);
-  *java_fields_count = r.next_uint();
-  *injected_fields_count = r.next_uint();
+  r.read_field_counts(java_fields_count, injected_fields_count);
+  int length = *java_fields_count + *injected_fields_count;
+
+  GrowableArray<FieldInfo>* array = new GrowableArray<FieldInfo>(length);
   while (r.has_next()) {
     FieldInfo fi;
     r.read_field_info(fi);
     array->append(fi);
   }
   assert(array->length() == length, "Must be");
-  assert(array->length() == *java_fields_count + *injected_fields_count, "Must be");
   return array;
 }
 
 void FieldInfoStream::print_from_fieldinfo_stream(Array<u1>* fis, outputStream* os, ConstantPool* cp) {
-  int length = FieldInfoStream::num_total_fields(fis);
   FieldInfoReader r(fis);
-  int java_field_count = r.next_uint();
-  int injected_fields_count = r.next_uint();
+  int java_fields_count;
+  int injected_fields_count;
+  r.read_field_counts(&java_fields_count, &injected_fields_count);
   while (r.has_next()) {
     FieldInfo fi;
     r.read_field_info(fi);
     fi.print(os, cp);
   }
 }
+
+class FieldInfoComparator: public PackedTableLookup::Comparator {
+  const FieldInfoReader* _reader;
+  ConstantPool* _cp;
+  const Symbol* _name;
+  const Symbol* _signature;
+
+public:
+  FieldInfoComparator(const FieldInfoReader* reader, ConstantPool* cp, const Symbol* name, const Symbol* signature):
+    _reader(reader), _cp(cp), _name(name), _signature(signature) {}
+
+  int compare_to(uint32_t position) override {
+    FieldInfoReader r2(*_reader);
+    r2.set_position_and_next_index(position, -1);
+    u2 name_index, sig_index;
+    r2.read_name_and_signature(&name_index, &sig_index);
+    Symbol* mid_name = _cp->symbol_at(name_index);
+    Symbol* mid_sig = _cp->symbol_at(sig_index);
+
+    return FieldInfoStream::compare_name_and_sig(_name, _signature, mid_name, mid_sig);
+  }
+
+#ifdef ASSERT
+  void reset(uint32_t position) override {
+    FieldInfoReader r2(*_reader);
+    r2.set_position_and_next_index(position, -1);
+    u2 name_index, signature_index;
+    r2.read_name_and_signature(&name_index, &signature_index);
+    _name = _cp->symbol_at(name_index);
+    _signature = _cp->symbol_at(signature_index);
+  }
+#endif // ASSERT
+};
+
+#ifdef ASSERT
+void FieldInfoStream::validate_search_table(ConstantPool* cp, const Array<u1>* fis, const Array<u1>* search_table) {
+  if (search_table == nullptr) {
+    return;
+  }
+  FieldInfoReader reader(fis);
+  int java_fields, injected_fields;
+  reader.read_field_counts(&java_fields, &injected_fields);
+  assert(java_fields > 0, "must be");
+
+  PackedTableLookup lookup(fis->length() - 1, java_fields - 1, search_table);
+  assert(lookup.element_bytes() * java_fields == static_cast<unsigned int>(search_table->length()), "size does not match");
+
+  FieldInfoComparator comparator(&reader, cp, nullptr, nullptr);
+  // Check 1: assert that elements have the correct order based on the comparison function
+  lookup.validate_order(comparator);
+
+  // Check 2: Iterate through the original stream (not just search_table) and try if lookup works as expected
+  reader.set_position_and_next_index(0, 0);
+  reader.read_field_counts(&java_fields, &injected_fields);
+  while (reader.has_next()) {
+    int field_start = reader.position();
+    FieldInfo fi;
+    reader.read_field_info(fi);
+    if (fi.field_flags().is_injected()) {
+      // checking only java fields that precede injected ones
+      break;
+    }
+
+    FieldInfoReader r2(fis);
+    int index = r2.search_table_lookup(search_table, fi.name(cp), fi.signature(cp), cp, java_fields);
+    assert(index == static_cast<int>(fi.index()), "wrong index: %d != %u", index, fi.index());
+    assert(index == r2.next_index(), "index should match");
+    assert(field_start == r2.position(), "must find the same position");
+  }
+}
+#endif // ASSERT
+
+void FieldInfoStream::print_search_table(outputStream* st, ConstantPool* cp, const Array<u1>* fis, const Array<u1>* search_table) {
+  if (search_table == nullptr) {
+    return;
+  }
+  FieldInfoReader reader(fis);
+  int java_fields, injected_fields;
+  reader.read_field_counts(&java_fields, &injected_fields);
+  assert(java_fields > 0, "must be");
+  PackedTableLookup lookup(fis->length() - 1, java_fields - 1, search_table);
+  auto printer = [&] (size_t offset, uint32_t position, uint32_t index) {
+    reader.set_position_and_next_index(position, -1);
+    u2 name_index, sig_index;
+    reader.read_name_and_signature(&name_index, &sig_index);
+    Symbol* name = cp->symbol_at(name_index);
+    Symbol* sig = cp->symbol_at(sig_index);
+    st->print("   [%zu] #%d,#%d = ", offset, name_index, sig_index);
+    name->print_symbol_on(st);
+    st->print(":");
+    sig->print_symbol_on(st);
+    st->print(" @ %p,%p", name, sig);
+    st->cr();
+  };
+
+  lookup.iterate(printer);
+}
+
+int FieldInfoReader::search_table_lookup(const Array<u1>* search_table, const Symbol* name, const Symbol* signature, ConstantPool* cp, int java_fields) {
+  assert(java_fields >= 0, "must be");
+  if (java_fields == 0) {
+    return -1;
+  }
+  FieldInfoComparator comp(this, cp, name, signature);
+  PackedTableLookup lookup(_r.limit() - 1, java_fields - 1, search_table);
+  uint32_t position;
+  static_assert(sizeof(uint32_t) == sizeof(_next_index), "field size assert");
+  if (lookup.search(comp, &position, reinterpret_cast<uint32_t*>(&_next_index))) {
+    _r.set_position(static_cast<int>(position));
+    return _next_index;
+  } else {
+    return -1;
+  }
+}