RFR: 8352075: Perf regression accessing fields [v4]

[Radim Vansa]

On Tue, 13 May 2025 11:39:32 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Move constant to static final var
>
> Also can you include this:
> 
> diff --git a/src/hotspot/share/oops/fieldInfo.cpp b/src/hotspot/share/oops/fieldInfo.cpp
> index 300b45277ad..7668faf6187 100644
> --- a/src/hotspot/share/oops/fieldInfo.cpp
> +++ b/src/hotspot/share/oops/fieldInfo.cpp
> @@ -37,8 +37,8 @@ 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() ? cp->symbol_at(generic_signature_index())->as_utf8() : "",
> +                is_contended() ? contended_group() : 0);
>  }
>  
> 
> which I fixed for printing.
> 
> Also, sorting the fields is inlined code in parse_fields.  Can you make it it's own method?  And it seems to sort the fields before the injected fields are added, so how do you find the injected fields?  I think injected fields aren't added to klasses with >16 fields maybe?
> 
> I was going to write a test with JVMTI GetFields and > 16 fields because I'm not sure how/if you're fixing the order that the fields are returned by that API.

@coleenp @SirYwell Based on the feedback and considerations from your reviews, I've pushed a version that does not reorder fields. The `CCC` test shows the same performance as the original PR, in case of huge class the improvement is even better (0.23 s vs 0.5 s). The price for O(log(N)) field lookup is 3 bytes per field for medium classes (17 - 256 non-injected fields), 4-5 bytes for bigger classes.
Due to not reordering the fields at all the chance for regression is significantly lower. I have not executed the whole JCK but tried the mentioned test and it does pass.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/24847#issuecomment-2880137455