Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Minimal VM build failures #67

Closed
wants to merge 4 commits into from
+53 −10
Closed

Fix Minimal VM build failures #67

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
57cd571
Fix
shipilev Sep 27, 2021
3ab4f12
Cleaner disabling
shipilev Sep 28, 2021
9ba55ed
INCLUDE_JVMTI all Thread methods
shipilev Sep 28, 2021
c78adb0
Remove excessive guard
shipilev Sep 29, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/hotspot/share/prims/jvm.cpp
View file
Original file line number Diff line number Diff line change
@@ -3888,10 +3888,15 @@ JVM_ENTRY_NO_ENV(jint, JVM_FindSignal(const char *name))
JVM_END

JVM_ENTRY(void, JVM_VirtualThreadMountBegin(JNIEnv* env, jobject vthread, jboolean first_mount))
#if INCLUDE_JVMTI
JvmtiVTMTDisabler::start_VTMT(vthread, 0);
#else
fatal("Should only be called with JVMTI enabled");
#endif
JVM_END

JVM_ENTRY(void, JVM_VirtualThreadMountEnd(JNIEnv* env, jobject vthread, jboolean first_mount))
#if INCLUDE_JVMTI
assert(thread->is_in_VTMT(), "VTMT sanity check");
JvmtiVTMTDisabler::finish_VTMT(vthread, 0);
oop vt = JNIHandles::resolve(vthread);
@@ -3913,9 +3918,13 @@ JVM_ENTRY(void, JVM_VirtualThreadMountEnd(JNIEnv* env, jobject vthread, jboolean
if (JvmtiExport::should_post_vthread_mount()) {
JvmtiExport::post_vthread_mount(vthread);
}
#else
fatal("Should only be called with JVMTI enabled");
#endif
JVM_END

JVM_ENTRY(void, JVM_VirtualThreadUnmountBegin(JNIEnv* env, jobject vthread, jboolean last_unmount))
#if INCLUDE_JVMTI
HandleMark hm(thread);
Handle ct(thread, thread->threadObj());

@@ -3938,9 +3947,16 @@ JVM_ENTRY(void, JVM_VirtualThreadUnmountBegin(JNIEnv* env, jobject vthread, jboo

assert(!thread->is_in_VTMT(), "VTMT sanity check");
JvmtiVTMTDisabler::start_VTMT(vthread, 1);
#else
fatal("Should only be called with JVMTI enabled");
#endif
JVM_END

JVM_ENTRY(void, JVM_VirtualThreadUnmountEnd(JNIEnv* env, jobject vthread, jboolean last_unmount))
#if INCLUDE_JVMTI
assert(thread->is_in_VTMT(), "VTMT sanity check");
JvmtiVTMTDisabler::finish_VTMT(vthread, 1);
#else
fatal("Should only be called with JVMTI enabled");
#endif
JVM_END
9 changes: 9 additions & 0 deletions src/hotspot/share/prims/jvmtiThreadState.cpp
View file
Original file line number Diff line number Diff line change
@@ -786,3 +786,12 @@ void JvmtiThreadState::set_thread(JavaThread* thread) {
}
_thread = thread;
}

int JvmtiVTMTDisabler::VTMT_disable_count() {
return _VTMT_disable_count;
}

int JvmtiVTMTDisabler::VTMT_count() {
return _VTMT_count;
}

4 changes: 2 additions & 2 deletions src/hotspot/share/prims/jvmtiThreadState.hpp
View file
Original file line number Diff line number Diff line change
@@ -95,8 +95,8 @@ class JvmtiVTMTDisabler {
void set_self_suspend() { _self_suspend = true; }
static void start_VTMT(jthread vthread, int callsite_tag);
static void finish_VTMT(jthread vthread, int callsite_tag);
static int VTMT_disable_count() { return _VTMT_disable_count; };
static int VTMT_count() { return _VTMT_count; }
static int VTMT_disable_count();
static int VTMT_count();
Comment on lines +98 to +99
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what is being fixed here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As stated in PR: "jvmtiExport and jvmtiThreadState headers are usually included even with JVMTI turned off, but .cpp would not be compiled without JVMTI. Therefore, non-trivial implementations should go into .cpp." You cannot put the accessor bodies here, because the _VTMT_count and _VTMT_disable_count symbols would be inaccessible without .cpp compiled, and you'll get linker errors.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of other choices here. One is like the following example:

  inline static bool should_post_class_file_load_hook()           {
    JVMTI_ONLY(return _should_post_class_file_load_hook);
    NOT_JVMTI(return false;)
  }

However I don't think anyone should actually be calling any VTMT code when JVMTI is disabled. So you could instead put the entire thing (including declaration) in a #ifdef INCLUDE_JVMTI. You would then need to also #ifdef around the callers, or use JVMTI_ONLY. If there are many, then go with my first suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why we should bother. These methods are only used in asserts, AFAICS, so the performance here is irrelevant. Moving these definitions out of .hpp matches the rest of the JvmtiVTMTDisabler. There are no other uses of INCLUDE_JVMTI in jvmtiThreadState.hpp, as it is assumed, I think, that you could include it without JVMTI enabled. If JVMTI is disabled in build time, using these methods would produce linker errors. Sorry, but I disagree with these suggestions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they are only used in asserts, then shouldn't they have a #ifdef ASSERTS around them? And there are implicit #ifdef INCLUDE_JVMTI uses in jvmtiThreadState.hpp:

  void oops_do(OopClosure* f, CodeBlobClosure* cf) NOT_JVMTI_RETURN; // GC support
  void nmethods_do(CodeBlobClosure* cf) NOT_JVMTI_RETURN;

};

///////////////////////////////////////////////////////////////
3 changes: 0 additions & 3 deletions src/hotspot/share/runtime/continuation.cpp
View file
Original file line number Diff line number Diff line change
@@ -24,7 +24,6 @@

#include "precompiled.hpp"
#include "classfile/javaClasses.hpp"
#include "gc/g1/g1CollectedHeap.hpp"
#include "gc/shared/gc_globals.hpp"
#include "oops/instanceStackChunkKlass.hpp"
#include "oops/oopsHierarchy.hpp"
@@ -75,8 +74,6 @@
#include "utilities/exceptions.hpp"
#include "utilities/macros.hpp"

#include "gc/g1/g1CollectedHeap.inline.hpp"

#define CONT_JFR false

#define SENDER_SP_RET_ADDRESS_OFFSET (frame::sender_sp_offset - frame::return_addr_offset)
13 changes: 10 additions & 3 deletions src/hotspot/share/runtime/frame.hpp
View file
Original file line number Diff line number Diff line change
@@ -450,9 +450,16 @@ class frame {
int adjust_offset(Method* method, int index); // helper for above fn
public:
// Memory management
void oops_do(OopClosure* f, CodeBlobClosure* cf, const RegisterMap* map) { oops_do_internal(f, cf, NULL, DerivedPointerTable::is_active() ?
DerivedPointerIterationMode::_with_table :
DerivedPointerIterationMode::_ignore, map, true); }
void oops_do(OopClosure* f, CodeBlobClosure* cf, const RegisterMap* map) {
#if COMPILER2_OR_JVMCI
DerivedPointerIterationMode dpim = DerivedPointerTable::is_active() ?
DerivedPointerIterationMode::_with_table :
DerivedPointerIterationMode::_ignore;
#else
DerivedPointerIterationMode dpim = DerivedPointerIterationMode::_ignore;;
#endif
oops_do_internal(f, cf, NULL, dpim, map, true);
}
void oops_do(OopClosure* f, CodeBlobClosure* cf, DerivedOopClosure* df, const RegisterMap* map) { oops_do_internal(f, cf, df, DerivedPointerIterationMode::_ignore, map, true); }
void oops_do(OopClosure* f, CodeBlobClosure* cf, const RegisterMap* map, DerivedPointerIterationMode derived_mode) const { oops_do_internal(f, cf, NULL, derived_mode, map, true); }
void nmethods_do(CodeBlobClosure* cf) const;
4 changes: 2 additions & 2 deletions src/hotspot/share/runtime/handshake.cpp
View file
Original file line number Diff line number Diff line change
@@ -619,7 +619,7 @@ void HandshakeState::do_self_suspend() {
assert(_handshakee->thread_state() == _thread_blocked, "Caller should have transitioned to _thread_blocked");

while (is_suspended_or_blocked()) {
assert(!_handshakee->is_in_VTMT(), "no suspend allowed in VTMT transition");
JVMTI_ONLY(assert(!_handshakee->is_in_VTMT(), "no suspend allowed in VTMT transition");)
log_trace(thread, suspend)("JavaThread:" INTPTR_FORMAT " suspended", p2i(_handshakee));
_lock.wait_without_safepoint_check();
}
@@ -706,7 +706,7 @@ class SuspendThreadHandshake : public HandshakeClosure {
};

bool HandshakeState::suspend() {
assert(!_handshakee->is_in_VTMT(), "no suspend allowed in VTMT transition");
JVMTI_ONLY(assert(!_handshakee->is_in_VTMT(), "no suspend allowed in VTMT transition");)
JavaThread* self = JavaThread::current();
if (_handshakee == self) {
// If target is the current thread we can bypass the handshake machinery
8 changes: 8 additions & 0 deletions src/hotspot/share/runtime/thread.cpp
View file
Original file line number Diff line number Diff line change
@@ -1022,8 +1022,10 @@ JavaThread::JavaThread() :
_in_deopt_handler(0),
_doing_unsafe_access(false),
_do_not_unlock_if_synchronized(false),
#if INCLUDE_JVMTI
_is_in_VTMT(false),
_is_VTMT_disabler(false),
#endif
_jni_attach_state(_not_attaching_via_jni),
#if INCLUDE_JVMCI
_pending_deoptimization(-1),
@@ -1774,6 +1776,7 @@ void JavaThread::send_thread_stop(oop java_throwable) {
this->interrupt();
}

#if INCLUDE_JVMTI
void JavaThread::set_is_in_VTMT(bool val) {
_is_in_VTMT = val;
if (val) {
@@ -1785,6 +1788,7 @@ void JavaThread::set_is_VTMT_disabler(bool val) {
_is_VTMT_disabler = val;
assert(JvmtiVTMTDisabler::VTMT_count() == 0, "must be 0");
}
#endif

// External suspension mechanism.
//
@@ -1793,9 +1797,11 @@ void JavaThread::set_is_VTMT_disabler(bool val) {
// - Target thread will not enter any new monitors.
//
bool JavaThread::java_suspend() {
#if INCLUDE_JVMTI
// Suspending a JavaThread in VTMT or disabling VTMT can cause deadlocks.
assert(!is_in_VTMT(), "no suspend allowed in VTMT transition");
assert(!is_VTMT_disabler(), "no suspend allowed for VTMT disablers");
#endif

ThreadsListHandle tlh;
if (!tlh.includes(this)) {
@@ -2361,6 +2367,7 @@ void JavaThread::print_stack_on(outputStream* st) {
}
}

#if INCLUDE_JVMTI
// Rebind JVMTI thread state from carrier to virtual or from virtual to carrier.
JvmtiThreadState* JavaThread::rebind_to_jvmti_thread_state_of(oop thread_oop) {
set_mounted_vthread(thread_oop);
@@ -2373,6 +2380,7 @@ JvmtiThreadState* JavaThread::rebind_to_jvmti_thread_state_of(oop thread_oop) {

return jvmti_thread_state();
}
#endif

// JVMTI PopFrame support
void JavaThread::popframe_preserve_args(ByteSize size_in_bytes, void* start) {
6 changes: 6 additions & 0 deletions src/hotspot/share/runtime/thread.hpp
View file
Original file line number Diff line number Diff line change
@@ -926,8 +926,10 @@ class JavaThread: public Thread {
volatile bool _doing_unsafe_access; // Thread may fault due to unsafe access
bool _do_not_unlock_if_synchronized; // Do not unlock the receiver of a synchronized method (since it was
// never locked) when throwing an exception. Used by interpreter only.
#if INCLUDE_JVMTI
bool _is_in_VTMT; // thread is in virtual thread mount transition
bool _is_VTMT_disabler; // thread currently disabled VTMT
#endif

// JNI attach states:
enum JNIAttachStates {
@@ -1247,11 +1249,13 @@ class JavaThread: public Thread {
return (_suspend_flags & _thread_suspended) != 0;
}

#if INCLUDE_JVMTI
bool is_VTMT_disabler() const { return _is_VTMT_disabler; }
bool is_in_VTMT() const { return _is_in_VTMT; }

void set_is_in_VTMT(bool val);
void set_is_VTMT_disabler(bool val);
#endif

bool is_cont_force_yield() { return cont_preempt(); }

@@ -1590,8 +1594,10 @@ class JavaThread: public Thread {
JvmtiThreadState *jvmti_thread_state() const { return _jvmti_thread_state; }
static ByteSize jvmti_thread_state_offset() { return byte_offset_of(JavaThread, _jvmti_thread_state); }

#if INCLUDE_JVMTI
// Rebind JVMTI thread state from carrier to virtual or from virtual to carrier.
JvmtiThreadState *rebind_to_jvmti_thread_state_of(oop thread_oop);
#endif

// JVMTI PopFrame support
// Setting and clearing popframe_condition