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

[Thomas Schatzl]

Hi,

On Mon, 2013-06-17 at 16:59 -0700, Hiroshi Yamauchi wrote:
> 
>                         I also agree that adding the assert(!
>                         SafepointSynchronize::is_at_safepoint()) (or
>                         assert(!
>                         Thread->current()->is_VM_thread())) may be
>                         good enough and if sneaking
>                         can be turned off for a case like this, it'd
>                         be safer.
>                 I think we should go for this solution, i.e. adding
>                 the assert (because
>                 it would be wrong anyway to call this code during
>                 safepoint) and using
>                 try_lock()/unlock().
>                 
>                 Try_lock() itself is the same as the current code, and
>                 unlock() too
>                 (except for some additional checks that should fail
>                 fast). The code that
>                 is guarded handles occasional skipping already and
>                 actually exits fast
>                 if the frequency is too high, so I do not see a big
>                 advantage doing
>                 custom code.
> 
> 
> I tried a version with try_lock()/unlock() as in:
>
>[...]
>
> It appears that the VM thread can call this at a safepoint (a young
> gen collection) unfortunately, and the !is_at_safepoint() assert
> fails. Here's a snippet of the stack trace:
> 
> 
> #7  0xf64e0be4 in CMSCollector::sample_eden_chunk (this=0x709366b0)
> #8  0xf652ef14 in DefNewGeneration::allocate (this=0xf5c42900,
> word_size=18, is_tlab=false)
> #9  0xf6654c62 in GenCollectedHeap::attempt_allocation
> (this=0xf5c2c8a8, size=18, is_tlab=false, first_only=false)
> #10 0xf646ec02 in GenCollectorPolicy::satisfy_failed_allocation

Yes, this is a valid execution path - it is after actual gc, but still
in the safepoint.

> Now, if I comment out the assert, it seems to work (though I haven't
> tested it very long.) This may be good if in fact sneak won't happen
> with try_lock()/unlock() only.
>
> I haven't tried this, but another potential approach might be to give
> up sampling (just return) if it's called by the VM thread at a
> safepoint, though the VM thread might allocate a large object, and the
> evenness of the sample distribution could suffer to some extent.

I'd just remove the assert :)

As already mentioned multiple times, try_lock() does never actually
block the thread, so the vm thread cannot actually sneak, as you already
noticed in your question to David, and he confirmed. The guarded code
can also never block, it's just a few assignments.


Thomas

>