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

[Johan Sjölen]

On Thu, 25 Aug 2022 21:41:46 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
>> 
>>  - Fix tests for updated threadHelper
>>  - Remove outdated threadHelper
>>  - Update documentation
>>  - Fix outdated headers and remove dead code
>>  - Review comments
>>  - Implement MRSWMutex
>
> src/hotspot/share/utilities/readWriteLock.cpp line 139:
> 
>> 137:     return;
>> 138:   }
>> 139: }
> 
> I was wondering why a lock for write doesn't hold the underlying
> PlatformMonitor across the scope of the mutex region. Something like this
> (completely untested)
> 
> 
> void ReadWriteLock::write_lock(Thread* current) {
>   _mon.lock();
>   while (true) {
>     const int32_t count = Atomic::load_acquire(&_count);
>     if (count < 0) {
>       // Some other writer is waiting for readers to complete.
>       // Approx:
>       // while (Atomic::load_acquire(&_count)) _mon.wait();
>       await_write_unlock(current);
>     } else if (Atomic::cmpxchg(&_count, count, -(count + 1)) == count) {
>       // Claimed the write slot, but there might be active readers.
>       if (count != 0) {
>         // Approx:
>         // while (Atomic::load_acquire(&_count) != -1) _mon.wait();
>         await_no_active_readers(current);
>       }
>       return; // return still holding lock
>     } // else failed to claim write ownership, so try again.
>   }
> }
> 
> void ReadWriteLock::write_unlock() {
>   assert(Atomic::load(&_count) == -1, "invariant");
>   Atomic::release_store(&_count, (int32_t)0);
>   _mon.notify_all();
>   _mon.unlock();
> }
> 
> 
> This seems simpler to me.  Am I missing something?
> 
> And looking at this also makes me wonder if `_mon` can just be a normal
> HotSpot `Monitor`, which would resolve many of the questions and comments I've
> made that are mostly driven by the direct use of `PlatformMonitor`.

This lock is essentially unbounded, are there any implications regarding stopping the VM from going into safepoints? I assume not, if we're using `Monitor`.

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

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