On Mon, 3 Aug 2026 09:05:40 GMT, Tobias Hartmann <[email protected]> wrote:
>> This PR replaces the Valhalla draft PR >> https://github.com/openjdk/valhalla/pull/2642, which was not integrated due >> to the code freeze immediately before the Valhalla mainline integration. >> >> --------- >> >> It is conceptually very similar to acmp: it has two parts. >> >> # Static Expansion >> >> If the operand of `identityHashCode` is known at compile-time, we can >> basically inline the implementation of >> `ValueObjectMethods.valueObjectHashCode`. There are a few points worth >> noting. >> 1. The seed of the hash is the hash of the mirror class object. That object >> is not a value object, but an identity class. We look in the header of the >> said class whether it was cached already, which is very likely. Otherwise, >> we give up the static expansion: it is not worth replacing a call with a >> call. >> 2. Oops are hell. We don't expand when oops are involved. >> 3. The runtime implementation performs some unsafe gets to get all pieces of >> a segment in a simple, greedy way. Which means that in the case we have a >> value class made of 2 `int`, the likely outcome is that it would get both at >> once with a single call to `getLong`. This is fine, we can do that, but we >> need to mark the access as mismatch and unsafe (but aligned, since the acmp >> maps are smartly done). But in the case we have a class made of a single >> `int`, it would be unfortunate to mark the `getInt` access as mismatch since >> it prevents some optimizations. So if we are getting exactly a field, we >> detect it, and we mark the load as non-mismatch (match?). >> >> # Fast Path >> >> This supports only objects with a simple shape: one segment of data, being >> 1, 2, 4 or 8-byte long, which is enough to cover many migrated classes (but >> not dates for instance). This feature is morally very similar to the acmp >> fast path. It is controlled by the diagnostic flag `UseHashcodeFastPath`. >> Alike acmp, we need to sabotage the fast path in case we can do a static >> expansion later. >> >> I can only suggest you take a look at `inlineKlass.hpp` on how that works. >> >> # Benchmarking >> >> Microbenchmarking is rather unsurprising: >> - haven't changed: >> - `null` was and is still fast (1-2ns) >> - hashcode cached in the header was and is still fast (2ns) >> - identity objects without cached hashcode are not too slow, and still >> aren't (20-25ns) >> - known at compile time (60-70ns): >> - value objects with oops >> - unknown at compile time (60-70ns): >> - value objects with sizes that are not 1, 2, 4 or 8 >> - value objects with oops >> - ... > > test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestHashcodeFastPath.java > line 508: > >> 506: // TODO: complicated structure, static expansion, random values >> with/without late removal of fast path >> 507: // TODO: all the simple structures, fast path taken if enabled >> 508: // TODO: example with oops, no fast path taken, no expansion > > I think we should file at least a follow-up RFE for these and remove the > TODOs from the test (or add the listed cases with this PR). Random values > would be good. It's a leftover. What I meant was done. In `run2`, I already use `G.ints()` and `G.longs()` for the random. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32144#discussion_r3706233306
