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