RFR: 8180450: secondary_super_cache does not scale well [v14]

[Vladimir Kozlov]

On Fri, 12 Apr 2024 15:10:19 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> This PR is a redesign of subtype checking. 
>> 
>> The implementation of subtype checking in the HotSpot JVM is now twenty years old. There have been some performance-related bugs reported, and the only way to fix them is a redesign of the way it works.
>> 
>> So what's changed, so that the old design should be replaced?
>> 
>> Firstly, the computers of today aren't the computers of twenty years ago. It's not merely a matter of speed: the systems are much more parallel, both in the sense of having more cores and each core can run many instructions in parallel. Because of this, the speed ratio between memory accesses and the rate at which we can execute instructions has become wider and wider.
>> 
>> The most severe reported problem is to do with the "secondary supers cache". This is a 1-element per-class cache for interfaces (and arrays of interfaces). Unfortunately, if two threads repeatedly update this cache, the result is that a cache line ping-pongs between cores, causing a severe slowdown. 
>> 
>> Also, the linear search for an interface that is absent means that the entire list of interfaces has to be scanned. This plays badly with newer language features such as JEP 406, pattern matching for switch.
>> 
>> However, the computers of today can help us. The very high instruction-per-cycle rate of a Great Big Out-Of-Order (GBOOO) processor allows us to execute many of the instructions of a hash table lookup in parallel, as long as we avoid dependencies between instructions. 
>> 
>> The solution
>> ------------
>> 
>> We use a hashed lookup of secondary supers. This is a 64-way hash table, with linear probing for collisions. The table is compressed, in that null entries are removed, and the resulting hash table fits into the same secondary supers array as today's unsorted array of secondary supers. This means that existing code in HotSpot that simply does a linear scan of the secondary supers array does not need to be altered.
>> 
>> We add a bitmap field to each Klass object. This bitmap contains an occupancy bit corresponding to each element of the hash table, with a 1 indicating element presence. As well as allowing the hash table to be decompressed, this bimap is used as a simple kind of Bloom Filter. To determine whether a superclass is present, we simply have to check a single bit in the bitmap. If the bit is clear, we know that the superclass is not present. If the bit is set, we have to do a little arithmetic and then consult the hash table.
>> 
>> It works like th...
>
> Andrew Haley has updated the pull request incrementally with one additional commit since the last revision:
> 
>   JDK-8180450: secondary_super_cache does not scale well

Few comments.

src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 4432:

> 4430:     }
> 4431:   }
> 4432: 

Can you move it up after `StubRoutines::_montgomerySquare` initialization?

src/hotspot/cpu/x86/stubRoutines_x86.hpp line 41:

> 39:   // Windows have more code to save/restore registers
> 40:   _compiler_stubs_code_size     = 20000 LP64_ONLY(+39000) WINDOWS_ONLY(+2000),
> 41:   _final_stubs_code_size        = 10000 LP64_ONLY(+20000) WINDOWS_ONLY(+2000) ZGC_ONLY(+24000)

Do we still need it after you moved code to compiler stubs section?

src/hotspot/cpu/x86/vm_version_x86.cpp line 1786:

> 1784:       }
> 1785:       FLAG_SET_DEFAULT(UseSecondarySupersTable, false);
> 1786:     }

No need this with other changes I suggest.

src/hotspot/cpu/x86/vm_version_x86.hpp line 793:

> 791:   // x86_64 supports secondary supers table
> 792:   constexpr static bool supports_secondary_supers_table() {
> 793:     return LP64_ONLY(true) NOT_LP64(false); // not implemented on x86_32

I think it should be:

  return LP64_ONLY(supports_popcnt()) NOT_LP64(false); // not implemented on x86_32

and no need changes in `vm_version_x86.cpp`. The main check will be done in `arguments.cpp`

src/hotspot/share/cds/filemap.hpp line 274:

> 272:   bool compressed_oops()                   const { return _compressed_oops; }
> 273:   bool compressed_class_pointers()         const { return _compressed_class_ptrs; }
> 274:   bool use_secondary_supers_table()        const { return _use_secondary_supers_table; }

Do we really need this accessor which is used only in one place?

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

PR Review: https://git.openjdk.org/jdk/pull/18309#pullrequestreview-1998112521
PR Review Comment: https://git.openjdk.org/jdk/pull/18309#discussion_r1562961409
PR Review Comment: https://git.openjdk.org/jdk/pull/18309#discussion_r1562966870
PR Review Comment: https://git.openjdk.org/jdk/pull/18309#discussion_r1562945973
PR Review Comment: https://git.openjdk.org/jdk/pull/18309#discussion_r1562945087
PR Review Comment: https://git.openjdk.org/jdk/pull/18309#discussion_r1562954617