RFR: 8352075: Perf regression accessing fields [v3]

[Ioi Lam]

On Tue, 20 May 2025 21:41:59 GMT, John R Rose <jrose at openjdk.org> wrote:

>> The problem is CDS; I have really started with `fast_compare()`, but after dehydration the pointers changed and the comparison did not work anymore. This is also a reason why I could not use the hashcode for the ordering.
>> If you'd prefer lexicographical sort (just a few extra lines) I could use that one...
>
> It's not good to introduce a new symbol comparator in a random place.  Symbol comparators should be defined and maintained centrally, in symbol.[ch]pp.
> 
> If there's a bug with fast_compare, I want us to fix it eventually.  So for now, define a companion function in symbol.hpp called fast_compare_cds_safe.  We can work with it from there.  You can use your proposed logic but we will probably want to fiddle with it, and probably merge it with fast_compare (per se) if CDS can be adjusted.

This problem should happen only with dynamic CDS archives.

With the static CDS archive (and AOT caches as well; `-XX:AOTCache` basically are static CDS archives), Symbols are copied into the archive while preserving the order of their relative addresses.

However, when creating a dynamic CDS archive, we first load a static archive. Let's say we load the static archive at address 0x800000000, a Symbol `A` in the static archive could have an address of `0x800001000`. Now, we load a class `K` who has a method named `B` but such a Symbol is not in the static archive, this Symbol will be dynamically allocated from the Metaspace. Since the address is decided by the OS, this symbol could have an address below the static archive. E.g., `0x700000000`;

Let's say `K` defines two methods, `A()` and `B()`, they are stored in the `InstanceKlass::_methods` array for `K`. This array is sorted by ascending addresses of the method's names. So `K::B()` comes before `K::A()`.

When `K` is copied into the dynamic archive, the symbol `B` will also be copied into the dynamic archive. The dynamic archive resides in a memory region above the static archive, so `B` will have an address like `0x800002000`. Therefore, we want the copy of `K` in the dynamic archive to have a `_methods` array where  `K::A()` comes before `K::B()`.

This is done in [`DynamicArchiveBuilder::sort_methods()`](https://github.com/openjdk/jdk/blob/cedd1a5343dceb5394b8ed5ea78bb717f05c8caf/src/hotspot/share/cds/dynamicArchive.cpp#L138)

My recommendation is to sort your table using `fast_compare()`, and re-sort this table in the same fashion as `DynamicArchiveBuilder::sort_methods()`.

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

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