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

[Jon Masamitsu]

Thanks for the changes.

http://cr.openjdk.java.net/~tschatzl/8136678/webrev.0_to_1/src/share/vm/gc/g1/g1IHOPControl.cpp.frames.html

185 static double percentage_of(double quantity, double base_quantity) {


looks a lot like

share/vm/gc/g1/g1RemSetSummary.cpp

static double percent_of(size_t numerator, size_t denominator) {

Time to add a function to globalDefinitions?



Minor issues.

http://cr.openjdk.java.net/~tschatzl/8136678/webrev.0_to_1/src/share/vm/gc/g1/g1IHOPControl.hpp.frames.html

You can ignore this but

103 // depending on predictions of current allocation rate and time 
periods between


might be better as

103 // based on predictions of current allocation rate and time periods 
between


Something got jumbled up?

117 // non-young gen occupancy compared against at the end of GC, but we 
need to Jon


On 11/17/2015 01:24 AM, 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20151117/f8b5436f/attachment.htm>