During the code review cycle for the following bug fix:

    JDK-8072439 fix for 8047720 may need more work

Markus had an idea for making MutexLocker smarter when it is
being used by non-JavaThreads. Here's the e-mail conversation:

On 2/24/15 2:33 PM, Daniel D. Daugherty wrote:
> More replies embedded below...
>
> On 2/24/15 10:02 AM, Markus Gronlund wrote:
>>
>> Actually thinking about this a bit more:
>>
>>
>>
>> I think we could make all uses of PeriodicTask_lock to be acquired with
>> MutexLocker (not Ex), and avoid passing the Mutex::_no_safepoint_check
>> flag (and lengthy comments):
>>
>>
>>
>> JavaThreads will (check for and) block for safepoints in WatcherThread::enroll/disenroll
>> if the PeriodicTask_lock is being held by someone else. Same thing in before_exit().
>>
>>
>>
>> Since the WatcherThread is not a JavaThread and will never check for a safepoint if
>> there is a contended lock, it will call IWait() (to park) directly.
>>
>>
>>
>> This would also make it possible to change the PeriodicTask_lock from being asserted
>> as a "_safepoint_check_sometimes" to a "_safepoint_check_always".
>
> Coleen (and Max) would like that... :-)
>
>> In order to do this however, we would need to rework Monitor::Wait():
>>
>>
>>
>> The only place (currently) where there is a requirement to pass "Mutex::_no_safepoint_check"
>> is when the WatcherThread calls Wait() - but this is because we have this in there:
>>
>>
>>
>> // !no_safepoint_check logically implies java_thread
>>
>> guarantee(no_safepoint_check || Self->is_Java_thread(), "invariant");
>>
>>
>>
>> This does not make sense - a WatcherThread should not need to explicitly say "please go
>> outside the safepoint protocol" - it is not a JavaThread so to it, there is no such thing as a
>> safepoint.
>
> That's why MutexLockerEx exists... but I see your point.
> We're evolving this area with the _safepoint_check_* stuff
> so why not make MutexLocker smarter...
>
>> In Monitor::lock() we branch to a safepoint check based on the Self->isJavaThread(), but in
>> Monitor::wait() we also allow for JavaThreads to circumvent the protocol if they pass in the
>> correct flag.
>
> This is definitely a little inconsistent.
>
>> Maybe we can change Monitor::Wait() a wee bit (I know this is sensitive code), and still allow
>> for arbitrary passings of "no_safepoint_checks" for JavaThreads, but if there is nothing passed,
>> we take the safepoint route if there is a JavaThread, and not if there is anything else (similar
>> to Monitor::lock()). Callers which are not JavaThreads should not need to pass these options.
>> Combining this with the lock assertion states, such as, "_safepoint_check_always" will disallow
>> anyone (any JavaThread) to circumvent the safepoint protocol for the PeriodicTask_lock.
>
> Yes I agree this can likely be cleaned up a bit...
>
>> I will try some experiments, so Dan please go ahead with what you already have.
>
> So I'll leave the further cleanup to your experiment
> and a new bug. I'll move forward with the webrev plus
> the tweaks I identified in my reply to your first e-mail.
>
> Thanks again for the reviews!
>
> Dan
>
>>
>>
>> Cheers
>>
>> Markus