RFR: 8352075: Perf regression accessing fields [v21]

Radim Vansa rvansa at openjdk.org
Thu Jun 5 20:55:03 UTC 2025


On Thu, 5 Jun 2025 18:54:07 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Radim Vansa has updated the pull request incrementally with three additional commits since the last revision:
>> 
>>  - Moved jtreg test
>>  - Improved documentation
>>  - Fix coding style (asterisk placement)
>
> src/hotspot/share/oops/fieldInfo.cpp line 164:
> 
>> 162:   r.read_field_counts(&java_fields, &injected_fields);
>> 163:   assert(java_fields >= 0, "must be");
>> 164:   if (java_fields == 0 || fis->length() == 0 || static_cast<uint>(java_fields) < BinarySearchThreshold) {
> 
> I don't know why you only sort Java fields and ignore the injected fields.  JavaClasses::compute_offsets calls find_local_field, so might not find an injected field, I assume in the java.lang.Class (mirror).  Should this sorted cache exclude classes with injected fields? ie if injected_fields > 0?
> If you exclude classes with injected fields, you could remove the javaClasses code (and maybe not have to re-sort any fields during dynamic dumping (?))

I don't build a search table for injected fields because I am trying to fix performance of `InstanceKlass::find_local_field` and this uses `JavaFieldStream` - that is/was ignoring injected fields in the iteration as well.
Classes with injected fields are not excluded, we just don't build the table for them. There's not lookup by name+signature, just `InstanceKlass::field(int index)` which uses iteration through `AllFieldStream`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24847#discussion_r2130327017


More information about the serviceability-dev mailing list