Storing both "fasttime" and time in Ticks and Tickspan

[Kim Barrett]

> On Aug 24, 2018, at 8:03 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi all,
> 
>  storing of timestamps or durations in gc code is pretty messed up,
> almost every collector uses its own data type and time resolution to
> store time stamps and durations.
> 
> In the past this caused us quite a few headaches when somebody
> accidentally stored the durations in variables meant for time stamps
> and vice versa, or added together time stamps and durations.
> 
> So I was looking into unifying this a bit, and found that JFR did
> better than gc code, by having the Ticks and Tickspan types.
> 
> When looking at the code that for some reason I would like to ask
> about, if JFR is compiled in, they contain fields for a "regular"
> (monotonic) time and a "fast" time.
> 
> I understand the difference and the reasons to some degree, but the
> question I have is whether storing both is actually necessary?
> 
> All (JFR) code always uses either one or another, and if one is enabled
> the other seems to be never read from from code using them.
> 
> Actually if fasttime is disabled, the fasttime value is not updated;
> strangely the "regular" time is always updated, regardless of whether 
> it's enabled or not.
> Further, if JFR is not enabled in the build (compiled out), JFR always
> uses the "regulr" time, so it seems to be available anyway.
> 
> In a first change to change the timebase for G1 (and ZGC uses it
> throughout already), I started using Ticks/Tickspan everywhere, but I
> would like to ask about the reason of 
> 
> - storing both values (fast and regular) in the Tickspan/Ticks class
> always as it takes extra space for no apparent gain
> 
> - and in extension actually ever using the "fast" time as the "regular"
> one seems to be always available.
> 
> Is the performance advantage of the "fast" time which is only used on
> 32 bit x86 (non-Zero platforms) worth keeping it? Considering that the
> "regular" time is always taken in addition to this "fast" time.
> 
> On x86-64 we do not use the "fast" time at all although it should have
> the same performance benefits (which are the only benefits I can see).
> 
> Any comments?
> 
> Thanks,
>  Thomas

Thanks for bringing this up and digging into it a bit.

I hadn't noticed that the alternate mechanism only does anything
different when on x86 non-Zero platforms.

And it was only used by JFR until that was open-sourced, when the
existing Ticks/Tickspan was modified to use it too (for JFR).

The rdtsc helper do_time_measurements is subject to once in a blue
moon catastrophic miscalculation, because of an abnormal delay between
access to one time source and access to the other.

My experience over the years with user-level attempts to use TSC as a
time source have been universally bad, whether I was the implementor
of the attempt or not. It has a long history of the hardware folks
providing it in some form and then breaking it (TSC, variable TSC,
invariant TSC, different invariant TSC per socket, invariant TSC
synchronized at startup, ...). Even figuring out which of the various
domains one is operating in can be challenging.

For example, on my dev machine, in dmesg I see:

[    0.056000] TSC synchronization [CPU#0 -> CPU#1]:
[    0.056000] Measured 872086 cycles TSC warp between CPUs, turning off TSC clock.
[    0.056000] tsc: Marking TSC unstable due to check_tsc_sync_source failed

And yet, the ergonomics function in rdtsc_x86.cpp would by default
allow its use.  So I'm glad it's not being used for x64.

I remember there was a "fast user-space time" initiative for Linux
something like 12-15 years ago. I thought that actually finished a
long time ago. I also recall a hires time initiative around that time,
which I also think finished a long time ago. (Indeed, on my dev
machine, /proc/timer_list shows resolution = 1 nsecs and hres_active
true.)

For Windows, there's QueryPerformanceCounter, which is already being
used by os::elapsed_counter().

So I think the FastUnorderedElapsedCounterSource and everything
related to it should be removed.  (I think this includes
cpu/x86/rdtsc_x86.[ch]pp and UseFastUnorderedTimeStamps.)

Removing a few milliseconds of sleeping during the initialization of
this feature is a nice little startup benefit.