RFR (M): 8136681: Factor out IHOP calculation from G1CollectorPolicy

[Thomas Schatzl]

Hi,

On Mon, 2015-11-16 at 14:21 -0800, Jon Masamitsu wrote:
> 
> On 11/16/2015 05:31 AM, Thomas Schatzl wrote:
> > Hi Jon,
> >
> >    thanks a lot for all these reminders for better documentation. I have
> > been working too long on this functionality so that "everything is
> > clear" to me :)
> >
> > New webrevs with hopefully more complete explanations at:
> > http://cr.openjdk.java.net/~tschatzl/8136681/webrev.1_to_2/
> > (incremental)
> > http://cr.openjdk.java.net/~tschatzl/8136681/webrev.2/ (changes)
> http://cr.openjdk.java.net/~tschatzl/8136681/webrev.1_to_2/src/share/vm/gc/g1/g1IHOPControl.hpp.frames.html
> 
> 35 // The initial IHOP value relative to the target occupancy.
> 36 double _initial_ihop_percent;
> 
> 
> Does _initial_ihop_percent ever change?  Meaning is it a value that
> is saved when the control is initialized and stays at the value?

Yes it is constant in the current implementations.

The percentage value itself is not an interesting value.

> http://cr.openjdk.java.net/~tschatzl/8136681/webrev.1_to_2/src/share/vm/gc/g1/g1CollectedHeap.cpp.frames.html
> 
> Can you check if there is any live data in the region?  An empty
> region should not old-allocated?

An empty region does not have the evacuation_failed() flag set. I.e.
empty means that all objects were evacuated successfully, which means
there has been no evacuation failure in it.

> 5643 if (cur->is_young()) {
> 5644 policy->add_last_old_allocated_bytes(HeapRegion::GrainBytes);
> 5645 }
> 
> Most of the changes looked like they were in response to Mikael's comments
> and I'm assuming he'll know what needed to be done there.

Yes :) Thanks for looking through this.

Thanks,
  Thomas