review(XS): 7145345: Code cache sweeper must cooperate with safepoints

[Tom Rodriguez]

>>>> 
>>> That is theoretically possible I guess. Well, I think the only solution here would be to safepoint inside of the sweeper loop and restart then sweep after we wake up.
>> 
>> Why do we have to restart the sweep? Can't we just safepoint and continue the loop when we return?
> Yes, that's actually what I meant. 
>> 
>>> And also to limit the amount of methods it does per sweep (instead of the fraction) so that it doesn't starve compilation.
>>>>>> Then I think you should gate the amount of work the sweeper does instead. That would actually tend to distribute the sweeper work between different compiler threads better. Right now a single compile can be delayed a long time because it's trying to process a quarter of the cache.
>>>>> You're right. The a big fraction can starve compilation. I guess I could change the code to use a concrete amount of methods to sweep instead of the fraction. Fractions are not very convenient. But the bailout will be necessary in any case.
>>>> 
>>>> I agree that we need to allow safepoints in the sweeper loop but I'd rather not simply abort the sweeper loop if a safepoint happens in the middle. That just seems arbitrary.
>>>> 
>>>> Maybe the sweeper should have a time limit instead?
>>> But that would mean that we inhibit safepoints for that amount of time? If we don't enter the safepoint within a couple of ms after we see the request that would be really bad.
>> 
>> I wasn't suggesting that. I agree that sweeping should never inhibit safepointing. Is it possible that the individual work on an nmethod, like cleanup_inline_caches, could be responsible for delaying safepointing by a meaningful amount?
> May be, I didn't measure that part. I'll make sure I check that. 
> 
> So, would the following solution seem reasonable to you:
> http://cr.openjdk.java.net/~iveresov/7145345/webrev.01/

Structurally that looks pretty reasonable.  NmethodSweepFraction is an advertised flag so we'd have to silently ignore it instead of rejecting it.  But for 7u4 it might be best to just increase the NmethodSweepFraction and add a safepoint check in the loop instead of introducing a new flag that will likely require some more tuning.  256 seems rather low and I'd be concerned about the sweeper getting too far behind since it has more work to do with tiered.  With tiered we may need to reconsider how the sweeper does its work.

tom

> 
> Scanning 256 nmethods on a rather old x86 with fastdebug takes about 2ms.
> 
> igor
>