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

Andrew Haley aph at openjdk.org
Mon Jul 22 16:50:47 UTC 2024


> This patch expands the use of a hash table for secondary superclasses
> to the interpreter, C1, and runtime. It also adds a C2 implementation
> of hashed lookup in cases where the superclass isn't known at compile
> time.
> 
> HotSpot shared runtime
> ----------------------
> 
> Building hashed secondary tables is now unconditional. It takes very
> little time, and now that the shared runtime always has the tables, it
> might as well take advantage of them. The shared code is easier to
> follow now, I think.
> 
> There might be a performance issue with x86-64 in that we build
> HotSpot for a default x86-64 target that does not support popcount.
> This means that HotSpot C++ runtime on x86 always uses a software
> emulation for popcount, even though the vast majority of machines made
> for the past 20 years can do popcount in a single instruction. It
> wouldn't be terribly hard to do something about that.
> 
> Having said that, the software popcount is really not bad.
> 
> x86
> ---
> 
> x86 is rather tricky, because we still support
> `-XX:-UseSecondarySupersTable` and `-XX:+UseSecondarySupersCache`, as
> well as 32- and 64-bit ports. There's some further complication in
> that only `RCX` can be used as a shift count, so there's some register
> shuffling to do. All of this makes the logic in macroAssembler_x86.cpp
> rather gnarly, with multiple levels of conditionals at compile time
> and runtime.
> 
> AArch64
> -------
> 
> AArch64 is considerably more straightforward. We always have a
> popcount instruction and (thankfully) no 32-bit code to worry about.
> 
> Generally
> ---------
> 
> I would dearly love simply to rip out the "old" secondary supers cache
> support, but I've left it in just in case someone has a performance
> regression.
> 
> The versions of `MacroAssembler::lookup_secondary_supers_table` that
> work with variable superclasses don't take a fixed set of temp
> registers, and neither do they call out to to a slow path subroutine.
> Instead, the slow patch is expanded inline.
> 
> I don't think this is necessarily bad. Apart from the very rare cases
> where C2 can't determine the superclass to search for at compile time,
> this code is only used for generating stubs, and it seemed to me
> ridiculous to have stubs calling other stubs.
> 
> I've followed the guidance from @iwanowww not to obsess too much about
> the performance of C1-compiled secondary supers lookups, and to prefer
> simplicity over absolute performance. Nonetheless, this is a
> complicated patch that touches many areas.

Andrew Haley has updated the pull request incrementally with two additional commits since the last revision:

 - Review comments
 - Review comments

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/19989/files
  - new: https://git.openjdk.org/jdk/pull/19989/files/98f6b2b7..c252efcb

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=19989&range=03
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19989&range=02-03

  Stats: 41 lines in 10 files changed: 9 ins; 17 del; 15 mod
  Patch: https://git.openjdk.org/jdk/pull/19989.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/19989/head:pull/19989

PR: https://git.openjdk.org/jdk/pull/19989


More information about the core-libs-dev mailing list