-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Having a Great Time #4556
Comments
Theoretically Time spans should have xxseconds and months. It cover all known (for me) cases. See http://www.w3resource.com/sql/data-type.php#INTERVAL for details. |
It won't cover adding a conceptual day over switching of DST. Say you have a datetime on the last day of DST and want to add the timespan of one conceptual day, this timespan does not equal to |
I think this is overcomplicating things, we should have a single However nanosecond precision and storing a monotonic clock are both things that i'd like to see |
The goals are:
Dealing with a single Actually, having a separate Int64 (seconds) and Int32 (nanoseconds) makes computations a little more complicated (and probably slow). I wish we had support for Int128 in Crystal (I tried to add it, but it's tricky) which LLVM would optimize :) I'm not sure about the "transparent" monotonic clock. It would require 2 system calls (1 realtime, 1 monotonic) then store these 2 values (at least 2 × long). Can the nanosecond precision of the monotonic clock really be trusted for the realtime? |
@ysbaddaden I tried to add Int128 a while ago and it worked :-) I just didn't mention/commit or suggested it because I think it's not well supported in all platforms. Plus parsing of Int128 without support for Int128 is a bit tricky (but we can leave that for a later release). I can try to do it again (I lost that branch, but it was not much code) and send a PR and we can see how well it works in other platforms, if that helps with the implementation of Time. |
And BTW I agree with all comments from @RX14: I'd like to keep it simple (just |
I'm not entirely sure what you mean. The monotonic clock wouldn't be used for calculations of wall time at all. I was simply suggesting that the precision of the wall clock ticks could be kept the same or reduced (not sure about that) since nanosecond precision is most likely used for calculating time spans instead of for a really accurate wall clock. NTP is only so accurate and few computers have atomic clocks installed so nanoseconds on the wall clock is very unlikely to actually mean anything. Although I think maybe it's best just not to worry about the size of |
I did not intend on suggesting a port of Java's DateTime API to Crystal. But the fact that such an fundamental part of the stdlib was introduced into a very mature language and is based on a 3rd party package that had been the de-facto standard prior to Java 8, makes it an interesting source of inspiration worth taking a look. They have undoubtedly put some thinking into it. Then again, of course, it is Java, they inherently need to overcomplicate things ;) While I'm not particularly qualified for detailed reasoning about the internals, I have some ideas about the user-exposed API and its current shortcomings: I would like to see a way to express different units of time, say to represent a date without time. This might require a different class ( And some improvement could be made to time intervals. Currently, |
I'm revising my argument (I was deep inside ticks and nanoseconds). Before going further, I believe we should stick to dates representable as ISO8601, where 1 minute = 60 seconds, 1 hour = 60 minutes, and 1 day = 24 hours, with offset in minutes. We shouldn't have to deal with other formats, leap seconds (that's the system's job), or whatever (only how many days per month). Furthermore, this is how the underlying systems work. An advantage of individual attributes, is that we can represent any kind of date (thought still limited by year integer size), and accessing individual fields doesn't need special computation. An ISO8601 date fits in a 128 bits struct, allowing -32768 to +32768 years —i.e. larger than the 1583-9999 range of ISO8601. struct Time
@year : Int16
@month : Int8
@day : Int8
@hour : Int8
@minute : Int8
@second : Int8
@nanosecond : Int32
@offset : Int16
@kind : Kind # Int8
end A disadvantage is that adding or substracting times or a duration ( I'm not saying we should absolutely follow this model, but that it stands valid. After all, we represent dates more than we add or substract dates, especially if we introduce monotonic clocks to measure elapsed time, which should also be used for timeouts. I don't like merging a monotonic and wall time inside a single struct: I usually need to represent dates or measure elapsed time, not both. It may be nice to have both, like getting start and stop times, computing the duration (monotonic clock) and printing both start/stop times (wall clock), but this doesn't apply to all the other cases where we only need to represent a date (e.g. I'm more in favor to introduce solutions for measuring elapsed time (monotonic) or checking whether something did timeout, which is the only meaningful representation of monotonic clocks. For example: duration = Time.measure { do_heavy_work } # => Time::Span |
@ysbaddaden I think users shouldn't have to think about whether they want a wall clock or a monotonic clock (they'll get it wrong) and crystal should do the right thing by default. Usually the |
Programmers should not have to think about the specific implications, this is true. But they need to have the right tools for what they are doing. What is the so called right thing? For measuring purposes, a monotonic clock is required. For everything related to presenting dates and times for/from users, we need a wall clock. Go currently only supports real time because the language was initially intended for Googles production servers on which the wall clock never resets. On other systems this has led to major issues caused by leap seconds (including Cloudflare's DNS failure last year). The approach is quite interesting: Because of Go 1 compatibility they were unable to modify the API of Go's stdlib. Instead they changed the representation of the Time struct to contain both monotonic and real time. While this approach is feasible, it was primarily dictated by API compatibility. In Crystal this limitation does not exist. I think it oversimplifies things on the outside and make it complex on the inside that can lead to errors. Measuring and representing dates and times are entirely different things. They should be separate concepts inside Crystals stdlib and provide the right APIs for their purpose so users will naturally take the right one at the right place. To support @ysbaddaden argument, I don't think there is such a huge disadvantage in having individual attributes: Calculations concerning dates usually involve conceptual durations like Ok, reading it again, he already mentioned that. |
@straight-shoota I'm not sure Go did that because of backwards compatibility. They add more APIs, types and functions to newer Go releases, so they could have added some type for monotonic time exclusively. However, I believe they put monotonic time inside their current Time type because it's more intuitive and users will be happier and less confused. Of course it's more work and a bit tedious for the programmer implementing that, but that's far fewer people that the ones how are going to use it. And if you dig into Ruby's source code and APIs you'll learn that Ruby is the same way: optimize for user happiness, not implementor happiness (Crystal tries to be the same). Say I want to now how much time takes an operation. I have time = Time.now
# Some operation...
puts Time.now - time It would be very convenient if that would work out of the box and give a correct result. I use that code all of the time, and I believe many other programmers too (it's the most intuitive way to measure elapsed time). I wouldn't like to know later, somehow, that I should have used some other type because "details I don't really care about, I just want it to work". So, at least I won't accept a solution that introduced a separate type other than Time. I'd like Time to behave like in Go, having both measures (I think the monotonic measure is included in |
They did it because they wanted all existing code using the current API to measure elapsed time to work correctly without changes. It looks like an easy and user-friendly solution, this is true. But I fear it can also cause trouble and lead to unexpected results when When time differences are calculated on monotonic time if it is available on both, this can produce the following paradoxon: time1 = Time.now # time1 has both wall and mono time
time2 = Time.new(time1.ticks) # time2 has only wall time
time3 = Time.now
time1 == time2 # => true - compared using wall
time3 - time1 == time3 - time2 # => false - if wall and mono in differ in time1
# the previous line is effectively doing this:
time3.mono - time1.mono == time3.wall - time2.wall I wouldn't want to be surprised in that way by the implicit behavior of Any programmer who expects accurate results from elapsed time calculations should know that What your are doing in your example could also be accomplished by a dedicated API for time measuring such as clock = Clock.now # this is guaranteed to use elapsed time
# Some operation
puts clock - Clock.now
# or
puts clock.stop If there is an easy and elegant interface that gets heavily promoted as the sacred method to measure elapsed time in Crystal, people will pick it up and do things this way rather than using |
I think that @straight-shoota is right, and in Go would only be a monotonic time, if not compatibility issues. Realtime is secondary. |
@asterite I'd like to avoid having a separate # good & slim:
elapsed = Time.measure { do_something }
# bad & noisy
start = Time.now
do_something
elapsed = Time.now - start Just like we have & recommend # excellent
Benchmark.ips { test }
# horribly bad
start = Time.now
100.times { test }
elapsed = start - Time.now I'd like stdlib to provide a clock = Time::Measure.new
clock.elapsed # 00:00:01
clock.elapsed?(1.minute) # false |
I have an issue when storing an offset: adding or substracting from a Time may cross a DST change (or sometimes a TZ change) thus change the offset, which we can't reflect. For example in Europe/Paris, the following time will have a +60 minutes offset (winter), even though April is summer time (+120 minutes): Time.new(2017, 1, 1) + 90.days # => 2017-04-01 00:00:00 +01:00 Ruby does just that (keep the offset), but Rails behaves correctly (it recomputes the offset). It would seem they always transform a time to local time, or at least to the configured location (whatever # ruby keeps the offset as-is:
Time.new(2017, 1, 1) + 90 * 86400 # => 2017-04-01 00:00:00 +01:00
require "active_support/all"
# rails recomputes the offset:
Time.new(2017, 1, 1) + 90.days # => 2017-04-01 00:00:00 +02:00
# arbitrary offset -> local time
Time.parse("2017-01-01T00:00:00+02:00") # => 2017-12-31 23:00:00 +01:00 Does it look like an acceptable solution? |
I must say, I have become favourable of the idea to get rid of time zones as internal value altogether. As quoted from https://unix4lyfe.org/time/:
Absolute time values could be exclusively stored in a standard time zone (=UTC) and offsets are only applied when they are converted to a human-readable representation. Time zones are only necessary to make time values better understandable for humans. Otherwise there is no distinction between the same instant expressed in different offsets. It just makes calculations harder, because you have to consider changes in time zone offsets such as switching from/to DST. Time values in UTC are distinct, whereas certain values in other time zones can be ambiguous. For example, when using DST the clock is usually set back one hour once a year. So all instants of this hour in the specific time zone can relate to two different instances in UTC. The usual strategy is to arbitrarily convert to the later one, but this will always have some unexpected effects like Conversions from UTC are injective: If all zonal time values are immediately converted to UTC when entering For database systems it is considered good practice to store times exclusively in UTC. In most use cases there is no benefit from keeping the time zone which was used to determine the time value in the first place around for later use. An absolute time value will usually be presented in the local time zone of the user at the point of access, not at the point of creation. For recording and displaying absolute points in time this approach should be sufficient in any means. It is quite easy to implement and should be the preferred way for this use case. It does however create some trouble for calculations with conceptual time spans, because you cannot easily add an amount of Maybe it would be a good solution to store the time value (in UTC for the above reasons) alongside a time zone identifier which can be accessed for calculations when necessary. |
Counter example: Time.now.at_beggining_of year In Europe/Paris this should return January 1st at 00:00 with +01:00 offset expressed as local time, not January first at 01:00 with +01:00 offset because we computed from UTC then converted to local time. |
This it what I was talking about with respect to conceptual time spans. Your example essentially |
Yes, recording and later representation of time will likely be different so we should record time expressed as UTC (e.g. UNIX timestamp). Yet time instances seem different, they operate on time (addition, substraction, arbitrary change) and it's always conceptual, as you perfectly explained, and relative to the user's representation of time (i.e. its location). Hence why I'm more keen to always transform to local time. |
Okay, I think we can agree, that an (absolute) time value should always have a reference to a local time zone. And then there is the case of floating time, i.e. with no direct correlation to a specific instant. Which can be time-zone independent (like an alarm at 8:00 in the morning, in whatever time zone applies) or based on entirely different scale (like 2:31 in a cat video on Youtube). While this could theoretically be expressed with a "special" nil timezone, I feel more like treating this as something completely different through type safety. |
This really should be split into multiple issues, general issues with little actionable specifics should be avoided. A lot of this has been implemented anyway. |
Time in stdlib is found to be lacking in different ways and there have been some mentions of possible improvements
I've done some research on time and date solutions in other languages and as far as I can tell, the Joda Time based Date-Time API in Java 8 looks like a good source of inspiration.
A few key concepts (as far as I understand it):
Time
andDate
, withDateTime
being a composite. Internally, the values are represented as individual fields (hour, minute, second : Int8, nano : Int32
,year : Int32, month, day : Int16
)Local(Date/Time)
(no timezone) andLocalDateTime
is wrapped inZonedDateTime
to add a timezone.Duration
(accurate, time based) orPeriod
(conceptual, date based).I'm not familiar enough with Crystal internals and don't know if and how these ideas could be useful in Crystal, but this could be a good place to discuss plans for improvements of Crystal's time API.
Other notable resources (other items might be appended):
The text was updated successfully, but these errors were encountered: