Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8265489: Stress test times out because of long ObjectSynchronizer::mo…
…nitors_iterate(...) operation

Backport-of: a5e4def526697d88ff31a5fdb41d823b899372f2
  • Loading branch information
GoeLin committed Nov 3, 2022
1 parent 092be50 commit f2a10fd
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 47 deletions.
54 changes: 26 additions & 28 deletions src/hotspot/share/prims/jvmtiEnvBase.cpp
Expand Up @@ -718,8 +718,8 @@ JvmtiEnvBase::get_owned_monitors(JavaThread *calling_thread, JavaThread* java_th
}

// Get off stack monitors. (e.g. acquired via jni MonitorEnter).
JvmtiMonitorClosure jmc(java_thread, calling_thread, owned_monitors_list, this);
ObjectSynchronizer::monitors_iterate(&jmc);
JvmtiMonitorClosure jmc(calling_thread, owned_monitors_list, this);
ObjectSynchronizer::monitors_iterate(&jmc, java_thread);
err = jmc.error();

return err;
Expand Down Expand Up @@ -1454,34 +1454,32 @@ JvmtiMonitorClosure::do_monitor(ObjectMonitor* mon) {
// to the list.
return;
}
if (mon->owner() == _java_thread ) {
// Filter out on stack monitors collected during stack walk.
oop obj = mon->object();
bool found = false;
for (int j = 0; j < _owned_monitors_list->length(); j++) {
jobject jobj = ((jvmtiMonitorStackDepthInfo*)_owned_monitors_list->at(j))->monitor;
oop check = JNIHandles::resolve(jobj);
if (check == obj) {
// On stack monitor already collected during the stack walk.
found = true;
break;
}
// Filter out on stack monitors collected during stack walk.
oop obj = mon->object();
bool found = false;
for (int j = 0; j < _owned_monitors_list->length(); j++) {
jobject jobj = ((jvmtiMonitorStackDepthInfo*)_owned_monitors_list->at(j))->monitor;
oop check = JNIHandles::resolve(jobj);
if (check == obj) {
// On stack monitor already collected during the stack walk.
found = true;
break;
}
if (found == false) {
// This is off stack monitor (e.g. acquired via jni MonitorEnter).
jvmtiError err;
jvmtiMonitorStackDepthInfo *jmsdi;
err = _env->allocate(sizeof(jvmtiMonitorStackDepthInfo), (unsigned char **)&jmsdi);
if (err != JVMTI_ERROR_NONE) {
_error = err;
return;
}
Handle hobj(Thread::current(), obj);
jmsdi->monitor = _env->jni_reference(_calling_thread, hobj);
// stack depth is unknown for this monitor.
jmsdi->stack_depth = -1;
_owned_monitors_list->append(jmsdi);
}
if (found == false) {
// This is off stack monitor (e.g. acquired via jni MonitorEnter).
jvmtiError err;
jvmtiMonitorStackDepthInfo *jmsdi;
err = _env->allocate(sizeof(jvmtiMonitorStackDepthInfo), (unsigned char **)&jmsdi);
if (err != JVMTI_ERROR_NONE) {
_error = err;
return;
}
Handle hobj(Thread::current(), obj);
jmsdi->monitor = _env->jni_reference(_calling_thread, hobj);
// stack depth is unknown for this monitor.
jmsdi->stack_depth = -1;
_owned_monitors_list->append(jmsdi);
}
}

Expand Down
4 changes: 1 addition & 3 deletions src/hotspot/share/prims/jvmtiEnvBase.hpp
Expand Up @@ -639,17 +639,15 @@ class ResourceTracker : public StackObj {
// Jvmti monitor closure to collect off stack monitors.
class JvmtiMonitorClosure: public MonitorClosure {
private:
JavaThread *_java_thread;
JavaThread *_calling_thread;
GrowableArray<jvmtiMonitorStackDepthInfo*> *_owned_monitors_list;
jvmtiError _error;
JvmtiEnvBase *_env;

public:
JvmtiMonitorClosure(JavaThread* thread, JavaThread *calling_thread,
JvmtiMonitorClosure(JavaThread *calling_thread,
GrowableArray<jvmtiMonitorStackDepthInfo*> *owned_monitors,
JvmtiEnvBase *env) {
_java_thread = thread;
_calling_thread = calling_thread;
_owned_monitors_list = owned_monitors;
_error = JVMTI_ERROR_NONE;
Expand Down
11 changes: 6 additions & 5 deletions src/hotspot/share/runtime/synchronizer.cpp
Expand Up @@ -1056,10 +1056,13 @@ JavaThread* ObjectSynchronizer::get_lock_owner(ThreadsList * t_list, Handle h_ob

// Visitors ...

void ObjectSynchronizer::monitors_iterate(MonitorClosure* closure) {
void ObjectSynchronizer::monitors_iterate(MonitorClosure* closure, JavaThread* thread) {
MonitorList::Iterator iter = _in_use_list.iterator();
while (iter.has_next()) {
ObjectMonitor* mid = iter.next();
if (mid->owner() != thread) {
continue;
}
if (!mid->is_being_async_deflated() && mid->object_peek() != NULL) {
// Only process with closure if the object is set.

Expand Down Expand Up @@ -1547,9 +1550,7 @@ class ReleaseJavaMonitorsClosure: public MonitorClosure {
public:
ReleaseJavaMonitorsClosure(JavaThread* thread) : _thread(thread) {}
void do_monitor(ObjectMonitor* mid) {
if (mid->owner() == _thread) {
(void)mid->complete_exit(_thread);
}
(void)mid->complete_exit(_thread);
}
};

Expand All @@ -1572,7 +1573,7 @@ void ObjectSynchronizer::release_monitors_owned_by_thread(JavaThread* current) {
assert(current == JavaThread::current(), "must be current Java thread");
NoSafepointVerifier nsv;
ReleaseJavaMonitorsClosure rjmc(current);
ObjectSynchronizer::monitors_iterate(&rjmc);
ObjectSynchronizer::monitors_iterate(&rjmc, current);
assert(!current->has_pending_exception(), "Should not be possible");
current->clear_pending_exception();
}
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/runtime/synchronizer.hpp
Expand Up @@ -133,7 +133,7 @@ class ObjectSynchronizer : AllStatic {

// JNI detach support
static void release_monitors_owned_by_thread(JavaThread* current);
static void monitors_iterate(MonitorClosure* m);
static void monitors_iterate(MonitorClosure* m, JavaThread* thread);

// Initialize the gInflationLocks
static void initialize();
Expand Down
16 changes: 6 additions & 10 deletions src/hotspot/share/services/threadService.cpp
Expand Up @@ -622,18 +622,14 @@ void StackFrameInfo::print_on(outputStream* st) const {
class InflatedMonitorsClosure: public MonitorClosure {
private:
ThreadStackTrace* _stack_trace;
Thread* _thread;
public:
InflatedMonitorsClosure(Thread* t, ThreadStackTrace* st) {
_thread = t;
InflatedMonitorsClosure(ThreadStackTrace* st) {
_stack_trace = st;
}
void do_monitor(ObjectMonitor* mid) {
if (mid->owner() == _thread) {
oop object = mid->object();
if (!_stack_trace->is_owned_monitor_on_stack(object)) {
_stack_trace->add_jni_locked_monitor(object);
}
oop object = mid->object();
if (!_stack_trace->is_owned_monitor_on_stack(object)) {
_stack_trace->add_jni_locked_monitor(object);
}
}
};
Expand Down Expand Up @@ -692,8 +688,8 @@ void ThreadStackTrace::dump_stack_at_safepoint(int maxDepth) {
if (_with_locked_monitors) {
// Iterate inflated monitors and find monitors locked by this thread
// not found in the stack
InflatedMonitorsClosure imc(_thread, this);
ObjectSynchronizer::monitors_iterate(&imc);
InflatedMonitorsClosure imc(this);
ObjectSynchronizer::monitors_iterate(&imc, _thread);
}
}

Expand Down

1 comment on commit f2a10fd

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.