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

[Jon Masamitsu]

Mikael,

This is partial.  If you send out a second webrev based on Mikael's
review, I'll finish with that.


http://cr.openjdk.java.net/~tschatzl/8136681/webrev/src/share/vm/gc/g1/g1CollectedHeap.hpp.frames.html

> 1370 // Returns the number of regions the humongous object of the 
> given word size
> 1371 // covers.

"covers" is not quite right since to me it says that the humongous 
object completely uses the
region.  I'd use "requires".

http://cr.openjdk.java.net/~tschatzl/8136681/webrev/src/share/vm/gc/g1/g1IHOPControl.hpp.html

   49   // Update information about recent time during which allocations happened,
   50   // how many allocations happened and an additional safety buffer.

// Update information about

I DON'T KNOW WHICH OF THESE IS MORE PRECISE.
//   Time during which allocations occurred (sum of mutator execution time + GC pause times)
OR
//   Concurrent marking time (concurrent mark end - concurrent mark start)

//   Allocations in bytes during that time
//   Safety buffer ???

I couldn't figure out what the safety buffer is supposed to be.  It seems to
be the young gen size but don't know why.

   33 // Manages the decision about the threshold when concurrent marking should start.

// IHOP = Initiating heap occupancy in percent of the entire heap

   34 class G1IHOPControl : public CHeapObj<mtGC> {
   35  protected:
   36   double _ihop_percent;
// Occupancy in bytes.  This is always less than or equal to the (maximum or current maximum?) heap occupancy
   37   size_t _target_occupancy;


	// Most recent complete allocation period (time from first young-only GC to
	// end of marking) in seconds.  Any period which is interrupted by a full GC
	// is ignored.
   65    _last_allocation_time_s(0.0),
	// Number of bytes allocated during that period (mutator allocation + promotions)
   66    _last_allocated_bytes(0),
	// Length of most recent marking cycle in seconds
   67    _last_marking_length_s(0.0) { }

	// Occupany in bytes at which a concurrent marking cycle is started. This count
	// includes the waste in regions occupied by humongous objects and waste at
	// the end of any old region that is not the currently being used for promotions
	// and the waste at the end of survivor regions.

   69   size_t get_conc_mark_start_threshold() { return (size_t) (_ihop_percent * _target_occupancy / 100.0); }

This might be better named update_marking_length() but can be done in a later cleanup.

   	// Update marking length (concurrent marking end - concurret marking start)
   77   virtual void update_time_to_mixed(double marking_length_s) {

Have you given much thought to what affect a to-space exhaustion should 
have on
IHOP?   I understand that it is not the design center for this but I 
think that to-space
exhaustion can confuse the statistics.   Maybe a reset of the statistics 
and a dropping
the IHOP to a small value (current heap occupancy or even 0) until you 
get 3 successful
marking cycles.  Or think about it later.

Jon

On 11/5/2015 1:42 AM, Thomas Schatzl wrote:
> Hi all,
>
>    can I have reviews for this preparatory change for adaptive IHOP
> sizing, that factors out the IHOP calculation from G1Collectorpolicy
> into separate classes?
>
> So instead of having this single line at G1CollectorPolicy:944
>
>   944   size_t marking_initiating_used_threshold =
>   945     (_g1->capacity() / 100) * InitiatingHeapOccupancyPercent;
> [...]
>   949   if ((cur_used_bytes + alloc_byte_size) >
> marking_initiating_used_threshold) {
>
> that determines whether marking should start, that decision is
> externalized into a G1IHOPControl class with a
> get_conc_mark_start_threshold() method.
>
> This allows flexible replacement of the IHOP calculation algorithm
> depending on some switch using a factory method.
>
> In addition to that, this change tracks and passes several information
> IHOPControl instances need, like allocation between gcs, mutator time
> from initial mark to the first mixed gc, and others.
>
> The change also adds some test.
>
> It depends on all my recent patches in this area, namely:
> 8140689: Skip last young-only gc if nothing to do in the mixed gc phase
> 8140597: Forcing an initial mark causes G1 to abort mixed collections
> 8140585: PLAB statistics are flushed too late
> 8139874: After G1 Full GC, the next GC is always a young-only GC
> 8138740: Start initial mark right after mixed GC if needed
>
> The problem is that it expects (and verifies) that the GC pauses are
> executed in the "correct" order.
>
> There is no behavioral change in this CR.
>
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8136681
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8136681/webrev/
> Testing:
> jprt, vm.gc, lots of manual testing, new VM test
>
> Note that SQE will create additional tests.
>
> Thanks,
>    Thomas
>