On 1/7/20 2:25 PM, [email protected] wrote:
Chris,
The macro NOT_JVMTI_RETURN is never used outside of the prims/ folder.
Also, there is more work to get rid of the JVMTI code in the
ServiceThread.
I don't know how important is this for the minimal build.
So, I'd leave it alone for now and just fix the build issue.
I think minimal vm should to exclude code at a larger granularity than
this, ie. jvmtiThreadState.cpp isn't compiled at all. We want to avoid
as many #ifdef INCLUDE_JVMTI as possible. I don't think you should add
it to serviceThread.cpp.
thanks,
Coleen
Thanks,
Serguei
On 1/7/20 10:04 AM, Chris Plummer wrote:
Hi Serguei,
The reason you don't see a build failure is because the
implementation of ServiceThread::enqueue_deferred_event() is not in a
JVMTI file that gets excluded from minimalVM builds, thus it is still
callable and linkable. If you choose to use NOT_JVMTI_RETURN for it
in the header file, then you will also need to #ifdef around the
implementation.
Chris
On 1/7/20 9:16 AM, [email protected] wrote:
Hi Chris,
The slowdebug minimal build does not fail without NOT_JVMTI_RETURN
in the ServiceThread::enqueue_deferred_event().
I'm curious why and will check if it is really needed.
If so, will add it as well.
Thanks,
Serguei
On 1/6/20 8:05 PM, [email protected] wrote:
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
