CMS parallel initial mark; vm thread sneaking in Thread::try_lock()

[David Holmes]

Thomas,

lock-sneaking is something very specific to the VMThread to avoid 
deadlocks at safepoints. I really don't think we want to see it promoted 
into the API for mutexes/monitors.

I hadn't been paying this thread too much attention but now I'm starting 
to think I need to.

David

On 13/06/2013 5:46 PM, Thomas Schatzl wrote:
> Hi,
>
> On Wed, 2013-06-12 at 07:25 -0700, Jon Masamitsu wrote:
>> On 6/11/2013 4:07 PM, Thomas Schatzl wrote:
>>> Hi,
>>>
>>> On Tue, 2013-06-11 at 15:30 -0700, Jon Masamitsu wrote:
>>>> On 6/10/13 12:49 PM, Thomas Schatzl wrote:
>>>>> Hi,
>>>>>
>>>>> On Fri, 2013-06-07 at 11:28 -0700, Jon Masamitsu wrote:
>>>>>> On 6/7/2013 4:23 AM, Thomas Schatzl wrote:
>> [...]
>>>> Or do you mean at the use of try_lock() assert that
>>>> we're not the VM thread?
>>>>
>>> Sorry, I was unclear. In the sampling code itself: sampling should not
>>> be performed during a safepoint as far as I can see. So e.g. add to the
>>> sampling method an
>>>
>>>     assert(!SafepointSynchronize::is_at_safepoint(), "...");
>>
>> OK.  That would be sufficient.
>>
>>>
>>> There is also the option to add a parameter to try_lock() that prohibits
>>> sneaking.
>>
>> Or a try_lock_no_sneak() that is called from try_lock() and could be used
>> here.  I hate that name.  And I wouldn't touch mutex.cpp myself (would worry
>> about unintended performance consequences).
>
> I would almost prefer the try_lock_no_sneak() approach you suggested -
> the assert would need some additional explanation while imo it would be
> immediately clear with the try_lock_no_sneak(). (But doing both is fine
> too).
>
> (Actually I'd expect that a try_lock() does not do unexpected things
> like sneaking in the first place. Try_lock() is a common name for that
> functionality in libraries with some expected behavior. That likely does
> not include something like sneaking the lock... i.e. you should be
> required to call try_lock_sneak() in the places where you want that,
> but, oh well...)
>
> As for that change to add try_lock_no_sneak() in mutex code, this is a
> minor structural change only. Performance wise I do not expect a big
> difference: the CAS isn't too fast, the surrounding loop does not give a
> real performance guarantee in the first place, and the
> try_lock_no_sneak() call would be the only call within mutex.cpp, i.e.
> most likely the compiler would inline it anyway into the try_lock()
> method.
>
> Thanks,
> Thomas
>