On Wed, 14 Jan 2026 08:43:34 GMT, Serguei Spitsyn <[email protected]> wrote:
>> src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java line
>> 171:
>>
>>> 169: public int hashCode() {
>>> 170: try {
>>> 171: return JDWP.ObjectReference.ObjectHashCode.process(vm,
>>> this).hashCode;
>>
>> Don't we want to limit the slow path to value objects?
>
>> We didn't do similar for Loom related APIs. I looked through the current
>> canXXX list, and they all seem related to JVMTI capabilities, which don't
>> apply to these two APIs. I think the expectation should be that the user
>> checks the JDWP version to determine if it is implemented.
>
> Yes, I remember about Loom. Asked this to be sure we are in sync. I agree
> with your suggestion.
> Don't we want to limit the slow path to value objects?
ok
>> src/jdk.jdi/share/classes/com/sun/tools/jdi/ReferenceTypeImpl.java line 334:
>>
>>> 332:
>>> 333: // Inline type is a concrete value class.
>>> 334: boolean isInlined() {
>>
>> Why is this called isInlined()? Shouldn't it be isValueClass()? Do we want
>> to expose it to users through ReferenceType? Should we also consider
>> exposing a new isIdentity() API?
>
>> Why is this called isInlined()? Shouldn't it be isValueClass()?
>
> I had the same question. The `isInlined()` matches the obsolete approach. The
> `isValueClass()` looks better.
>
>> Do we want to expose it to users through ReferenceType?
>
> If the JDI implementation needed it then there are some chances debuggers may
> need it as well. We have some time to decide. Preview allows to change the
> API specs in the future. I have a minor preference to expose it now. It is
> pretty simple and clear.
>
>> Should we also consider exposing a new isIdentity() API?
>
> It would be consistent with other modifier bits.
`isInlined` name (and its implementation) is consistent with hotspot code.
This API if required by ObjectReferenceImpl to determine if the reference is a
value object (so the class cannot be abstract or interface).
To me `IsIdentity` (I think `isValue` is better) should be in `ClassType`, I'm
updating ClassTypeImpl, it can be added to ClassType interface later if
debuggers really need it).
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1834#discussion_r2692339958
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1834#discussion_r2692365585
