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

Reply via email to