RFR: 8331341: secondary_super_cache does not scale well: C1 and interpreter [v23]

[Andrew Haley]

On Thu, 10 Oct 2024 02:25:10 GMT, Dean Long <dlong at openjdk.org> wrote:

>> Andrew Haley has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 61 commits:
>> 
>>  - Merge from 4ff72dc57e65e99b129f0ba28196994edf402018
>>  - Fix s390
>>  - Use post-incrememnt RegSet operator.
>>  - Merge branch 'clean' into JDK-8331658-work
>>  - Fix merge
>>  - Merge branch 'clean' into JDK-8331658-work
>>  - Merge from JDK head.
>>  - Cleanup
>>  - Fix shared code
>>  - Fix shared code
>>  - ... and 51 more: https://git.openjdk.org/jdk/compare/4ff72dc5...a7612674
>
> src/hotspot/share/oops/klass.inline.hpp line 96:
> 
>> 94:   uintx bitmap = _secondary_supers_bitmap;
>> 95: 
>> 96:   constexpr int highest_bit_number = SECONDARY_SUPERS_TABLE_SIZE - 1;
> 
> It looks like this needs to be the number of bits in a uintx in order to shift out the high bits, which matches the current definition of SECONDARY_SUPERS_TABLE_SIZE.  But I'm wondering if we should separate the two, so that we could, for example, temporarily reduce SECONDARY_SUPERS_TABLE_SIZE to a smaller size for stress testing without the algorithm breaking.  So highest_bit_number here and in the assembly code would be something like sizeof(uintx) * BitsPerByte - 1.

Yes, it is the number of bits in the bitmap. Always 64 up 'til now. The fundamental nature of the algorithm is such that there is an occupancy bit for each possible slot. I could assert that here.

Severe stress testing is supported by `StressSecondarySupers`, which uses a terrible hash function. I think that option tests all of the code paths.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19989#discussion_r1795401743