On Mon, 3 Aug 2026 12:30:33 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 >> - ... > > src/hotspot/share/opto/inlinetypenode.cpp line 1029: > >> 1027: >> 1028: const Type* arg_type = igvn.type(arg); >> 1029: assert(!arg_type->maybe_null(), "must check null beforehand"); > > Why is this guaranteed? Isn't this only done in the intrinsic slow path but > not in the general case? I don't think so. The intrinsics starts by doing something like if (arg == null) { if (static) { return 0; } throw NullPointerException(); } // Here, arg != null // the rest of the intrinsics ... with cache path, fast path and slow path. Here, we are expanding the slow path (that is the runtime call), so we should already be after the null check. Am I missing something? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32144#discussion_r3706145049
