RFR: 8352075: Perf regression accessing fields [v20]

Radim Vansa rvansa at openjdk.org
Tue Jun 3 05:53:55 UTC 2025


On Tue, 3 Jun 2025 00:05:35 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/utilities/packedTable.hpp line 38:
>> 
>>> 36:   uint32_t _key_mask;
>>> 37:   unsigned int _value_shift;
>>> 38:   uint32_t _value_mask;
>> 
>> Aren't all 4 of these types the same?  can you make them all uint32_t or all unsigned int?  (former preferred).
>
> Can you explain somewhere how fields are mapped to this?  I assume they're sorted, for some reason I expected the packed table to be {name-cp-index, sig-cp-index, offset-in-fieldstream-for-direct-access}.  Does every field get 4 ints ?  So why is it packed into ```Array<u1>``` rather than just use ```Array<u4>```?  So much packing code that I don't know how anyone could ever debug it.

Yes, in practice these all are of the same size, but in case of the masks (as well as in case of arguments in API) I want to stress out that these are 32 bit numbers. The `unsigned int`s are just 'some not too big number'.
Is there any general guidance on deciding between `unsigned int` (I suppose just `unsigned` is not recommended), `uint32_t` and `u4`?

I was hoping that the comment on line 68 explains the intended use, but I can be more verbose and document each method. When the packed table is used for fieldinfo, it's { offset-in-fieldstream, index-in-fieldstream }. The Comparator implementation can translate offset-in-fieldstream -> { name, signature } and then do the comparison. The `index-in-fieldstream` is kind of second-class citizen; we need to fill it into `FieldInfo` and it is not encoded in the stream, therefore we need to encode it in the packed table.

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

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


More information about the serviceability-dev mailing list