RFR(M): 8028128: Add a type safe alternative for working with counter based data

[Vitaly Davidovich]

Similar issue with the Ticks versions.

Sent from my phone
On Nov 12, 2013 8:17 PM, "Vitaly Davidovich" <vitalyd at gmail.com> wrote:

> Yeah, naming things is often a pain.
>
> 51 inline bool operator<=(const Tickspan& lhs, const Tickspan& rhs) {
>   52   return !operator<(lhs,rhs);
>   53 }
>   54
>   55 inline bool operator>=(const Tickspan& lhs, const Tickspan& rhs) {
>   56   return !operator<(lhs,rhs);
>   57 }
>
> These look like bugs; first one should probably be !operator>(lhs,rhs) and
> !operator<(lhs,rhs), respectively.
>
> Sent from my phone
> On Nov 12, 2013 10:32 AM, "Markus Gronlund" <markus.gronlund at oracle.com>
> wrote:
>
>> Hi Vitaly,
>>
>>
>>
>> Thanks for you input!
>>
>>
>>
>> “You have some operator overloads for these that can compare them against
>> jlongs - doesn't that still make it easy to mix up what you're comparing?
>> Personally, since these are value types I'd make their API work only
>> against these new types and make caller create the right type for
>> comparison.”
>>
>> Thanks for spotting these – the compare against jlongs in from an older
>> version of the webrev where this was allowed – you are right, it should not
>> be allowed, I thought I had updated this, will remove them – many thanks
>> for spotting…
>>
>> As for the names,
>>
>> “Ticks and Tickspan don't seem too optimal of names.  What about
>> TimeInstance and TimeSpan?”
>>
>> I agree with you that the names could possibly be improved, believe me I
>> have tried to use multiple names – one thing I wanted was to emphasize that
>> these types (currently) are only based on counter’s (i.e ticks), and do not
>> represent any higher “time” abstraction. That is, the implementation is
>> tied to data from counters (based on ticks with a specific ticks
>> frequency). In my current context, I use these values to talk about “time”
>> (as you saw from the original post), but “time” can mean many things to
>> different people, esp. many implementations and clock sources.
>>
>> Also, there already exist a few “Time/Timer” types (although they are not
>> as good as I want them to be for current purposes), for example there is a
>> type called Timestamp (in runtime/timers.hpp).
>>
>> I have come to the conclusion that in order to address the overall time
>> abstractions in Hotspot, we would need to make a larger effort – this we
>> should indeed do, but it’s outside the current scope.
>>
>> Also I am pretty sure that when we do revisit the time abstractions, the
>> names for these types will most likely change – that made me leave the
>> names to emphasize the “ticks” sources.
>>
>> Thanks for your input.
>>
>> Best regards
>>
>> Markus
>>
>>
>>
>>
>>
>> *From:* Vitaly Davidovich [mailto:vitalyd at gmail.com]
>> *Sent:* den 12 november 2013 15:37
>> *To:* Markus Gronlund
>> *Cc:* serviceability-dev; hotspot-runtime-dev at openjdk.java.net;
>> hotspot-gc-dev at openjdk.java.net
>> *Subject:* Re: RFR(M): 8028128: Add a type safe alternative for working
>> with counter based data
>>
>>
>>
>> Hi Markus,
>>
>> Ticks and Tickspan don't seem too optimal of names.  What about
>> TimeInstance and TimeSpan?
>>
>> You have some operator overloads for these that can compare them against
>> jlongs - doesn't that still make it easy to mix up what you're comparing?
>> Personally, since these are value types I'd make their API work only
>> against these new types and make caller create the right type for
>> comparison.
>>
>> Just my $.02, feel free to ignore :).
>>
>> Thanks
>>
>> Sent from my phone
>>
>> On Nov 12, 2013 6:16 AM, "Markus Gronlund" <markus.gronlund at oracle.com>
>> wrote:
>>
>> Greetings,
>>
>>
>>
>> Kindly asking for reviews for the following changeset:
>>
>>
>>
>> Webrev: http://cr.openjdk.java.net/~mgronlun/8028128/webrev01/
>>
>>
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8028128
>>
>>
>>
>> *Description:*
>>
>>
>>
>> Currently, when working with counter based data, the primary data type
>> representation in the VM are raw jlong's. jlong's are employed to represent
>> both:
>>
>> 1. a single instance in time, for example a "now" timestamp
>> 2. a time duration, a timespan, for example the duration between "now" -
>> "then"
>>
>> Having a raw jlong type represent both of these shifts the responsibility
>> to the programmer to always ensure the correct validity of the counter
>> data, without any assistance either at compile time or at runtime.
>>
>> The event based tracing framework relies heavily on counter based data in
>> order to keep track of time and timing related information and we have seen
>> counter data errors where the representation sent to the tracing framework
>> has submitted the wrong time, i.e. a timespan when a time instant was
>> expected and vice versa.
>>
>> We should therefore add an alternative to jlongs for representing counter
>> based data. We should enlist the help of the type system to enforce the
>> correct data types at compile time as well as introduce strong runtime
>> asserts in order to help with the correct usage of time and to reduce the
>> number of potential bugs.
>>
>> I will therefore add two new types in src/share/vm/utilities:
>>
>> 1. "Ticks" - represents a single counter based timestamp, a single
>> instance in time.
>> 2. "Tickspan" - represents a counter based duration, a timespan,  between
>> two "Ticks"
>>
>>
>>
>> Please see the added files:
>>
>>
>>
>> -          src/share/vm/utilities/ticks.hpp
>>
>> -          src/share/vm/utilities/ticks.inline.hpp
>>
>> -          src/share/vm/utilities/ticks.cpp
>>
>>
>>
>> I have also updated some of the existing event based tracing "event
>> sites" to employ usage of these new types as well as updated a few places
>> in the event based tracing framework implementation to accommodate these
>> types.
>>
>>
>>
>> *Testing completed:*
>>
>>
>>
>> nsk.stress.testlist (UTE/Aurora)
>>
>> GC nightlies (Aurora)
>>
>> gc/gctests/* (UTE/Aurora)
>>
>> nsk.gc.testlist (UTE/Aurora)
>>
>> SpecJBB2013 (no regression)
>>
>> SpecJBB2005 (no regression)
>>
>> Kitchensink (locally Windows) runs for +> 9 days (still running…)
>>
>> Thanks in advance
>> Markus
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20131112/5fd80a4f/attachment.htm>