RFR (M): 8228609: G1 copy cost prediction uses used vs. actual copied bytes

[Kim Barrett]

> On Oct 22, 2019, at 1:30 PM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>  can I have reviews for this change that makes G1 calculate and the use actual amount of bytes copied for Object Copy phase estimation?
> 
> The problem is that the "used" value that is currently used for this can differ a lot from the number of actually copied bytes during the parallel phases.
> 
> Sources for differences are:
> - TLAB sizing
> - TLAB/region fragmentation
> - all of that multiplied by the number of threads
> 
> Particularly if the amount of copied data is small compared to the number of regions all this can add up and disturb the prediction quite a lot, although overall it's not that bad.
> 
> It's only that this and other small inaccuracies add up.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8228609
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8228609/webrev/
> Testing:
> hs-tier1-5
> 
> Thanks,
>  Thomas

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
 105 size_t G1ParScanThreadState::copied_words() {
 106   size_t result = _surviving_words;
 107   _surviving_words = 0;
 108   return result;
 109 }

The reset behavior seems unexpected, based on the name, which looks
like an accessor.

I think the reset behavior is to avoid double-counting by the
recording in evacuate_live_objects. That led me to consider suggesting
a more appropriate place for the reset might be in G1PSTS::flush(),
where the lab_waste and lab_undo_waste (that were recorded nearby)
also get reset. But I don't think that flush() is happening in the
right place to prevent double-counting of the waste values. Bug?

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1ParScanThreadState.cpp
 319     _surviving_words += word_sz;

Is it really worth having a separate accumulator for the total?  It
seems like we could instead have copied_words() return the sum over
the _surviving_young_words.

But that might not work because of the (lack of) reset in the right
place, per above.

------------------------------------------------------------------------------
src/hotspot/share/gc/g1/g1Policy.cpp
 782       double cost_per_byte_ms = (average_time_ms(G1GCPhaseTimes::ObjCopy) + average_time_ms(G1GCPhaseTimes::OptObjCopy)) / copied_bytes;

[pre-existing]

I think this is computing the rate at which active_workers worker
threads copies bytes. What if active_workers changes?

------------------------------------------------------------------------------