On Mon, 28 Oct 2024 13:12:22 GMT, Richard Reingruber <rr...@openjdk.org> wrote:
>> src/hotspot/share/runtime/objectMonitor.hpp line 202: >> >>> 200: >>> 201: // Used in LM_LEGACY mode to store BasicLock* in case of inflation >>> by contending thread. >>> 202: BasicLock* volatile _stack_locker; >> >> IIUC the new field `_stack_locker` is needed because we cannot store the >> `BasicLock*` anymore in the `_owner` field as it could be interpreted as a >> thread id by mistake. >> Wouldn't it be an option to have only odd thread ids? Then we could store >> the `BasicLock*` in the `_owner` field without loosing the information if it >> is a `BasicLock*` or a thread id. I think this would reduce complexity quite >> a bit, woudn't it? > > `ObjectMonitor::_owner` would never be `ANONYMOUS_OWNER` with `LM_LEGACY`. I remember I thought about doing this but discarded it. I don't think it will reduce complexity since we still need to handle that as a special case. In fact I removed several checks throughout the ObjectMonitor code where we had to check for this case. Now it works like with LM_LIGHTWEIGHT (also a plus), where once the owner gets into ObjectMonitor the owner will be already fixed. So setting and clearing _stack_locker is contained here in ObjectSynchronizer::inflate_impl(). Granted that we could do the same when restricting the ids, but then complexity would be the same. Also even though there are no guarantees about the ids I think it might look weird for somebody looking at a thread dump to only see odd ids. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21565#discussion_r1819748043
