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

[Tom Rodriguez]

On Feb 14, 2012, at 2:57 PM, Igor Veresov wrote:

> On Tuesday, February 14, 2012 at 2:31 PM, Tom Rodriguez wrote:
>> 
>> On Feb 14, 2012, at 2:10 PM, Igor Veresov wrote:
>> 
>>> On Tuesday, February 14, 2012 at 1:20 PM, Tom Rodriguez wrote:
>>>> 
>>>> On Feb 14, 2012, at 1:08 PM, Igor Veresov wrote:
>>>> 
>>>>> I tried that. It made no measurable difference. I figured it's better to start running a compile while we're doing GC, which is what will happen after the bailout.
>>>> 
>>>> So it's not about yielding to a safepoint? It's about starting the compile sooner?
>>> No-no, starting compiler is just an alternative of entering a safepoint. The choices basically are:
>>> 1. Safepoint right in the sweeper and continue sweeping after the safepoint.
>>> 2. Bailout from the sweeper but safepoint immediately, don't select the task from the compile queue and don't compile.
>>> 3. Bailout and processed with selecting the task and transition to native to compile (at which point a safepoint will happen).
>>> 
>>> So, I chose (3) because the process of selecting a task from the queue and transitioning to native to start a compile appear to be fast enough.
>> 
>> But it can arbitrarily starve the sweeper if we're safepointing often enough, like for scavenges.
> 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?

> 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?

tom

> 
> igor 
>> 
>> tom
>> 
>>> 
>>> igor
>>>> 
>>>> tom
>>>> 
>>>>> 
>>>>> igor
>>>>> 
>>>>> On Tuesday, February 14, 2012 at 1:04 PM, Tom Rodriguez wrote:
>>>>> 
>>>>>> 
>>>>>> On Feb 14, 2012, at 12:36 PM, Igor Veresov wrote:
>>>>>> 
>>>>>>> Code cache sweeper must yield to safepoint. With high numbers of methods sweeping can block the safepoint from happening.
>>>>>> 
>>>>>> Why don't you just safepoint instead of aborting the current sweep?
>>>>>> 
>>>>>> We may also want to increase the sweep fraction for tiered.
>>>>>> 
>>>>>> tom
>>>>>> 
>>>>>>> 
>>>>>>> Webrev: http://cr.openjdk.java.net/~iveresov/7145345/webrev.00/
>>>>>>> 
>>>>>>> 
>>>>>>> igor
>