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

[Thomas Schatzl]

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