RFR: 8291555: Implement alternative fast-locking scheme [v26]

Roman Kennke rkennke at openjdk.org
Thu Mar 16 06:34:35 UTC 2023


On Wed, 15 Mar 2023 19:40:33 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>>> Would it be possible to open/send me the failing test that triggers vframeArray assert
>>> or extract a reproducer that you could publish?
>> 
>> I have started an internal discussion at Oracle to see what it would take
>> to move that test from closed to open. Will keep you posted.
>
>> > Would it be possible to open/send me the failing test that triggers vframeArray assert
>> > or extract a reproducer that you could publish?
>> 
>> I have started an internal discussion at Oracle to see what it would take to move that test from closed to open. Will keep you posted.
> 
> Thank you!
> 
> Regarding moving this PR back to draft, I am not sure. I can do that, yes. But really the fundamental algorithm and implementation is basically fixed since half a year already. I have re-worked it into a fresh PR based on the request to put it behind a flag. The recent change to a fixed-size lock-stack has probably invalidated part of your previous reviews, and I am sorry for that. On the upside, it removed a lot of complexity in the JIT compilers and assembly code generators.
> 
> What else do I expect to happen?
> 
> Thomas is working on an ARM(32) port, but this is quite separate and could even land after this PR is done.
> 
> I still don't quite like the naming. Fast-locking doesn't really say anything and it's not (meant to be) faster than the previous stack-locking. It is an alternative (and less racy, on the object header) way to implement a thin-locking layer before inflating monitors, that is all. So maybe -XX:+UseNewThinLocking? It is somewhat temporary anyway. At least my hope is that when we eventually switch to Lilliput turned on by default, we would entirely remove stack-locking.
> 
> I would also add some code in arguments.cpp to keep this new thin locking turned off on platforms that don't yet support it.
> 
> Besides that, from my POV, it is pretty much done.
> 
> What do you think?

> @rkennke The changed to fixed-size lock stack was a significant change as you note and that suggested to me that the design was still in flux. So I have to wonder whether everything is in fact now stable? (or as stable as one should expect for an experimental new feature)

I think it is, except for the few points that I mentioned earlier, and anything that comes up in reviews, I don't expect any major design changes. In fact, I would actively hold them back if anything comes up, to move this PR across the line at this point. I can't think of any bad spots where I thunk 'oh this is ugly - this needs a better approach' though.


> > Fast-locking doesn't really say anything and it's not (meant to be) faster than the previous stack-locking.
> 
> 
> 
> Agreed. But I don't think "Thin locks" is an option as that was specifically an IBM locking implementation. Historically Hotspot's locking mechanism has internally been referred to as stack-locks, so I would suggest UseNewStackLocks

That's fine by me.

Thank you,
Roman

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

PR: https://git.openjdk.org/jdk/pull/10907


More information about the serviceability-dev mailing list