On Wed, 30 Apr 2025 20:15:36 GMT, Frederic Parain <fpar...@openjdk.org> wrote:
>> 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_f...
I don't think adding DEBUG_ONLY optimizes anything and kind of looks messy, I
don't like this change unless the product compiler complains about an unused
parameter.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24847#discussion_r2071473593
