[master] RFR: Fix BasicLock to test for UseObjectMonitorTable rather than LM_LIGHTWEIGHT [v3]
Axel Boldt-Christmas
aboldtch at openjdk.org
Wed Jun 19 06:23:26 UTC 2024
On Mon, 17 Jun 2024 22:02:54 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:
>> The test compiler/uncommontrap/TestDeoptOOM.java was failing with -Xcomp because it was looking for the object monitor in the basicLock which is only the case for -XX:+UseObjectMonitorTable.
>>
>> Testing tier1 locally with table on and off.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>
> And another place where we clear_object_monitor_cache if not using the OM table.
I think we should try to keep it invariant that only with `LM_LEGACY` do you call `*displaced_header` member functions on BasicLocks.
src/hotspot/share/runtime/basicLock.cpp line 34:
> 32: void BasicLock::print_on(outputStream* st, oop owner) const {
> 33: st->print("monitor");
> 34: if (UseObjectMonitorTable) {
This was a pre-existing bug. It should be:
```c++
if (UseObjectMonitorTable) {
ObjectMonitor* mon = object_monitor_cache();
if (mon != nullptr) {
mon->print_on(st);
}
} else if (LockingMode == LM_LEGACY) {
I guess no testing ever printed a BasicLock with LM_MONITOR.
src/hotspot/share/runtime/basicLock.inline.hpp line 38:
> 36: assert(!UseObjectMonitorTable, "must be");
> 37: Atomic::store(&_metadata, header.value());
> 38: }
These should stay exclusive to `LM_LEGACY`.
src/hotspot/share/runtime/deoptimization.cpp line 1657:
> 1655: mon_info->lock()->set_bad_metadata_deopt();
> 1656: }
> 1657: #endif
Along the same lines keep `set_displaced_header` exclusive to `LM_LEGACY`
```c++
if (LockingMode == LM_LEGACY) {
mon_info->lock()->set_displaced_header(markWord::unused_mark());
} else if (UseObjectMonitorTable) {
mon_info->lock()->clear_object_monitor_cache();
}
#ifdef ASSERT
else {
assert(LockingMode == LM_MONITOR || !UseObjectMonitorTable, "must be");
mon_info->lock()->set_bad_metadata_deopt();
}
#endif
-------------
Changes requested by aboldtch (Committer).
PR Review: https://git.openjdk.org/lilliput/pull/182#pullrequestreview-2127071351
PR Review Comment: https://git.openjdk.org/lilliput/pull/182#discussion_r1645469948
PR Review Comment: https://git.openjdk.org/lilliput/pull/182#discussion_r1645425556
PR Review Comment: https://git.openjdk.org/lilliput/pull/182#discussion_r1645429078
More information about the lilliput-dev
mailing list