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