On Tue, 28 Apr 2026 20:58:02 GMT, Chris Plummer <[email protected]> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review: comments updates for two tests
>
> src/hotspot/share/prims/jvmtiImpl.cpp line 500:
>
>> 498: BufferedValuePayload dst(obj_copy, klass);
>> 499: src.copy_to(dst);
>> 500: }
>
> When you combine the semantics of klass->allocate_instance() and copying with
> BufferedValuePayload, are you guaranteed to have a fully initialized object?
> Also, does BufferedValuePayload do deep cloning, and if yes, is that
> necessary? I think it would be good to have someone from the GC team to look
> at this specific code to make sure there are no GC concerns with what you are
> doing.
Thanks for the questions.
> When you combine the semantics of klass->allocate_instance() and copying with
> BufferedValuePayload, are you guaranteed to have a fully initialized object?
What do you mean by this? The purpose is to create a snapshot even though the
object has not been fully initialized yet. In fact, it can be non-initialized
at all. Is it a concern about a need for some barrier actions like:
// Must ensure the content of the buffered value is visible
// before publishing the buffered value oop
OrderAccess::storestore();
Please, see `FlatValuePayload::read(TRAPS)`.
I guess, we can check with @dean-long if it is really needed.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/2363#discussion_r3157597246
