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