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

Johan Sjölén duke at openjdk.org
Fri Aug 26 11:50:01 UTC 2022


On Thu, 25 Aug 2022 19:25:55 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Johan Sjölén 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 32:
> 
>> 30: #include "utilities/readWriteLock.hpp"
>> 31: 
>> 32: void ReadWriteLock::read_lock(Thread *current) {
> 
> The more common (though far from universal) style in HotSpot code cuddles the `*` punctuator with the type rather than the variable, i.e. `Thread* current`.

Good find, we have two different styles in the header and this file too.

> src/hotspot/share/utilities/readWriteLock.cpp line 64:
> 
>> 62: void ReadWriteLock::read_unlock() {
>> 63:   for (;;) {
>> 64:     const int32_t count = Atomic::load_acquire(&_count);
> 
> I think an invariant here is that count is never 0 or -1.  Consider asserting that.

Good find.

> src/hotspot/share/utilities/readWriteLock.cpp line 81:
> 
>> 79:       if (count == -2) {
>> 80:         Locker locker(&_mon);
>> 81:         _mon.notify_all();
> 
> Unfortunately, this also wakes up all pending readers.  I wonder if there's a way to avoid that by having two monitors (only having monitors and not bare condvars makes that harder).

Yes. I propose leaving this work for a future RFE. There are many potential improvements, I want to await the need for those to implement them.

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

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


More information about the hotspot-dev mailing list