Skip to content

Commit

Permalink
8318015: Lock inflation not needed for OSR or Deopt for new locking m…
Browse files Browse the repository at this point in the history
…odes

Reviewed-by: pchilanomate, dlong
  • Loading branch information
TheRealMDoerr committed Oct 17, 2023
1 parent 6aa837e commit d0ea2a5
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 24 deletions.
35 changes: 21 additions & 14 deletions src/hotspot/share/runtime/basicLock.cpp
Expand Up @@ -66,19 +66,26 @@ void BasicLock::move_to(oop obj, BasicLock* dest) {
// is small (given the support for inflated fast-path locking in the fast_lock, etc)
// we'll leave that optimization for another time.

if (displaced_header().is_neutral()) {
// The object is locked and the resulting ObjectMonitor* will also be
// locked so it can't be async deflated until ownership is dropped.
ObjectSynchronizer::inflate_helper(obj);
// WARNING: We cannot put a check here, because the inflation
// will not update the displaced header. Once BasicLock is inflated,
// no one should ever look at its content.
} else {
// Typically the displaced header will be 0 (recursive stack lock) or
// unused_mark. Naively we'd like to assert that the displaced mark
// value is either 0, neutral, or 3. But with the advent of the
// store-before-CAS avoidance in fast_lock/compiler_lock_object
// we can find any flavor mark in the displaced mark.
if (LockingMode == LM_LEGACY) {
if (displaced_header().is_neutral()) {
// The object is locked and the resulting ObjectMonitor* will also be
// locked so it can't be async deflated until ownership is dropped.
ObjectSynchronizer::inflate_helper(obj);
// WARNING: We cannot put a check here, because the inflation
// will not update the displaced header. Once BasicLock is inflated,
// no one should ever look at its content.
} else {
// Typically the displaced header will be 0 (recursive stack lock) or
// unused_mark. Naively we'd like to assert that the displaced mark
// value is either 0, neutral, or 3. But with the advent of the
// store-before-CAS avoidance in fast_lock/compiler_lock_object
// we can find any flavor mark in the displaced mark.
}
dest->set_displaced_header(displaced_header());
}
dest->set_displaced_header(displaced_header());
#ifdef ASSERT
else {
dest->set_displaced_header(markWord(badDispHeaderDeopt));
}
#endif
}
26 changes: 17 additions & 9 deletions src/hotspot/share/runtime/sharedRuntime.cpp
Expand Up @@ -3257,16 +3257,24 @@ JRT_LEAF(intptr_t*, SharedRuntime::OSR_migration_begin( JavaThread *current) )
kptr2 = fr.next_monitor_in_interpreter_frame(kptr2) ) {
if (kptr2->obj() != nullptr) { // Avoid 'holes' in the monitor array
BasicLock *lock = kptr2->lock();
// Inflate so the object's header no longer refers to the BasicLock.
if (lock->displaced_header().is_unlocked()) {
// The object is locked and the resulting ObjectMonitor* will also be
// locked so it can't be async deflated until ownership is dropped.
// See the big comment in basicLock.cpp: BasicLock::move_to().
ObjectSynchronizer::inflate_helper(kptr2->obj());
if (LockingMode == LM_LEGACY) {
// Inflate so the object's header no longer refers to the BasicLock.
if (lock->displaced_header().is_unlocked()) {
// The object is locked and the resulting ObjectMonitor* will also be
// locked so it can't be async deflated until ownership is dropped.
// See the big comment in basicLock.cpp: BasicLock::move_to().
ObjectSynchronizer::inflate_helper(kptr2->obj());
}
// Now the displaced header is free to move because the
// object's header no longer refers to it.
buf[i] = (intptr_t)lock->displaced_header().value();
}
// Now the displaced header is free to move because the
// object's header no longer refers to it.
buf[i++] = (intptr_t)lock->displaced_header().value();
#ifdef ASSERT
else {
buf[i] = badDispHeaderOSR;
}
#endif
i++;
buf[i++] = cast_from_oop<intptr_t>(kptr2->obj());
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/share/utilities/globalDefinitions.hpp
Expand Up @@ -1036,7 +1036,8 @@ const juint badHeapWordVal = 0xBAADBABE; // value used to zap
const juint badMetaWordVal = 0xBAADFADE; // value used to zap metadata heap after GC
const int badCodeHeapNewVal= 0xCC; // value used to zap Code heap at allocation
const int badCodeHeapFreeVal = 0xDD; // value used to zap Code heap at deallocation

const intptr_t badDispHeaderDeopt = 0xDE0BD000; // value to fill unused displaced header during deoptimization
const intptr_t badDispHeaderOSR = 0xDEAD05A0; // value to fill unused displaced header during OSR

// (These must be implemented as #defines because C++ compilers are
// not obligated to inline non-integral constants!)
Expand Down

3 comments on commit d0ea2a5

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

@rkennke
Copy link
Contributor

Choose a reason for hiding this comment

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

/backport lilliput-jdk21u

@openjdk
Copy link

@openjdk openjdk bot commented on d0ea2a5 Mar 25, 2024

Choose a reason for hiding this comment

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

@rkennke the backport was successfully created on the branch backport-rkennke-d0ea2a51 in my personal fork of openjdk/lilliput-jdk21u. To create a pull request with this backport targeting openjdk/lilliput-jdk21u:lilliput, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

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

The commit being backported was authored by Martin Doerr on 17 Oct 2023 and was reviewed by Patricio Chilano Mateo and Dean Long.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/lilliput-jdk21u:

$ git fetch https://github.com/openjdk-bots/lilliput-jdk21u.git backport-rkennke-d0ea2a51:backport-rkennke-d0ea2a51
$ git checkout backport-rkennke-d0ea2a51
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/lilliput-jdk21u.git backport-rkennke-d0ea2a51

Please sign in to comment.