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

[Thomas Schatzl]

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