On Thu, 23 Apr 2026 10:01:45 GMT, Marc Chevalier <[email protected]> wrote:
>> `acmp` for value object might be expensive: it may need substitutability
>> test, which is done through a Java call. That is the last resort when
>> everything failed, that is: we are in the presence of two value objects of
>> the same class, not statically known or speculated (profile absent or
>> polluted). While a lot of the acmp logic is trying to improve acmp at
>> compile-time, this change proposes a runtime check to take a fast path.
>>
>> The idea is that if the object is small enough, we can read all the fields
>> at once with a single 8-byte load, we exclude the non-interesting bits with
>> a mask, and we can simply compare the masked value from both objects.
>>
>> The first step happens at class loading. When the class is loaded, we check
>> whether all the fields can be read at once, from which offset, and the mask
>> to filter out useless bits. Useless bits may have two sources:
>> - padding between fields
>> - header: this happens because we cannot necessarily start reading at
>> `payload_offset`, the payload might be smaller than a long, and we could
>> overread. On the other hand, the header is always long enough (never less
>> than 8 bytes), so we may read from somewhere in the header to make sure we
>> don't go further than the payload. This piece of header must be filtered out.
>>
>> This strategy may not apply if the object is too big, or if it contains an
>> oop. Loading oop without precaution, concurrently with the GC is not a good
>> idea. This limitation is rather harmless: this fast path is mostly made not
>> to pay the price of a complicated `acmp` for magically migrated classes
>> (such as `Integer`), which are small and don't contain oops.
>>
>> There is an interesting corner case: the mask can be 0 for an empty value
>> class. We use a negative offset to signal that the fast path doesn't apply,
>> since it is not a valid value for the offset: we shouldn't load before the
>> object.
>>
>> The second step is to prepare the runtime check. In `do_acmp`, when we used
>> to prepare the Java call, now we have a parallel fast path. The condition
>> uses the fact that we already have the class node available (used for
>> checking operands have the same types). We can get the fast acmp offset from
>> the class, check if it looks valid (that is non-negative), and if so, take
>> the fast path that will load the mask, do the two loads and masking, and
>> compare the given values. If the class of the operand is known precisely at
>> parsing, we don't emit the fast path since the call will be nicely
>> intrinsified.
>>
>> The thir...
>
> Marc Chevalier has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Comment: do_acmp -> InlineKlass::Members
Great work Marc. This looks good to me but someone from the runtime team should
also take a look. Maybe @fparain?
src/hotspot/share/oops/inlineKlass.hpp line 103:
> 101:
> 102: /* When we can't intrinsify the substitutability check, we can still
> avoid the call to isSubstitutable at runtime if the value object is small
> enough.
> 103: * If all the fields are contained at once in a single long, we can
> load such a long on both operand, use a bitwise mask to remove the extra bits
Suggestion:
* If all the fields are contained at once in a single long, we can load
such a long from both operands, use a bitwise mask to remove the extra bits
src/hotspot/share/oops/inlineKlass.hpp line 207:
> 205: int null_marker_offset_in_payload() const { return
> null_marker_offset() - payload_offset(); }
> 206:
> 207: int64_t fast_acmp_offset() const { return
> members()._fast_acmp_offset; }
Indentation is off here.
src/hotspot/share/opto/parse2.cpp line 2483:
> 2481:
> 2482: assert(load_offset->in(2) != nullptr, "");
> 2483: if(!load_offset->in(2)->is_AddP()) return nullptr;
Suggestion:
if (!load_offset->in(2)->is_AddP()) return nullptr;
src/hotspot/share/opto/parse2.cpp line 2495:
> 2493:
> 2494: assert(load_members->in(2) != nullptr, "");
> 2495: if(!load_members->in(2)->is_AddP()) return nullptr;
Suggestion:
if (!load_members->in(2)->is_AddP()) return nullptr;
test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestAcmpFastPath.java line 289:
> 287: }
> 288:
> 289: // No acm fast path: the type is precise, and the call will be
> intrinsified
Should be `acmp, a few more occurrences below and in the other test.
-------------
Marked as reviewed by thartmann (Committer).
PR Review:
https://git.openjdk.org/valhalla/pull/2353#pullrequestreview-4159920686
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2353#discussion_r3128631986
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2353#discussion_r3130537597
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2353#discussion_r3128780459
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2353#discussion_r3128781519
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2353#discussion_r3128551450
