RFR (S/M): 8136678: Implement adaptive sizing algorithm for IHOP

[Thomas Schatzl]

Hi all,

  let me retract review for this change now. The adaptive IHOP control
is now dependent on external variables (in
G1AdaptiveIHOPControl::actual_target_threshold(), G1ReservePercent,
G1HeapWastePercent), and so the internal tests are brittle.

I need to find a solution for that before asking for further review.

Thanks,
  Thomas


On Tue, 2015-11-17 at 10:24 +0100, Thomas Schatzl wrote:
> Hi Jon,
> 
>   thanks a lot for your comments, appreciated.
> 
> On Fri, 2015-11-13 at 13:03 -0800, Jon Masamitsu wrote:
> > Thomas,
> > 
> > http://cr.openjdk.java.net/~tschatzl/8136678/webrev/src/share/vm/gc/g1/g1IHOPControl.hpp.frames.html
> > 
> > Can you add a comment describing what this is?
> > 
> > 91 size_t _prev_unrestrained_young_size;
> > 
> > Why "recalculate" instead of just "calculate"?  "calculate" has the 
> > virtual of
> > being a little shorter name.
> 
> Fixed.
> 
> > 95 // Updates _current_threshold according to internal state.
> > 96 void recalculate();
> > 
> > This says that you want the target_occupancy to be at the maximum value
> > that will still allow young gen sizes as set with G1ReservePercen? That 
> > seems
> > a bit aggressive to me since it is the value that will be used until 
> > there is
> > enough data to create a better estimate.   Maybe arbitrarily add an 
> > extra 20%
> > to safe_heap_percentage?
> > 
> > 1257 if (safe_heap_percentage < 100) {
> > 1258 target_occupancy = G1CollectedHeap::heap()->max_capacity() * (100.0 
> > - safe_heap_percentage) / 100.0;
> > 1259 }
> > 
> > http://cr.openjdk.java.net/~tschatzl/8136678/webrev/src/share/vm/gc/g1/g1IHOPControl.cpp.frames.html
> 
> I fixed that by moving this calculation into the policy to make clear
> that this _target_occupancy is an internal value. I would have needed
> this refactoring anyway for JDK-8140777.
> 
> (And it certainly looks better.)
> 
> > Add a flag in place of the "2" just to make experimentation easier?
> > 
> > 133 return (_marking_times_s.num() > 2) && (_allocation_rate_s.num() > 2);
> > 
> 
> Added.
> 
> > 
> > 
> > Do you still want the comment at the end?
> > 
> > 244 size_t const settled_ihop3 = 0; // target_size - (young_size + 
> > alloc_amount2/alloc_time2 * marking_time2);
> 
> Removed. Copy&paste error :)
> 
> > Rest looks good.
> > 
> > Jon
> 
> New webrevs:
> 
> http://cr.openjdk.java.net/~tschatzl/8136678/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8136678/webrev.1/ (full)
> 
> Thanks,
>   Thomas
>