[master] RFR: 8302209: [Lilliput] Optimize fix-anon monitor owner path [v2]

Roman Kennke rkennke at openjdk.org
Wed Mar 8 12:57:41 UTC 2023


On Wed, 8 Mar 2023 07:37:16 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> Reading through fast_unlock() some more...
> 
> What I did not understand at first is why you pop the object twice in fast_unlock: once in the stub, then again in fast_unlock_impl. But "Stacked" means thin-locked, right? So the section at [c2_MacroAssembler_x86.cpp 948ff](https://github.com/openjdk/lilliput/blob/55b6c241bad84d7aedb7c7a9a874773914ad359c/src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp#L948) is for handling stack-based thin locks?

Right. And the other path that handles anonymous owners is dealing with ObjectMonitor based locking.
 
> If that is so, at some point we may want to rename things to make it simpler. E.g. "Stacked"->"Thin" or similar. "Thin" would be a good fit for both new-style locks and old stack-based locks. I understand that you don't want to do this downstream though to keep the delta small.

Yes. That whole code is quite a mess, and I already did some cleanups in upstream, but it still warrants some serious work. In particular, I would like to get rid of that implicit dealing with ZF there.

> Could we factor out lock stack pushing and popping into some utility functions?
> 
> Also, would it make sense to emit an assert in debug when popping to not get into negative values, similar to what you do in `MacroAssembler::fast_lock_impl`?

Yes, but it's creeping scope of this PR. This belongs in the bigger fast-locking PR IMO: https://github.com/openjdk/jdk/pull/10907

-------------

PR: https://git.openjdk.org/lilliput/pull/74


More information about the lilliput-dev mailing list