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