RFR (S): 8165501: Serial, Parallel and G1 should track the time since last gc for millis_since_last_gc() with full precision

[Ivan Walulya]

Hi Thomas, 

A few comments below,

src/hotspot/share/gc/parallel/psParallelCompact.cpp
-  jlong now = os::javaTimeNanos() / NANOSECS_PER_MILLISEC;
-  jlong ret_val = now - _time_of_last_gc;
+  jlong now = os::javaTimeNanos();
+  jlong ret_val = (now - _time_of_last_gc_ns) / NANOSECS_PER_MILLISEC;
 jlong now  => jlong now_ns  i see you do this for most of the variable names that refer to time in Nanos
src/hotspot/share/gc/g1/g1Policy.hpp
+  jlong time_of_last_gc() { return _time_of_last_gc_ns; }
same as previous comment, better to maintain the _ns
src/hotspot/share/gc/shared/generation.hpp
  // Time (in ms) when we were last collected or now if a collection is
  // in progress.
  virtual jlong time_of_last_gc(jlong now) {
    // Both _time_of_last_gc and now are set using a time source
    // that guarantees monotonically non-decreasing values provided
    // the underlying platform provides such a source. So we still
    // have to guard against non-monotonicity.
    NOT_PRODUCT(
      if (now < _time_of_last_gc_ns) {
        log_warning(gc)("time warp: " JLONG_FORMAT " to " JLONG_FORMAT, _time_of_last_gc_ns, now);
      }
    )
    return _time_of_last_gc_ns;
  }
  virtual void update_time_of_last_gc(jlong now)  {
    _time_of_last_gc_ns = now;
  }
comments should be edited to reflect the changes, in addition to _ns


//Ivan

> On 25 Jun 2020, at 09:35, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>  can I get reviews for this small change that unifies code a bit between Serial, Parallel and G1 with regards to tracking the time stamp for CollectedHeap::millis_since_last_gc?
> 
> In particular, it names the members uniformly, and removes an unnecessary division by internally storing raw os::JavaTimeNanos instead of some intermediate result.
> 
> Other collectors are not affected because they calculate millis_since_last_gc from an internal clock source.
> 
> There is no functionality change
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8165501
> Webrev:
> http://cr.openjdk.java.net/~tschatzl/8165501
> Testing:
> tier1-5 with 8243974, 8248221; see also 8248221 for manual testing info
> 
> Thanks,
>  Thomas