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

[Igor Veresov]

The tests seem to give about the same results. So, I guess we can settle on this variant.

igor


On Tuesday, February 14, 2012 at 6:50 PM, Igor Veresov wrote:

> I haven't tried yet, I wanted to make sure that you're ok with it first. Now that you are :) I'll send it for testing.
> 
> igor
> 
> 
> On Tuesday, February 14, 2012 at 6:35 PM, Tom Rodriguez wrote:
> 
> > 
> > On Feb 14, 2012, at 6:22 PM, Igor Veresov wrote:
> > 
> > > Tom, I left the safepointing in the loop and bumped the fraction to 16. Would that be ok?
> > > 
> > > Webrev: http://cr.openjdk.java.net/~iveresov/7145345/webrev.02/
> > 
> > That seems ok. I agree that the current flags don't necessarily give us the right level of control. Intuitively a flag that limits the amount of work that a sweep does seems like a good idea but I'd want more data about how it behaves and whether the sweeper ends up getting behind, leading to running out of code cache. There might also be better ways to hand the work out to the compiler threads. Currently it's done before a compile starts and it might make more sense to do it afterwards or to try to take advantage of idle compiler threads. The sweeper also isn't as efficient as it might be. I believe we're repeatedly cleaning caches which are already clean and we might be able to do better if we did more bookkeeping. 
> > 
> > Does this perform the same as your original fix?
> > 
> > tom
> > 
> > > 
> > > igor
> > > 
> > > On Tuesday, February 14, 2012 at 5:43 PM, Igor Veresov wrote:
> > > 
> > > > Although for this particular case with AccountSOA I can just leave the fraction as is.
> > > > 
> > > > 
> > > > igor
> > > > 
> > > > On Tuesday, February 14, 2012 at 5:40 PM, Igor Veresov wrote:
> > > > 
> > > > > 
> > > > > 
> > > > > On Tuesday, February 14, 2012 at 4:45 PM, Tom Rodriguez wrote:
> > > > > > > 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.
> > > > > You mean we should just statically set it to another value? Like, say 40?
> > > > > 
> > > > > The problem with the fraction is that it never guarantees the time we're going to starve compilation. Moreover this time changes as we add new methods to the code cache while sweeping is in progress. So I have no clue really what to set it to. I mean setting it statically seems wrong...
> > > > > 
> > > > > igor 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20120215/5d61952b/attachment.html