RFR: 8291714: Implement a Multi-Reader Single-Writer mutex for Hotspot

[Coleen Phillimore]

On Fri, 19 Aug 2022 06:46:17 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

>> I'm going to claim that MutexLocker with a nullptr argument is a mistake,
>> making it much harder to understand code that does that (or might do that, who
>> knows, there's nothing in the source code to suggest one way or the other).
>> 
>> My recollection when looking at this with Coleen a long time ago was that
>> there are some tiny number of places that were doing that. Reasons included
>> (1) called really early (before threads and mutexes are initialized), (2)
>> weird (and possibly broken) things where the VMThread (thinks it) needs to
>> ignore a lock, or (3) recursion. Making those few unusual cases look textually
>> identical to the usual case is not very helpful, IMO. Coleen says "At one
>> point I thought we'd squashed the null arguments to MutexLocker. We should do
>> that."
>
> I think the pattern of passing a nullptr argument to Lockers is a pattern that we reach for quite often. The benefit is that it allows for a conditional scope, without messing up the surrounding code.
> 
> Compare
> 
> Locker locker(should_lock ? &_lock : nullptr);
> doit1();
> doit2();
> doit3();
> 
> vs
> 
> if (should_lock) {
>   Locker locker(&_lock);
>   doit1();
>   doit2();
>   doit3();
> } else {
>   doit1();
>   doit2();
>   doit3();
> }
> 
> or, having to go extra miles to pull out the code into helper functions:
> 
> void MyClass::doall() {
>   doit1();
>   doit2();
>   doit3();
> }
> ...
> if (should_lock) {
>   Locker locker(&_lock);
>   doall();
> } else {
>   doall();
> }
> 
> 
> The first version is much easier to read, IMHO. If you want to ban nullptrs in Lockers, I'd like to see an alternative that retains the crispness of the code.

You can have a blah_locked() version in these cases.  Having code called in multiple situations (some paths locked and some not) might be not great.  Some higher level of code should have done the locking or not.
That said, the sweeper has multiple of these with the Compile_lock so this can't be attempted to be squashed until the sweeper is gone.

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

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