RFR (XS) 8071770 G1 does not implement millis_since_last_gc which is needed by RMI GC

[Joseph Provino]

Hi Thomas, does this look better?

I'm not sure it's exactly what you said.  Let me know and I'll change it.

joe

   2476  jlong G1CollectedHeap::millis_since_last_gc() {
   2477    // We need a monotonically non-decreasing time in ms but
   2478    // os::javaTimeNanos() used by os::elapsed_counter()
   2479    // does not guarantee monotonicity.
   2480    jlong now = os::elapsed_counter() / NANOSECS_PER_MILLISEC;
   2481    const G1Analytics* analytics = _g1_policy->analytics();
   2482    double last = analytics->last_known_gc_end_time_sec();
   2483    jlong ret_val = now - (last * 1000);
   2484    if (ret_val < 0) {
   2485      log_warning(gc)("millis_since_last_gc() would return : " 
JLONG_FORMAT
   2486        ". Returning zero instead.", ret_val);
   2487      return 0;
   2488    }
   2489    return ret_val;
   2490  }



On 7/14/2016 5:01 AM, Thomas Schatzl wrote:
> Hi,
>
> On Wed, 2016-07-13 at 15:03 -0400, Joseph Provino wrote:
>> Please review this small change to implement millis_since_last_gc in
>> G1.
>> CR: JDK-8071770 G1 does not implement millis_since_last_gc which is
>> needed by RMI GC
>> Webrev:  http://cr.openjdk.java.net/~jprovino/8071770/webrev.00
>> Passes JPRT.
>> Passes test included in the CR.
>> thanks.
> - in G1CollectedHeap::millis_since_last_gc():
>
> 2477   // We need a monotonically non-decreasing time in ms but
> 2478   // os::javaTimeMillis() does not guarantee monotonicity.
> 2479   jlong now = os::elapsed_counter() / NANOSECS_PER_MILLISEC;
> 2480   const G1Analytics* analytics = _g1_policy->analytics();
> 2481   double last = analytics->last_known_gc_end_time_sec();
> 2482   jlong ret_val = now - (last * 1000);
> 2483   // XXX See note in genCollectedHeap::millis_since_last_gc().
> 2484   if (ret_val < 0) {
> 2485     NOT_PRODUCT(log_warning(gc)("time warp: " JLONG_FORMAT,
> ret_val);)
> 2486     return 0;
> 2487   }
> 2488   return ret_val;
>
> Line 2478 references a method that is not called in this piece of code;
> 2483 seems to be a continuation of 2478, and what is the purpose of the
> "XXX"?
>
> I also think that the line in 2485 should not be NOT_PRODUCT. It is
> definitely interesting to have these kinds of issues printed in the log
> if they occur. The log message itself is nondescriptive.
>
> I would prefer only a reference to
> GenCollectedHeap::millis_since_last_gc() (and fixing the comment in
> that method, because it uses os::javaTimeNanos() and not
> os::javaTimeMillis(); and actually there are two semi-conflicting
> comments in that method too - just removing the first comment should be
> fine). The other reason is that the current comments duplicate some
> parts of that comment, which seems bad.
>
> Thanks,
>    thomas
>