On Thu, 18 Dec 2025 20:20:51 GMT, Chris Plummer <[email protected]> wrote:
>> Updated implementation of ObjectReference.equals and
>> ObjectReference.hashCode to comply the spec for value objects.
>> Added the test for value object ctor debugging, the test verifies the
>> behaviour is expected.
>> There is an issue with instance filter, it till be fixed separately (it's
>> not yet clear how it would be better to fix it)
>>
>> testing: tier1..4, hs-tier5-svc
>
> src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java line 150:
>
>> 148: public boolean equals(Object obj) {
>> 149: if (obj instanceof ObjectReferenceImpl other) {
>> 150: if (ref() == other.ref() && super.equals(obj)) {
>
> I'm wondering if the super.equals(obj) call is necessary. The way JDI is
> implemetned, there is a 1-to-1 relationship between the ObjectID and the
> ObjectReference. If the ObjectIDs match, then the ObjectRefernces instances
> should be one in the same. So either one of these checks should be
> sufficient. I don't think both are needed.
I agree that super.equals call is not necessary, just kept it as it was
> src/jdk.jdi/share/classes/com/sun/tools/jdi/ObjectReferenceImpl.java line 159:
>
>> 157: } catch (JDWPException exc) {
>> 158: throw exc.toJDIException();
>> 159: }
>
> So the downside here is that when we don't have equality w.r.t. ObjectIDs, we
> do the heavyweight call over JDWP. A while back I did a scan of users of
> equals() in the JDI implementation and jdb, and it seemed it was always done
> on known subtypes (ClassLoaderReference, ThreadReference, ClassReference,
> etc). So I doubt in practice you would ever see two value types being
> compared, unless the debugger was doing it for some reason. So if you did
> limit this JDWP call to value types, probably it would be rare to
> non-existent that it would ever be called.
agreed. Do I understand correctly that I cannot modify existing command (jdwp
protocol shoul be backward compatible?) to get classes and need a new one to
check if the class is value class?
> src/jdk.jdwp.agent/share/native/libjdwp/ObjectReferenceImpl.c line 410:
>
>> 408:
>> 409: ref = commonRef_idToRef(env, id);
>> 410: (void)outStream_writeInt(out, objectHashCode(ref));
>
> Interesting that the debug agent already has an objectHashCode() but there
> were no users of it.
`ObjectReferenceImpl.hashCode()` calculates it by itself from objectID (it
should call objectHashCode)
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1834#discussion_r2632546790
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1834#discussion_r2632555234
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1834#discussion_r2632560792