From 422e8094f0837516dbc40380ca72d30cff4b9c5e Mon Sep 17 00:00:00 2001 From: never Date: Sun, 29 Jan 2012 16:46:04 -0800 Subject: [PATCH] 7129164: JNI Get/ReleasePrimitiveArrayCritical doesn't scale Reviewed-by: kvn, iveresov, dholmes --- src/share/vm/memory/gcLocker.cpp | 89 ++++++++++++++++---- src/share/vm/memory/gcLocker.hpp | 103 ++++++++++++++---------- src/share/vm/memory/gcLocker.inline.hpp | 35 ++++---- src/share/vm/runtime/safepoint.cpp | 22 ++++- src/share/vm/runtime/safepoint.hpp | 9 ++- src/share/vm/runtime/thread.hpp | 3 +- 6 files changed, 181 insertions(+), 80 deletions(-) diff --git a/src/share/vm/memory/gcLocker.cpp b/src/share/vm/memory/gcLocker.cpp index db0a79d4d..8c7ae3bad 100644 --- a/src/share/vm/memory/gcLocker.cpp +++ b/src/share/vm/memory/gcLocker.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -31,38 +31,93 @@ volatile jint GC_locker::_jni_lock_count = 0; volatile jint GC_locker::_lock_count = 0; volatile bool GC_locker::_needs_gc = false; volatile bool GC_locker::_doing_gc = false; +jlong GC_locker::_wait_begin = 0; + +#ifdef ASSERT +volatile jint GC_locker::_debug_jni_lock_count = 0; +#endif + + +#ifdef ASSERT +void GC_locker::verify_critical_count() { + if (SafepointSynchronize::is_at_safepoint()) { + assert(!needs_gc() || _debug_jni_lock_count == _jni_lock_count, "must agree"); + int count = 0; + // Count the number of threads with critical operations in progress + for (JavaThread* thr = Threads::first(); thr; thr = thr->next()) { + if (thr->in_critical()) { + count++; + } + } + if (_jni_lock_count != count) { + tty->print_cr("critical counts don't match: %d != %d", _jni_lock_count, count); + for (JavaThread* thr = Threads::first(); thr; thr = thr->next()) { + if (thr->in_critical()) { + tty->print_cr(INTPTR_FORMAT " in_critical %d", thr, thr->in_critical()); + } + } + } + assert(_jni_lock_count == count, "must be equal"); + } +} +#endif + +bool GC_locker::check_active_before_gc() { + assert(SafepointSynchronize::is_at_safepoint(), "only read at safepoint"); + if (is_active() && !_needs_gc) { + verify_critical_count(); + _needs_gc = true; + if (PrintJNIGCStalls && PrintGCDetails) { + ResourceMark rm; // JavaThread::name() allocates to convert to UTF8 + _wait_begin = tty->time_stamp().milliseconds(); + gclog_or_tty->print_cr(INT64_FORMAT ": Setting _needs_gc. Thread \"%s\" %d locked.", + _wait_begin, Thread::current()->name(), _jni_lock_count); + } + + } + return is_active(); +} void GC_locker::stall_until_clear() { assert(!JavaThread::current()->in_critical(), "Would deadlock"); - if (PrintJNIGCStalls && PrintGCDetails) { - ResourceMark rm; // JavaThread::name() allocates to convert to UTF8 - gclog_or_tty->print_cr( - "Allocation failed. Thread \"%s\" is stalled by JNI critical section.", - JavaThread::current()->name()); - } MutexLocker ml(JNICritical_lock); + + if (needs_gc()) { + if (PrintJNIGCStalls && PrintGCDetails) { + ResourceMark rm; // JavaThread::name() allocates to convert to UTF8 + gclog_or_tty->print_cr(INT64_FORMAT ": Allocation failed. Thread \"%s\" is stalled by JNI critical section, %d locked.", + tty->time_stamp().milliseconds() - _wait_begin, Thread::current()->name(), _jni_lock_count); + } + } + // Wait for _needs_gc to be cleared - while (GC_locker::needs_gc()) { + while (needs_gc()) { JNICritical_lock->wait(); } } -void GC_locker::jni_lock_slow() { +void GC_locker::jni_lock(JavaThread* thread) { + assert(!thread->in_critical(), "shouldn't currently be in a critical region"); MutexLocker mu(JNICritical_lock); // Block entering threads if we know at least one thread is in a // JNI critical region and we need a GC. // We check that at least one thread is in a critical region before // blocking because blocked threads are woken up by a thread exiting // a JNI critical region. - while ((is_jni_active() && needs_gc()) || _doing_gc) { + while ((needs_gc() && is_jni_active()) || _doing_gc) { JNICritical_lock->wait(); } - jni_lock(); + thread->enter_critical(); + _jni_lock_count++; + increment_debug_jni_lock_count(); } -void GC_locker::jni_unlock_slow() { +void GC_locker::jni_unlock(JavaThread* thread) { + assert(thread->in_last_critical(), "should be exiting critical region"); MutexLocker mu(JNICritical_lock); - jni_unlock(); + _jni_lock_count--; + decrement_debug_jni_lock_count(); + thread->exit_critical(); if (needs_gc() && !is_jni_active()) { // We're the last thread out. Cause a GC to occur. // GC will also check is_active, so this check is not @@ -74,11 +129,17 @@ void GC_locker::jni_unlock_slow() { { // Must give up the lock while at a safepoint MutexUnlocker munlock(JNICritical_lock); + if (PrintJNIGCStalls && PrintGCDetails) { + ResourceMark rm; // JavaThread::name() allocates to convert to UTF8 + gclog_or_tty->print_cr(INT64_FORMAT ": Thread \"%s\" is performing GC after exiting critical section, %d locked", + tty->time_stamp().milliseconds() - _wait_begin, Thread::current()->name(), _jni_lock_count); + } Universe::heap()->collect(GCCause::_gc_locker); } _doing_gc = false; } - clear_needs_gc(); + + _needs_gc = false; JNICritical_lock->notify_all(); } } diff --git a/src/share/vm/memory/gcLocker.hpp b/src/share/vm/memory/gcLocker.hpp index c9d913abf..9fb5a7e60 100644 --- a/src/share/vm/memory/gcLocker.hpp +++ b/src/share/vm/memory/gcLocker.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -51,53 +51,70 @@ class GC_locker: public AllStatic { private: - static volatile jint _jni_lock_count; // number of jni active instances + // The _jni_lock_count keeps track of the number of threads that are + // currently in a critical region. It's only kept up to date when + // _needs_gc is true. The current value is computed during + // safepointing and decremented during the slow path of GC_locker + // unlocking. + static volatile jint _jni_lock_count; // number of jni active instances. + static volatile jint _lock_count; // number of other active instances static volatile bool _needs_gc; // heap is filling, we need a GC // note: bool is typedef'd as jint static volatile bool _doing_gc; // unlock_critical() is doing a GC + static jlong _wait_begin; // Timestamp for the setting of _needs_gc. + // Used only by printing code. + +#ifdef ASSERT + // This lock count is updated for all operations and is used to + // validate the jni_lock_count that is computed during safepoints. + static volatile jint _debug_jni_lock_count; +#endif + // Accessors static bool is_jni_active() { + assert(_needs_gc, "only valid when _needs_gc is set"); return _jni_lock_count > 0; } - static void set_needs_gc() { - assert(SafepointSynchronize::is_at_safepoint(), - "needs_gc is only set at a safepoint"); - _needs_gc = true; - } - - static void clear_needs_gc() { - assert_lock_strong(JNICritical_lock); - _needs_gc = false; - } - - static void jni_lock() { - Atomic::inc(&_jni_lock_count); - CHECK_UNHANDLED_OOPS_ONLY( - if (CheckUnhandledOops) { Thread::current()->_gc_locked_out_count++; }) - assert(Universe::heap() == NULL || !Universe::heap()->is_gc_active(), - "locking failed"); - } - - static void jni_unlock() { - Atomic::dec(&_jni_lock_count); - CHECK_UNHANDLED_OOPS_ONLY( - if (CheckUnhandledOops) { Thread::current()->_gc_locked_out_count--; }) - } + // At a safepoint, visit all threads and count the number of active + // critical sections. This is used to ensure that all active + // critical sections are exited before a new one is started. + static void verify_critical_count() NOT_DEBUG_RETURN; - static void jni_lock_slow(); - static void jni_unlock_slow(); + static void jni_lock(JavaThread* thread); + static void jni_unlock(JavaThread* thread); public: // Accessors static bool is_active(); static bool needs_gc() { return _needs_gc; } + // Shorthand - static bool is_active_and_needs_gc() { return is_active() && needs_gc();} + static bool is_active_and_needs_gc() { return needs_gc() && is_active(); } + + // In debug mode track the locking state at all times + static void increment_debug_jni_lock_count() { +#ifdef ASSERT + assert(_debug_jni_lock_count >= 0, "bad value"); + Atomic::inc(&_debug_jni_lock_count); +#endif + } + static void decrement_debug_jni_lock_count() { +#ifdef ASSERT + assert(_debug_jni_lock_count > 0, "bad value"); + Atomic::dec(&_debug_jni_lock_count); +#endif + } + + // Set the current lock count + static void set_jni_lock_count(int count) { + _jni_lock_count = count; + verify_critical_count(); + } - // Calls set_needs_gc() if is_active() is true. Returns is_active(). + // Sets _needs_gc if is_active() is true. Returns is_active(). static bool check_active_before_gc(); // Stalls the caller (who should not be in a jni critical section) @@ -131,20 +148,20 @@ class GC_locker: public AllStatic { // JNI critical regions are the only participants in this scheme // because they are, by spec, well bounded while in a critical region. // - // Each of the following two method is split into a fast path and a slow - // path. JNICritical_lock is only grabbed in the slow path. + // Each of the following two method is split into a fast path and a + // slow path. JNICritical_lock is only grabbed in the slow path. // _needs_gc is initially false and every java thread will go - // through the fast path (which does the same thing as the slow path - // when _needs_gc is false). When GC happens at a safepoint, - // GC_locker::is_active() is checked. Since there is no safepoint in the - // fast path of lock_critical() and unlock_critical(), there is no race - // condition between the fast path and GC. After _needs_gc is set at a - // safepoint, every thread will go through the slow path after the safepoint. - // Since after a safepoint, each of the following two methods is either - // entered from the method entry and falls into the slow path, or is - // resumed from the safepoints in the method, which only exist in the slow - // path. So when _needs_gc is set, the slow path is always taken, till - // _needs_gc is cleared. + // through the fast path, which simply increments or decrements the + // current thread's critical count. When GC happens at a safepoint, + // GC_locker::is_active() is checked. Since there is no safepoint in + // the fast path of lock_critical() and unlock_critical(), there is + // no race condition between the fast path and GC. After _needs_gc + // is set at a safepoint, every thread will go through the slow path + // after the safepoint. Since after a safepoint, each of the + // following two methods is either entered from the method entry and + // falls into the slow path, or is resumed from the safepoints in + // the method, which only exist in the slow path. So when _needs_gc + // is set, the slow path is always taken, till _needs_gc is cleared. static void lock_critical(JavaThread* thread); static void unlock_critical(JavaThread* thread); }; diff --git a/src/share/vm/memory/gcLocker.inline.hpp b/src/share/vm/memory/gcLocker.inline.hpp index 5c63e077b..64a04bffd 100644 --- a/src/share/vm/memory/gcLocker.inline.hpp +++ b/src/share/vm/memory/gcLocker.inline.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2012, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -28,16 +28,11 @@ #include "memory/gcLocker.hpp" inline bool GC_locker::is_active() { + assert(_needs_gc || SafepointSynchronize::is_at_safepoint(), "only read at safepoint"); + verify_critical_count(); return _lock_count > 0 || _jni_lock_count > 0; } -inline bool GC_locker::check_active_before_gc() { - if (is_active()) { - set_needs_gc(); - } - return is_active(); -} - inline void GC_locker::lock() { // cast away volatile Atomic::inc(&_lock_count); @@ -56,24 +51,28 @@ inline void GC_locker::unlock() { inline void GC_locker::lock_critical(JavaThread* thread) { if (!thread->in_critical()) { - if (!needs_gc()) { - jni_lock(); - } else { - jni_lock_slow(); + if (needs_gc()) { + // jni_lock call calls enter_critical under the lock so that the + // global lock count and per thread count are in agreement. + jni_lock(thread); + return; } + increment_debug_jni_lock_count(); } thread->enter_critical(); } inline void GC_locker::unlock_critical(JavaThread* thread) { - thread->exit_critical(); - if (!thread->in_critical()) { - if (!needs_gc()) { - jni_unlock(); - } else { - jni_unlock_slow(); + if (thread->in_last_critical()) { + if (needs_gc()) { + // jni_unlock call calls exit_critical under the lock so that + // the global lock count and per thread count are in agreement. + jni_unlock(thread); + return; } + decrement_debug_jni_lock_count(); } + thread->exit_critical(); } #endif // SHARE_VM_MEMORY_GCLOCKER_INLINE_HPP diff --git a/src/share/vm/runtime/safepoint.cpp b/src/share/vm/runtime/safepoint.cpp index d33893120..9146c20a2 100644 --- a/src/share/vm/runtime/safepoint.cpp +++ b/src/share/vm/runtime/safepoint.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -95,6 +95,7 @@ SafepointSynchronize::SynchronizeState volatile SafepointSynchronize::_state = SafepointSynchronize::_not_synchronized; volatile int SafepointSynchronize::_waiting_to_block = 0; volatile int SafepointSynchronize::_safepoint_counter = 0; +int SafepointSynchronize::_current_jni_active_count = 0; long SafepointSynchronize::_end_of_last_safepoint = 0; static volatile int PageArmed = 0 ; // safepoint polling page is RO|RW vs PROT_NONE static volatile int TryingToBlock = 0 ; // proximate value -- for advisory use only @@ -138,6 +139,9 @@ void SafepointSynchronize::begin() { { MutexLocker mu(Safepoint_lock); + // Reset the count of active JNI critical threads + _current_jni_active_count = 0; + // Set number of threads to wait for, before we initiate the callbacks _waiting_to_block = nof_threads; TryingToBlock = 0 ; @@ -375,6 +379,9 @@ void SafepointSynchronize::begin() { OrderAccess::fence(); + // Update the count of active JNI critical regions + GC_locker::set_jni_lock_count(_current_jni_active_count); + if (TraceSafepoint) { VM_Operation *op = VMThread::vm_operation(); tty->print_cr("Entering safepoint region: %s", (op != NULL) ? op->name() : "no vm operation"); @@ -585,6 +592,11 @@ void SafepointSynchronize::block(JavaThread *thread) { _waiting_to_block--; thread->safepoint_state()->set_has_called_back(true); + if (thread->in_critical()) { + // Notice that this thread is in a critical section + increment_jni_active_count(); + } + // Consider (_waiting_to_block < 2) to pipeline the wakeup of the VM thread if (_waiting_to_block == 0) { Safepoint_lock->notify_all(); @@ -861,8 +873,12 @@ void ThreadSafepointState::examine_state_of_thread() { // running, but are actually at a safepoint. We will happily // agree and update the safepoint state here. if (SafepointSynchronize::safepoint_safe(_thread, state)) { - roll_forward(_at_safepoint); - return; + roll_forward(_at_safepoint); + if (_thread->in_critical()) { + // Notice that this thread is in a critical section + SafepointSynchronize::increment_jni_active_count(); + } + return; } if (state == _thread_in_vm) { diff --git a/src/share/vm/runtime/safepoint.hpp b/src/share/vm/runtime/safepoint.hpp index 550715524..0456de4d3 100644 --- a/src/share/vm/runtime/safepoint.hpp +++ b/src/share/vm/runtime/safepoint.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2010, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -29,6 +29,7 @@ #include "code/nmethod.hpp" #include "memory/allocation.hpp" #include "runtime/extendedPC.hpp" +#include "runtime/mutexLocker.hpp" #include "runtime/os.hpp" #include "utilities/ostream.hpp" @@ -92,6 +93,7 @@ class SafepointSynchronize : AllStatic { private: static volatile SynchronizeState _state; // Threads might read this flag directly, without acquireing the Threads_lock static volatile int _waiting_to_block; // number of threads we are waiting for to block + static int _current_jni_active_count; // Counts the number of active critical natives during the safepoint // This counter is used for fast versions of jni_GetField. // An even value means there is no ongoing safepoint operations. @@ -146,6 +148,11 @@ public: return (_state != _not_synchronized); } + inline static void increment_jni_active_count() { + assert_locked_or_safepoint(Safepoint_lock); + _current_jni_active_count++; + } + // Called when a thread volantary blocks static void block(JavaThread *thread); static void signal_thread_at_safepoint() { _waiting_to_block--; } diff --git a/src/share/vm/runtime/thread.hpp b/src/share/vm/runtime/thread.hpp index 37bbf2937..6f09e0846 100644 --- a/src/share/vm/runtime/thread.hpp +++ b/src/share/vm/runtime/thread.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2011, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -1310,6 +1310,7 @@ class JavaThread: public Thread { // JNI critical regions. These can nest. bool in_critical() { return _jni_active_critical > 0; } + bool in_last_critical() { return _jni_active_critical == 1; } void enter_critical() { assert(Thread::current() == this, "this must be current thread"); _jni_active_critical++; } -- GitLab