RFR (S): 8248221: G1: millis_since_last_gc updated at wrong time

[Ivan Walulya]

Looks good!

//Ivan

> On 25 Jun 2020, at 10:04, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>  can I have reviews for this change that fixes the location at which the millis_since_last_gc timestamp is updated.
> 
> The spec of the only user in sun.rmi.transport.GC.maxObjectInspectionAge says:
> 
> * Returns the maximum <em>object-inspection age</em>, which is the number
> * of real-time milliseconds that have elapsed since the
> * least-recently-inspected heap object was last inspected by the garbage
> * collector.
> 
> Currently we do that update only at young gc, which is wrong. We should do that if/when a complete liveness analysis cycle has finished instead, i.e. end of marking/end of full gc.
> 
> I did some testing using the TestMillisSinceLastGC.java jtreg test in http://cr.openjdk.java.net/~tschatzl/8248221/webrev.test/. As expected, it failed for G1 but all other GCs tested there pass (Epsilon will probably also fail, because it never does any gc so I did not even try).
> 
> However the test intrinsically depends on timing i.e. the duration between some GC and checking the CollectedHeap::millis_since_last_gc() result, so I did not add it to this webrev. While in my ci testing it did not fail, I deemed it too risky to add.
> I also eyeballed that the output is still reasonable (i.e. no unit problem introduced by 8165501) using its log output for all affected collectors.
> 
> Depends on 8243974.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8248221
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8248221/webrev/
> Testing:
> tier1-5 with 8243974 and 8165501, manual testing as described above.
> 
> Thanks,
>  Thomas