On Thu, 27 Aug 2026 06:28:18 GMT, Yasumasa Suenaga <[email protected]> wrote:

>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ObjectHeap.java 
>> line 198:
>> 
>>> 196: 
>>> 197:   // This method would be used to instantiate flattened object.
>>> 198:   public Oop newOop(OopHandle handle, InlineKlass klass) {
>> 
>> Just want to make sure I'm understanding this correctly. `handle` is not 
>> actually a valid handle, but is instead an offset off of a valid handle. I 
>> think this should be made clear here.
>
> I updated the comment to explain where `handle` points. Is it enough?

>   // "handle" does not point directly to the payload of a flattened object.

I thought it did point directly to the payload of the flattened object. Maybe 
instead of "flattened object" you meant "holder object that contains the 
flattened field payload".

>> src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/oops/ObjectHeap.java 
>> line 199:
>> 
>>> 197:   // This method would be used to instantiate flattened object.
>>> 198:   public Oop newOop(OopHandle handle, InlineKlass klass) {
>>> 199:     return (handle == null) ? null
>> 
>> Can it really ever be null? This seems like it would be a bug.
>
> `handle` should not be null so far, but I guess this method might be required 
> to accept null in future in some case (I cannot explain the example now). We 
> should refer null marker in flattened object when we need to handle it as 
> null. However SA might want to call `newOop()` for flattened *null* object in 
> future because oop instantiation is different from HotSpot in SA.
> 
> I can agree we treat as the bug if `null` is passed as `handle` here, and 
> will add null check in future if needs. Which do you like?

i think adding a null check now would be good. If null is ever supported for a 
flattened field, SA is going to need additional changes to support it.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/32310#discussion_r3872976012
PR Review Comment: https://git.openjdk.org/jdk/pull/32310#discussion_r3872936071

Reply via email to