On 1/7/20 11:45 AM, [email protected] wrote:
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.
I've checked with Chris, and he is also okay with this.
So, I'll push the latest (2nd) patch.
Thanks,
Serguei
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
