Skip to content

Commit

Permalink
8325587: Shenandoah: ShenandoahLock should allow blocking in VM
Browse files Browse the repository at this point in the history
Reviewed-by: rehn, rkennke
  • Loading branch information
shipilev committed Feb 21, 2024
1 parent 5f16f34 commit 492e8bf
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 19 deletions.
6 changes: 5 additions & 1 deletion src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,11 @@ HeapWord* ShenandoahHeap::allocate_memory(ShenandoahAllocRequest& req) {
}

HeapWord* ShenandoahHeap::allocate_memory_under_lock(ShenandoahAllocRequest& req, bool& in_new_region) {
ShenandoahHeapLocker locker(lock());
// If we are dealing with mutator allocation, then we may need to block for safepoint.
// We cannot block for safepoint for GC allocations, because there is a high chance
// we are already running at safepoint or from stack watermark machinery, and we cannot
// block again.
ShenandoahHeapLocker locker(lock(), req.is_mutator_alloc());
return _free_set->allocate(req, in_new_region);
}

Expand Down
38 changes: 38 additions & 0 deletions src/hotspot/share/gc/shenandoah/shenandoahLock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,47 @@

#include "gc/shenandoah/shenandoahLock.hpp"
#include "runtime/atomic.hpp"
#include "runtime/interfaceSupport.inline.hpp"
#include "runtime/javaThread.hpp"
#include "runtime/os.inline.hpp"

// These are inline variants of Thread::SpinAcquire with optional blocking in VM.

class ShenandoahNoBlockOp : public StackObj {
public:
ShenandoahNoBlockOp(JavaThread* java_thread) {
assert(java_thread == nullptr, "Should not pass anything");
}
};

void ShenandoahLock::contended_lock(bool allow_block_for_safepoint) {
Thread* thread = Thread::current();
if (allow_block_for_safepoint && thread->is_Java_thread()) {
contended_lock_internal<ThreadBlockInVM>(JavaThread::cast(thread));
} else {
contended_lock_internal<ShenandoahNoBlockOp>(nullptr);
}
}

template<typename BlockOp>
void ShenandoahLock::contended_lock_internal(JavaThread* java_thread) {
int ctr = 0;
int yields = 0;
while (Atomic::cmpxchg(&_state, unlocked, locked) != unlocked) {
if ((++ctr & 0xFFF) == 0) {
BlockOp block(java_thread);
if (yields > 5) {
os::naked_short_sleep(1);
} else {
os::naked_yield();
yields++;
}
} else {
SpinPause();
}
}
}

ShenandoahSimpleLock::ShenandoahSimpleLock() {
assert(os::mutex_init_done(), "Too early!");
}
Expand Down
41 changes: 23 additions & 18 deletions src/hotspot/share/gc/shenandoah/shenandoahLock.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,34 +35,39 @@ class ShenandoahLock {
enum LockState { unlocked = 0, locked = 1 };

shenandoah_padding(0);
volatile int _state;
volatile LockState _state;
shenandoah_padding(1);
volatile Thread* _owner;
shenandoah_padding(2);

template<typename BlockOp>
void contended_lock_internal(JavaThread* java_thread);

public:
ShenandoahLock() : _state(unlocked), _owner(nullptr) {};

void lock() {
#ifdef ASSERT
assert(_owner != Thread::current(), "reentrant locking attempt, would deadlock");
#endif
Thread::SpinAcquire(&_state, "Shenandoah Heap Lock");
#ifdef ASSERT
assert(_state == locked, "must be locked");
assert(_owner == nullptr, "must not be owned");
_owner = Thread::current();
#endif
void lock(bool allow_block_for_safepoint) {
assert(Atomic::load(&_owner) != Thread::current(), "reentrant locking attempt, would deadlock");

// Try to lock fast, or dive into contended lock handling.
if (Atomic::cmpxchg(&_state, unlocked, locked) != unlocked) {
contended_lock(allow_block_for_safepoint);
}

assert(Atomic::load(&_state) == locked, "must be locked");
assert(Atomic::load(&_owner) == nullptr, "must not be owned");
DEBUG_ONLY(Atomic::store(&_owner, Thread::current());)
}

void unlock() {
#ifdef ASSERT
assert (_owner == Thread::current(), "sanity");
_owner = nullptr;
#endif
Thread::SpinRelease(&_state);
assert(Atomic::load(&_owner) == Thread::current(), "sanity");
DEBUG_ONLY(Atomic::store(&_owner, (Thread*)nullptr);)
OrderAccess::fence();
Atomic::store(&_state, unlocked);
}

void contended_lock(bool allow_block_for_safepoint);

bool owned_by_self() {
#ifdef ASSERT
return _state == locked && _owner == Thread::current();
Expand All @@ -77,9 +82,9 @@ class ShenandoahLocker : public StackObj {
private:
ShenandoahLock* const _lock;
public:
ShenandoahLocker(ShenandoahLock* lock) : _lock(lock) {
ShenandoahLocker(ShenandoahLock* lock, bool allow_block_for_safepoint = false) : _lock(lock) {
if (_lock != nullptr) {
_lock->lock();
_lock->lock(allow_block_for_safepoint);
}
}

Expand Down

3 comments on commit 492e8bf

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@pengxiaolong
Copy link

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 492e8bf Jun 28, 2024

Choose a reason for hiding this comment

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

@pengxiaolong Could not automatically backport 492e8bf5 to openjdk/jdk17u-dev due to conflicts in the following files:

  • src/hotspot/share/gc/shenandoah/shenandoahLock.cpp
  • src/hotspot/share/gc/shenandoah/shenandoahLock.hpp

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk17u-dev. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk17u-dev.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b backport-pengxiaolong-492e8bf5-master

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git 492e8bf563135d27b46fde198880e62d5f1940e8

# Backport the commit
$ git cherry-pick --no-commit 492e8bf563135d27b46fde198880e62d5f1940e8
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 492e8bf563135d27b46fde198880e62d5f1940e8'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk17u-dev with the title Backport 492e8bf563135d27b46fde198880e62d5f1940e8.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 492e8bf5 from the openjdk/jdk repository.

The commit being backported was authored by Aleksey Shipilev on 21 Feb 2024 and was reviewed by Robbin Ehn and Roman Kennke.

Thanks!

Please sign in to comment.