On Mon, 5 May 2025 06:35:39 GMT, Radim Vansa <rva...@openjdk.org> wrote:
>> src/hotspot/share/oops/fieldInfo.cpp line 52:
>>
>>> 50:
>>> 51: int FieldInfoStream::compare_symbols(const Symbol *s1, const Symbol
>>> *s2) {
>>> 52: // not lexicographical sort, since we need only total ordering
>>
>> If only a total ordering is required, why defining a new method instead of
>> reusing Symbol::fast_compare() ?
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24847#discussion_r2098921193
