On Tue, 6 Jan 2026 00:18:50 GMT, Leonid Mesnik <[email protected]> wrote:
>> Chad Rakoczy has updated the pull request incrementally with two additional
>> commits since the last revision:
>>
>> - Use jvmti_common functions
>> - Remove seperate process
>
> test/hotspot/jtreg/serviceability/jvmti/NMethodRelocation/libNMethodRelocationTest.cpp
> line 39:
>
>> 37: * Helper function to update the shouldExit field in the Java test class.
>> 38: */
>> 39: static void updateShouldExit() {
>
> The simpler and more usual way is to have native variable
> `static std::atomic<bool> should_exit;`
> and poke it from hava code with
>
> JNIEXPORT jboolean JNICALL
> java_..._shouldExitt(JNIEnv *env, jobject obj) {
> return should_exit.load();
> }
>
> It is more compact and and simpler.
> Also, I am not sure if AttachCurrentThread supposed to work for NonJavaThread
> or VM-intrenal thread, like compiler thread.
I'd suggest to introduce a native method `shouldExit()` which is called from
java level and returns the `should_exit` value. It will be much simpler than
the `updateShouldExit()` function implementation.
> test/hotspot/jtreg/serviceability/jvmti/NMethodRelocation/libNMethodRelocationTest.cpp
> line 116:
>
>> 114:
>> 115: // Only track events for "compiledMethod"
>> 116: char* name = get_method_name(jvmti, env, method);
>
> Can you please add new method
> `get_method_name(jvmti, method);`
> that uses
> `check_jvmti_error(jvmtiError err, const char* msg) `
> and doesn't need JNI.
>
> So no need to use `AttachCurrentThread`.
Good idea.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28683#discussion_r2663368888
PR Review Comment: https://git.openjdk.org/jdk/pull/28683#discussion_r2663386507
