On Tue, 5 May 2026 22:55:08 GMT, Serguei Spitsyn <[email protected]> wrote:

>> This PR implements two JVMTI enhancements: 
>> [8382702](https://bugs.openjdk.org/browse/JDK-8382702) and 
>> [8381063](https://bugs.openjdk.org/browse/JDK-8381063)
>> 
>> The update includes the following changes:
>>  - add spec clarifications for `VMObjectAlloc` and `SampledObjectAlloc` 
>> related to value object allocations
>>  - pass `nullptr` for `jobject` parameter of `VMObjectAlloc` and 
>> `SampledObjectAlloc` event callbacks
>>  - add test coverage with value object allocations for both event callbacks
>> 
>> Testing:
>>  - Test locally with newly added tests
>>  - Run mach5 tiers 1-6
>>  
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Serguei Spitsyn has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - review: add explaining comment to new test
>  - fix build error in new test on Windows

src/hotspot/share/prims/jvmtiExport.cpp line 1140:

> 1138:  public:
> 1139:    JvmtiObjectAllocEventMark(JavaThread *thread, oop obj) : 
> JvmtiClassEventMark(thread, oop_to_klass(obj)) {
> 1140:      _jobj = obj->is_inline()? nullptr : (jobject)to_jobject(obj); // 
> nullptr for value objects

ident: missed space

test/hotspot/jtreg/serviceability/jvmti/valhalla/SampledObjectAllocValue/SampledObjectAllocValue.java
 line 51:

> 49:     public static void main(String[] args) throws Exception {
> 50:         enableEvents(Thread.currentThread(), ValueClass.class);
> 51:         // Allocate value objects to trigger JVMYI SampledObjectAlloc 
> events.

JVMYI -> JVMTI

test/hotspot/jtreg/serviceability/jvmti/valhalla/SampledObjectAllocValue/libSampledObjectAllocValue.cpp
 line 58:

> 56:     return; // interested in tested value class only
> 57:   }
> 58:   if (object != nullptr) {

I thinks it would be nice to verify the size and klass values. At least not 
nullptr/zero.

test/hotspot/jtreg/serviceability/jvmti/valhalla/SampledObjectAllocValue/libSampledObjectAllocValue.cpp
 line 67:

> 65: void JNICALL
> 66: VMInit(jvmtiEnv* jvmti, JNIEnv* jni, jthread thread) {
> 67:   if (events_counter != 0) {

You can check this in `Java_SampledObjectAllocValue_enableEvents`
and get rid of VMInit.

test/hotspot/jtreg/serviceability/jvmti/valhalla/VMObjectAllocValue/VMObjectAllocValueTest.java
 line 26:

> 24: /**
> 25:  * @test
> 26:  * @summary Verifies that a VMObjectAlloc event is generated for object 
> created using MethodHandle

missed 'value'

test/hotspot/jtreg/serviceability/jvmti/valhalla/VMObjectAllocValue/libVMObjectAllocValueTest.cpp
 line 31:

> 29: 
> 30: static jvmtiEnv *jvmti = nullptr;
> 31: // JVMTI_ERROR_WRONG_PHASE guard

Wonder if this is still needed. No events should be generated after death.
Have you seen that test failed with JVMTI_ERROR_WRONG_PHASE?

test/hotspot/jtreg/serviceability/jvmti/valhalla/VMObjectAllocValue/libVMObjectAllocValueTest.cpp
 line 49:

> 47:   }
> 48: 
> 49:   char *signature = nullptr;

Not critical, but needed to deallocate signature.

test/hotspot/jtreg/serviceability/jvmti/valhalla/VMObjectAllocValue/libVMObjectAllocValueTest.cpp
 line 60:

> 58:     if (object != nullptr) {
> 59:       fatal(jni, "Failed: object parameter for value object alloc is 
> expected to be nullptr");
> 60:     }

Also check make sense to check size, at least for non-zero.

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2386#discussion_r3192805440
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2386#discussion_r3192782264
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2386#discussion_r3192866740
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2386#discussion_r3192819881
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2386#discussion_r3192854494
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2386#discussion_r3192851470
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2386#discussion_r3192843092
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/2386#discussion_r3192871875

Reply via email to