Hi Chris,

Good catch.
I agree, for consistency the enqueue_event() is better to follow the JvmtiDeferredEventQueue::enqueue() to avoid slowdebug minimal build failures.

New patch is:

diff --git a/src/hotspot/share/prims/jvmtiThreadState.hpp b/src/hotspot/share/prims/jvmtiThreadState.hpp
--- a/src/hotspot/share/prims/jvmtiThreadState.hpp
+++ b/src/hotspot/share/prims/jvmtiThreadState.hpp
@@ -396,7 +396,7 @@
   void set_should_post_on_exceptions(bool val) { _thread->set_should_post_on_exceptions_flag(val ? JNI_TRUE : JNI_FALSE); }

   // Thread local event queue, which doesn't require taking the Service_lock.
-  void enqueue_event(JvmtiDeferredEvent* event);
+  void enqueue_event(JvmtiDeferredEvent* event) NOT_JVMTI_RETURN;
   void post_events(JvmtiEnv* env);
   void run_nmethod_entry_barriers();
 };

Thanks,
Serguei

On 1/6/20 6:39 PM, Chris Plummer wrote:
Hi Serguei,

JvmtiDeferredEventQueue::enqueue() is a NOP for minimal builds, so it can be called even for minimalVM builds:

  void enqueue(JvmtiDeferredEvent event) NOT_JVMTI_RETURN;

The changes for JDK-8212160 seem to have put some wrapper code around its use, resulting in ServiceThread::enqueue_deferred_event() and JvmtiThreadState::enqueue_event() being added. Shouldn't NOP implementations also have been done for them?

thanks,

Chris

On 1/6/20 6:35 PM, Chris Plummer wrote:
Hold your horses. I have questions. Working on them now. Please don't push.

thanks,

Chris

On 1/6/20 6:29 PM, [email protected] wrote:
This looks trivial.  Thank you for fixing it.
Coleen

On 1/6/20 9:18 PM, [email protected] wrote:
Please, review a trivial fix for bug:
  https://bugs.openjdk.java.net/browse/JDK-8236124

Patch suggested by A. Shipilev:

diff --git a/src/hotspot/share/code/nmethod.cpp b/src/hotspot/share/code/nmethod.cpp
--- a/src/hotspot/share/code/nmethod.cpp
+++ b/src/hotspot/share/code/nmethod.cpp
@@ -1601,7 +1601,7 @@
       ServiceThread::enqueue_deferred_event(&event);
     } else {
       // This enters the nmethod barrier outside in the caller.
-      state->enqueue_event(&event);
+      JVMTI_ONLY(state->enqueue_event(&event));
     }
   }
 }


Summary:
  The slowdebug build was broken by the fix of JDK-8212160 which introduced new function: enqueue_event().
  The fix is to call it only if the JVM TI is enabled.

Testing:
  Ran slowdebug build locally.

Thanks,
Serguei






Reply via email to