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

[Stefan Karlsson]

On Thu, 18 Aug 2022 15:43:42 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> It would make sense in the same circumstances the MutexLocker also takes a NULL argument.
>
> 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(should_lock ? &_lock : nullptr);
  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.

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

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