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

[Igor Veresov]

On Tuesday, February 14, 2012 at 3:38 PM, Tom Rodriguez wrote:
> 
> 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?
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/

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

igor
 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20120214/dbb37a69/attachment.html