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

[Jon Masamitsu]

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:
>>>>> - the code to get the lock on _eden_chunk_sampling_active seems to be
>>>>> needlessly complicated.
>>>>>
>>>>> Please consider using a dedicated Monitor in conjunction with
>>>>> try_lock(). I.e.
>>>> Thomas,
>>>>
>>>> Any concerns about the "sneak" code in try_lock()?  That would only
>>>> matter if the VM thread were involved in the eden sampling along with
>>>> other GC threads, yes?
>>> I do not think so in this case. Only the VM thread can sneak, and only
>>> during safepoint. The sampling is done only for eden space and eventually
>>> from-space, which are never allocated to during gc (only by java threads
>>> outside of stw pauses), aren't they?
>>>
>>> The code could assert that in the sampling too.
>> Where would the assert go that asserts sampling is
>> being done?
>>
>> 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).

Jon

>
> E.g.
>
> bool try_lock(bool allow_vm_thread_to_sneak = true) {
>    ...
>    bool can_sneak = allow_vm_thread_to_sneak && ...
>    ...
> }
>
> Then in the code
>
> if (...->try_lock(false)) {
>    ...
>    ...->unlock();
> }
>
> All other uses would be unaffected.
>
> I would simply prefer using the existing synchronization primitives
> instead of rolling our own just for that particular use.
> They make the code also much more readable.
>
> Another reason I am not worried about vm thread sneaking in this case is
> that during gc only the vm thread runs (and try_lock() does not seem to
> be a synchronization point for the safepoint), which cannot sneak itself
> (or is at least benign).
>
> Thomas
>
>