RFR: 8272797: Mutex with rank safepoint_check_never imply allow_vm_block [v7]

[David Holmes]

On Fri, 27 Aug 2021 15:54:45 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> See CR for details.  This essentially makes Mutex that safepoint_check_never imply a NoSafepointVerifier, which is a stronger and simpler invariant.
>> Tested with tier1-6.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Reverse the order of safepoint_check_required and allow_vm_block arguments so that the first can imply the value of the second.  This is good because if we decide to rename the second to something like _allow_safepoint_use instead there's less to change. Also remove default arguments as they are evil.

Hi Coleen,

A few non-blocking queries on some of the locks but the changes are fine as-is.

Thanks,
David

src/hotspot/share/gc/shared/taskTerminator.cpp line 75:

> 73:   _queue_set(queue_set),
> 74:   _offered_termination(0),
> 75:   _blocker(Mutex::leaf, "TaskTerminator", Monitor::_safepoint_check_never),

Is this lock even used by any JavaThreads? I'm guessing not - in which cases the always/never and allow/not-allow are immaterial, and the change is harmless.

src/hotspot/share/gc/z/zMetronome.cpp line 31:

> 29: 
> 30: ZMetronome::ZMetronome(uint64_t hz) :
> 31:     _monitor(Monitor::leaf, "ZMetronome", Monitor::_safepoint_check_never),

Again the "false" to "true" stands out here, but again I have to assume this is not a lock that is used by JavaThreads. Makes me wonder whether these locks can never encounter contention by the VMThread (otherwise they would assert).

src/hotspot/share/memory/heapInspection.hpp line 248:

> 246:       _missed_count(0),
> 247:       _success(true),
> 248:       _mutex(Mutex::leaf, "Parallel heap iteration data merge lock", Mutex::_safepoint_check_always) {}

Is this lock even used by JavaThreads. ??

test/hotspot/gtest/metaspace/test_is_metaspace_obj.cpp line 52:

> 50: 
> 51:   void do_test(Metaspace::MetadataType mdType) {
> 52:     _lock = new Mutex(Monitor::leaf, "gtest-IsMetaspaceObjTest-lock", Monitor::_safepoint_check_never);

Again unclear what impact false to true might have here - assuming VM threads even lock this? Ditto for other metaspace tests.

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

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5218