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

[Thomas Schatzl]

Hi Ivan,

   thanks for your review!

On 25.06.20 11:47, Ivan Walulya wrote:
> 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 
> Nanossrc/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 
> _nssrc/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
> 

I think I caught them all in

http://cr.openjdk.java.net/~tschatzl/8165501/webrev.1

no incremental one since almost everything changed anyway, doing another 
pass at making the code look similar.

Retested using the mentioned test.

Thanks,
   Thomas