On Mon, 5 Jan 2026 21:11:00 GMT, Chad Rakoczy <[email protected]> wrote:
>> [JDK-8369150](https://bugs.openjdk.org/browse/JDK-8369150) >> >> The test checks for JVMTI `COMPILED_METHOD_LOAD` and >> `COMPILED_METHOD_UNLOAD` events to be published for a relocated nmethod. It >> would originally intermittently fail if the JVM exited before it had time to >> publish the events so now it loops and forces GCs to encourage event >> publishing. The test fails if the events are received in the incorrect order >> (such as an unload before a load) or if the correct events are not received >> and the test times out. > > Chad Rakoczy has updated the pull request incrementally with two additional > commits since the last revision: > > - Use jvmti_common functions > - Remove seperate process Thanks for update. The test looks simpler and work faster now. Please find my inline comments related ro AttachCurrentThread usage. Here are some inline comments 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. 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`. ------------- Changes requested by lmesnik (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/28683#pullrequestreview-3628832754 PR Review: https://git.openjdk.org/jdk/pull/28683#pullrequestreview-3628833420 PR Review Comment: https://git.openjdk.org/jdk/pull/28683#discussion_r2663164281 PR Review Comment: https://git.openjdk.org/jdk/pull/28683#discussion_r2663167608
