RFR (XXL): 8208390: G1 should always use ticks as internal time base instead of double

Thomas Schatzl thomas.schatzl at oracle.com
Tue Apr 28 08:14:54 UTC 2020


Hi all,

   can I have reviews for this (tedious to look at) change that moves 
G1's time base from double's to Ticks/Tickspan?

Some comments:

- this covers only immediate and immediately related changes to use 
Ticks/Tickspan in G1GCPhaseTimes. For various reasons some components 
still use doubles (G1Analytics, MMU, ...).

- only minor other cleanup has been performed apart from exceptions 
below. There is a big follow-up rename to also remove the _sec/_ms/* 
suffixes currently available for reference to rename variables, methods 
etc. at http://cr.openjdk.java.net/~tschatzl/8208390/webrev.rename/. 
Will file a separate CR for that.

- also a few obvious bugs or cleanups for incomprehensible code (in that 
it wasn't obvious what G1 was trying to achieve) have been filed or 
existing amended.

- G1GCPhaseTimes has been a bit sloppy about handling points in time vs. 
durations, so an explicit distinction had to be made in the change.

- for support of G1CollectedHeap::millis_since_last_gc() G1 previously 
reused G1Policy::_collection_pause_end; now that (at least 
superficially) _collection_pause_end and the value returned by 
G1CollectedHeap::millis_since_last_gc() use a different time source 
(Ticks vs. os::javaTimeNanos()) a separate variable has been introduced 
(_collection_pause_end_javaTime).

Not sure why that millis_since_last_gc() calculation should be part of 
G1Policy (it's unused there), particularly now, but I kept it there. 
Will file a CR for that.

- this change necessitated some improvements to the Ticks/Tickspan 
interface - cc'ed the jfr team to look at these too:

   - some conversion methods from SI time units (s, ms, us, ns) into the 
internal (jlong) format.

   - shortened the conversion methods from internal units to SI time 
units to SI abbreviations s/ms/us/ns because particularly the latter 
that are commonly used for printing are very long.

   - all these methods that convert to SI time units now return the 
"raw" double result from the method. Almost all of the uses are timing 
critical (99.99% just printing), and the single other use already did FP 
calculations anyway now and then.

   - I avoided cluttering the Ticks/Tickspan interface with more 
methods: G1 does some statistics with Ticks/Timespan, so actually some 
more arithmetic operations would have been nice. These methods have been 
implemented using template specializations in WorkerDataArray without 
adding new methods in Ticks/Tickspan.

- there are no helper functions/uses for WorkerDataArray<double> any 
more (I changed the single one in Shenandoah because it was not worth 
keeping imho so I just converted to Tickspan).

- there is one particular change in workerDataArray.inline.hpp that 
required me to manually expand some MAX2/MIN2 macros in a template 
method because otherwise the solaris studio compiler generated bad code 
on SPARC (caught by a gtest). I will file a CR to fix this when/if the 
compiler or the solaris situation changes.

CR:
https://bugs.openjdk.java.net/browse/JDK-8208390
Webrev:
http://cr.openjdk.java.net/~tschatzl/8208390/webrev/
Testing:
hs-tier1-5, eyeballing results, building and testing Shenandoah locally, 
eyeballing results afaiu them.

Thanks,
   Thomas


More information about the hotspot-jfr-dev mailing list