On Fri, 25 Apr 2025 13:05:51 GMT, Radim Vansa <rva...@openjdk.org> wrote:
>> Radim Vansa has updated the pull request incrementally with three additional
>> commits since the last revision:
>>
>> - Fix VerifyRawIndexesTest
>> - Fix reordering in layout and annotations
>> - Use qsort_r for different platforms
>
> src/hotspot/share/oops/fieldInfo.hpp line 290:
>
>> 288: static int compare_symbols(const Symbol *s1, const Symbol *s2);
>> 289:
>> 290: static Array<u1>* create_FieldInfoStream(ConstantPool* constants,
>> GrowableArray<FieldInfo>* fields, int java_fields, int injected_fields,
>
> In the latest form the ConstantPool parameter is used only for assertion,
> though I think that it is rather important assertion.
The ConstantPool parameter can be limited to debug builds (the ones with
asserts) with the following patch:
diff --git a/src/hotspot/share/classfile/classFileParser.cpp
b/src/hotspot/share/classfile/classFileParser.cpp
index 48646c0fb83..19471bbf7ee 100644
--- a/src/hotspot/share/classfile/classFileParser.cpp
+++ b/src/hotspot/share/classfile/classFileParser.cpp
@@ -5813,7 +5813,7 @@ void ClassFileParser::post_process_parsed_stream(const
ClassFileStream* const st
int injected_fields_count = _temp_field_info->length() - _java_fields_count;
_fieldinfo_stream =
- FieldInfoStream::create_FieldInfoStream(_cp, _temp_field_info,
_java_fields_count,
+ FieldInfoStream::create_FieldInfoStream(DEBUG_ONLY(_cp COMMA)
_temp_field_info, _java_fields_count,
injected_fields_count,
loader_data(), CHECK);
_fields_status =
MetadataFactory::new_array<FieldStatus>(_loader_data,
_temp_field_info->length(),
diff --git a/src/hotspot/share/classfile/javaClasses.cpp
b/src/hotspot/share/classfile/javaClasses.cpp
index f3fdf28b96b..80ee179576c 100644
--- a/src/hotspot/share/classfile/javaClasses.cpp
+++ b/src/hotspot/share/classfile/javaClasses.cpp
@@ -963,7 +963,7 @@ void java_lang_Class::fixup_mirror(Klass* k, TRAPS) {
}
Array<u1>* old_stream = ik->fieldinfo_stream();
assert(fields->length() == (java_fields + injected_fields), "Must be");
- Array<u1>* new_fis =
FieldInfoStream::create_FieldInfoStream(ik->constants(), fields, java_fields,
injected_fields, k->class_loader_data(), CHECK);
+ Array<u1>* new_fis =
FieldInfoStream::create_FieldInfoStream(DEBUG_ONLY(ik->constants() COMMA)
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);
}
diff --git a/src/hotspot/share/oops/fieldInfo.cpp
b/src/hotspot/share/oops/fieldInfo.cpp
index dd1fa11042d..eb90e6bdae8 100644
--- a/src/hotspot/share/oops/fieldInfo.cpp
+++ b/src/hotspot/share/oops/fieldInfo.cpp
@@ -66,7 +66,7 @@ int FieldInfoStream::compare_symbols(const Symbol *s1, const
Symbol *s2) {
}
}
-Array<u1>* FieldInfoStream::create_FieldInfoStream(ConstantPool* constants,
GrowableArray<FieldInfo>* fields, int java_fields, int injected_fields,
+Array<u1>* FieldInfoStream::create_FieldInfoStream(DEBUG_ONLY(ConstantPool*
constants COMMA) GrowableArray<FieldInfo>* fields, int java_fields, int
injected_fields,
ClassLoaderData*
loader_data, TRAPS) {
// The stream format described in fieldInfo.hpp is:
// FieldInfoStream := j=num_java_fields k=num_injected_fields
JumpTable_offset(0/4 bytes) Field[j+k] JumpTable[(j - 1)/16 > 0] End
diff --git a/src/hotspot/share/oops/fieldInfo.hpp
b/src/hotspot/share/oops/fieldInfo.hpp
index 1974e2e4117..b98d9230533 100644
--- a/src/hotspot/share/oops/fieldInfo.hpp
+++ b/src/hotspot/share/oops/fieldInfo.hpp
@@ -287,7 +287,7 @@ class FieldInfoStream : AllStatic {
static int compare_symbols(const Symbol *s1, const Symbol *s2);
- static Array<u1>* create_FieldInfoStream(ConstantPool* constants,
GrowableArray<FieldInfo>* fields, int java_fields, int injected_fields,
+ static Array<u1>* create_FieldInfoStream(DEBUG_ONLY(ConstantPool* constants
COMMA) GrowableArray<FieldInfo>* fields, int java_fields, int injected_fields,
ClassLoaderData*
loader_data, TRAPS);
static GrowableArray<FieldInfo>* create_FieldInfoArray(const Array<u1>* fis,
int* java_fields_count, int* injected_fields_count);
static void print_from_fieldinfo_stream(Array<u1>* fis, outputStream* os,
ConstantPool* cp);
diff --git a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp
b/src/hotspot/share/prims/jvmtiRedefineClasses.cpp
index 4d50d6cb637..46ffe767448 100644
--- a/src/hotspot/share/prims/jvmtiRedefineClasses.cpp
+++ b/src/hotspot/share/prims/jvmtiRedefineClasses.cpp
@@ -3555,7 +3555,7 @@ void VM_RedefineClasses::set_new_constant_pool(
if (update_required) {
Array<u1>* old_stream = scratch_class->fieldinfo_stream();
assert(fields->length() == (java_fields + injected_fields), "Must be");
- Array<u1>* new_fis = FieldInfoStream::create_FieldInfoStream(cp, fields,
java_fields, injected_fields, scratch_class->class_loader_data(), CHECK);
+ Array<u1>* new_fis = FieldInfoStream::create_FieldInfoStream(DEBUG_ONLY(cp
COMMA) fields, java_fields, injected_fields,
scratch_class->class_loader_data(), CHECK);
scratch_class->set_fieldinfo_stream(new_fis);
MetadataFactory::free_array<u1>(scratch_class->class_loader_data(),
old_stream);
}
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24847#discussion_r2069379965
