Skip to content
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

Duration Reform RFC #1040

Merged
merged 2 commits into from
Apr 27, 2015
Merged

Duration Reform RFC #1040

merged 2 commits into from
Apr 27, 2015

Conversation

wycats
Copy link
Contributor

@wycats wycats commented Apr 6, 2015


`Duration` implements:

* `Add`, `Sub`, `Mul`, `Div` which follow the overflow and underflow
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the Sub impl will panic if the result would be negative?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is the intention.

@brson
Copy link
Contributor

brson commented Apr 6, 2015

cc @lifthrasiir

```rust
pub struct Duration {
secs: u64,
nanos: u32 // may not be more than 1 billion
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean it is allowed to be exactly 1 billion, or is only 999,999,999 allowed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on

Nanoseconds […] carry into the secs field.

later in the proposal, the latter. But yeah, should be fixed.

@SimonSapin
Copy link
Contributor

👍

/// create a Duration from a number of milliseconds
pub fn from_millis(millis: u64) -> Timeout;

/// the number of seconds represented by the Timeout
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Timeout/Duration/

@aturon
Copy link
Member

aturon commented Apr 6, 2015

This is a clean and lean proposal that optimizes for the most important use cases, thereby sidestepping a lot of thorniness with the current type. 👍 from me.

can never exceed 1 billion or be less than 0, and carry into the
`secs` field.
* `Display`, which prints a number of seconds, milliseconds and
nanoseconds (if more than 0).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seconds, milliseconds and nanoseconds is the same unit with diffrent multipliers. Does this propose to display the same thing in three dirferent ways, e.g. Duration(15.0010000001s, 15001.000001ms, 15001000001ns)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's saying that durations would be printed like 15 seconds, 306 milliseconds, and 13 nanoseconds.

@nagisa
Copy link
Member

nagisa commented Apr 6, 2015

Overall 👍


* `Add`, `Sub`, `Mul`, `Div` which follow the overflow and underflow
rules for `u64` when applied to the `secs` field. Nanoseconds
can never exceed 1 billion or be less than 0, and carry into the
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

less than 0 is implied by the type, and as above this should never exceed 999,999,999.

@lifthrasiir
Copy link
Contributor

As the original author of Duration type, I too agree to the basic principle of this RFC: reducing Duration to what would be most important to the standard library. 👍

I still argue that the resulting type should not be called Duration. In fact, I think @wycats already thought about this possibility (the example code inconsistently mixes Duration and Timeout from editing mistakes). The Duration type as originally envisioned by Chrono is an instant-agnostic measure of time period (with a caveat of leap seconds), which doesn't fit well with the current Duration usage throughout libstd. Saturating secs method for example is apt for Timeouts but not so for Durations, in my humble opinion.

@netvl
Copy link

netvl commented Apr 7, 2015

Scala has Duration class which is used for concurrency and network code. I always found it very nice to work with.

Internally a duration is either an infinite duration or a finite duration which is represented as a pair (Long, TimeUnit). TimeUnit is an enum which represents a unit of time, e.g. second or millisecond or hour, and it also provides an ability to convert between units.

Duration can be constructed from a Long and a TimeUnit:

val d1 = Duration(5, TimeUnit.SECONDS);
val d2 = Duration(1, TimeUnit.HOURS);

There is also a convenient DSL implemented via implicits:

val d1 = 5.seconds
val d2 = 1.hour

Arithmetic operations on durations may "downgrade" the time unit, so the duration is always represented accurately. Because of such uniformity the max duration which may be represented by FiniteDuration is 2^63-1 nanoseconds, approximately 292 years. If Rust goes this way, we could use u64 which would double the maximum duration value.

@eternaleye
Copy link

@lifthrasiir I can see where you're coming from re: the Chrono Duration type, but I think the prior art of Joda is a pretty strong argument for the RFC's choice of terminology and behavior.

@netvl One interesting thing to consider (in the future extensibility sense) is that a 32-bit nanoseconds value has significant headroom - it may be possible to, in the future, either use the high bits as flag bits for 'infinite' &c, or use higher values as sentinels for special behaviors (including 'infinite').

One thing that immediately springs to mind: the high bit of nanos could be used to mark the whole duration as negative (avoiding "which one is signed" strangeness to an extent), and the next bit being set could indicate "special" values (infinite, different time unit for the u64, etc).

I'm not suggesting adding this to the RFC, but it's something to keep in mind IMO.

@wycats However, as a result of the above, I do think new() should be specified as either truncating nanoseconds to 10^9, overflowing them into seconds, or panicking on oversize values (it could be seen as a contract violation, and this would be my preference). The kernel learned that lesson re: unused flag bits in syscalls the hard way.

EDIT: To expand on the "flag bits in syscalls" thing, several Linux syscalls that took a "flags" parameter only checked for the defined flags when the syscall was introduced. As a result, programs passing garbage bits in undefined positions passed unnoticed, creating a back-compat hazard: Any future use of those bits would cause behavior changes in any program that passed garbage data, which used to silently succeed. These days, any new syscall is expected to error out if any undefined flag bits are nonzero, preventing the back-compat hazard from arising.

@Diggsey
Copy link
Contributor

Diggsey commented Apr 8, 2015

This is good, I think it's best to keep the Duration type as simple as possible, even in the future: instead of adding 'special values' to the Duration type to represent infinite durations or different units, I think a better route would be to wrap the Duration in an enum, which has cases Finite(Duration) and Infinite, and instead of different units, just add sufficiently many bits of precision that it can always use the highest precision unit.

This avoids the need to branch at all when adding and manipulating durations, it makes it simpler and easier to understand, and it doesn't cost anything as Durations are 128 bits in size, but only use 96 bits, so the compiler can easily use the remaining 32 bits for determinant values. There are also places where an infinite duration doesn't make sense.

@nagisa
Copy link
Member

nagisa commented Apr 8, 2015

If we had u128, I’d really want Duration to be picoseconds stored in an u128 (this is about 10¹⁹ years of representable time) so:

  1. There wouldn't be any special cases for values nanoseconds field can contain;
  2. Simpler code to get a value of desired precision.

@ryanhiebert
Copy link

From a back-seat perspective, @Diggsey 's comment that using an enum to wrap a duration for APIs that need it makes sense to me.

@eternaleye
Copy link

@Diggsey I agree it should (API-level) present as an enum; I'm more talking about how the repr of that enum might be able to use the headroom in nanos (and why, for extensibility reasons, new() should reject nanos > 10^9).

@aturon aturon self-assigned this Apr 9, 2015
@nrc nrc assigned wycats and unassigned aturon Apr 9, 2015
@wycats
Copy link
Contributor Author

wycats commented Apr 15, 2015

I updated this RFC, taking into consideration the comments here.

@aturon
Copy link
Member

aturon commented Apr 15, 2015

Thanks @wycats! I am personally quite happy with the simple, clean, and conservative approach here.

@netvl
Copy link

netvl commented Apr 15, 2015

I personally don't like that the proposal constantly refers to date/time libraries and joda-time in particular. The standard Duration type is not intended to allow working with dates in any way and it cannot be a foundation or a part of a proper date/time library, it can be and should be used only for specifying durations and timeouts for concurrency and network code, and so, for example, it is perfectly fine for it to have constructors accepting standard hours and minutes, without taking leap seconds and other quirks into account. I'd argue that days and other larger units are also fine.

And I also don't like that there is no way to work with milliseconds. Given the internal representation it absolutely makes sense to return an Option<u64>, adding a bignum-returning method later if it is necessary. Another approach to fix this is to restrict the maximum duration to u64::MAX nanoseconds. This gives approximately 459 years of duration. Given that Duration is anyway unsuitable for date/time calculations, 459 years is much more than enough for concurrency/network purposes. It also makes the implementation much simpler.

@eternaleye
Copy link

@netvl I disagree - In particular, I feel it's short-sighted to presume that the results of date calculations will not need to be used for durations and timeouts. A trivial example would be implementing cron in Rust; a good cron daemon should sleep until the earliest job, or wake up at a signal to recalculate jobs.

Using the joda-time definition gives a path forward - for now, a Duration type that while not sufficient for date calculations, has a definition compatible with a robust, tested API that is - meaning that when such an API arrives, interoperation "falls into the correctness pit."

@wycats Although, the RFC still seems to use Timeout as the return type of the Duration constructors, which made me do a double take :P

@netvl
Copy link

netvl commented Apr 15, 2015

@eternaleye,

I feel it's short-sighted to presume that the results of date calculations will not need to be used for durations and timeouts. A trivial example would be implementing cron in Rust; a good cron daemon should sleep until the earliest job, or wake up at a signal to recalculate jobs.

But this does not affect the built-in duration implementation at all. If you work with real world date and time (including such things as cron), you will need to use proper date/time library anyway, and such library will always have means to convert its duration representation to the one from the standard library (or at least it would be able to provide milliseconds/nanoseconds value which could be converted to the standard library duration).

The problem is, real world date/time manipulation is very complex, but most of this complexity is completely unnecessary for the main area where the built-in duration is used - that is, for threading and networking. We won't be able to combine real-world date/time handling and threading/networking duration anyway, and taking date/time libraries as a reference will only result in a half-baked API which would be unnecessarily generic and painful to use.

@eternaleye
Copy link

@netvl,
And I strongly disagree - yes, real-world date/time manipulation is complex, which is exactly why joda-time's definitions are what they are.

A good timeout format needs the following criteria:

  1. It must be absolute - how long it lasts cannot depend on when it starts
  2. It must be physical - how long it lasts must describe some real-world procession of time
  3. It must be uniform - adding one second to a three second timeout must yield a timeout with 4/3 the length.

The joda-time Duration meets all of these - it is absolute, measuring "seconds that pass" rather than offsetting into some human-meaningful "day" or such; it is physical, measuring SI seconds rather than some abstract monotonic clock; it is uniform, measuring SI seconds rather than UTC seconds with their leap seconds.

And the Duration type is simple - it is conceptually is just an SI time delta.

The Period type, then, is the "complex" date/time library equivalent - and Instant is the fundamental piece of the "conversion" operation.

@netvl
Copy link

netvl commented Apr 15, 2015

@eternaleye, I'm not sure what you're getting at. While joda-time Duration representation indeed is suitable for networking/threading purposes (as it is just a number of milliseconds; I'd argue that nanosecond precision would be nicer, but it doesn't affect the general idea), I meant that I'm against linking the built-in duration to any real-world date/time library at all because their use cases are very different. When we have a proper officially supported date/time library, we will always be able to convert from whatever Period implementation we will have there.

Anyway, units larger than second are used rarely, so I don't think the ability to create a "2 weeks" duration is that important. What is important is to have an ability to work with duration in terms of second fractions of various sizes efficiently - in particular, working with milliseconds. And that's why I think that we should take Java/Scala approach and represent the duration as a u64 of nanoseconds. For practical purposes it allows essentially unlimited durations, but it makes computations and working with fractions much easier. I don't see any drawbacks of such representation (maybe except the slightly more difficult conversion to timespec, but this is anyway trivial and amounts to two arithmetic actions at most) but benefits are obvious.

@eternaleye
Copy link

I'm not disagreeing over using u64 nanoseconds as the representation of Duration (though I think that if we go for integral nanoseconds, defining the valid range as 0..i64::MAX would be better so as to postpone the decision as to signed/unsigned a bit) - as far as I'm concerned, that's six of one and a half dozen of the other.

Unix userspace historically used the current layout but with microseconds, and has moved to the layout proposed. Linux kernelspace is moving to 64-bit nanoseconds, though I can't recall the signedness (I think they are signed). Windows uses milliseconds or 'ticks', and I don't know the signedness.

All have advantages and disadvantages.

My disagreement is over your dislike of the references to joda-time. I feel they are valuable in that they are:

  1. Citations of evidence that this proposal will not inhibit good date/time APIs in the future
  2. Laying out the expected behavior of a duration type, relative to the (wildly inconsistent) human ideas of time
  3. Illustrating why the expected behavior is what it is, and what is explicitly left to other pieces

@netvl
Copy link

netvl commented Apr 16, 2015

Citations of evidence that this proposal will not inhibit good date/time APIs in the future

But this proposal just can't inhibit good date/time API because they are/will be completely disjoint and so it is unnecessary to regard this duration type in the context of a date/time library at all! That's what my point is about.

Anyway, this discussion is getting increasingly more philosophical and disconnected from the actual proposal. I guess we should finish here.

rules for `u64` when applied to the `secs` field (in particular,
`Sub` will panic if the result would be negative). Nanoseconds
must be less than 1 billion and great than or equal to 0, and carry
into the `secs` field.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will it follow the rules of u64 or will it also panic in non-debug mode?

I'd vote for the latter, because these duration-manipulating methods are unlikely to be called in performance-sensitive code.

@tbu-
Copy link
Contributor

tbu- commented Apr 23, 2015

I like the proposal, thanks for writing it up!

@gkoz
Copy link

gkoz commented Apr 24, 2015

I'm curious (and I don't think it's explained in the RFC), why does the representation mimic timespec? Does it strive to fit any possible timespec value?

@aturon
Copy link
Member

aturon commented Apr 27, 2015

There's been a lot of positive feedback on this RFC, including from the original Duration author, and it seems reasonably clear to be a conservative proposal that can sit well with future development of the std::time module. I have merged the RFC.

As always, remember that this represents preliminary approval of the design. The reformed module will be placed under its own feature gate, and we will solicit feedback, and iterate based on that experience, before fully committing to the new API.

Thanks @wycats!

@netvl
Copy link

netvl commented Apr 28, 2015

Somewhat disappointing that no one commented on why seconds+nanoseconds is superior to just nanoseconds. It's not even in alternatives.

@nagisa
Copy link
Member

nagisa commented Apr 28, 2015

@netvl
We do not have a big enough integral type available yet and this structure has the same fields as C’s timespec structure which makes it easy and cheap to convert both ways.

@netvl
Copy link

netvl commented Apr 28, 2015

@nagisa, why u64 is not big enough? It gives over 500 years of duration. When you would need more, given that Duration should be used solely for timeouts and threading?

@netvl
Copy link

netvl commented Apr 28, 2015

And timespec compatibility is not very strong argument. It is not used by all API. I imagine that it is very possible to have e.g. Rust binding to some C library which accepts timeout in milliseconds. Windows API is also full of such functions, AFAIK.

@wycats
Copy link
Contributor Author

wycats commented Apr 28, 2015

@netvl Sorry that I missed your question!

My original design (when the type was called Timeout) was indeed a newtype of u64 (of nanos). I eventually became unconvinced of the benefit of maintaining two separate types (a Timeout in 1.0/1.1 plus a Duration to land later) and decided to propose a scope-restricted Duration.

While I didn't try to spec out exactly what a broader-scope Duration would look like, it seemed clear that limiting such a type to 500 years in perpetuity wasn't the right choice for a type intended to grow over time.

Can you say what benefits a Duration type that is just u64 of nanos would have?

@nagisa
Copy link
Member

nagisa commented Apr 28, 2015

Note that this is an implementation detail and can be changed anytime later without an RFC anyway back-compatibly.

Re u64:

It cannot represent all the values of timespec, hence restrincting use (useless or not) of system APIs to some extent.

@wycats
Copy link
Contributor Author

wycats commented Apr 28, 2015

@nagisa I wouldn't considering reducing the available representable time to be an allowed backwards-compatible change, but it sounds like we're on the same page 😄

@gkoz
Copy link

gkoz commented Apr 28, 2015

Isn't representing durations of hundreds of years at odds with the goal of avoiding "human complications"? Is e.g. 10e9 seconds a meaningful duration (for the purposes of a standard library, not a deep space probe) when you ignore those complications?

@netvl
Copy link

netvl commented Apr 29, 2015

@nagisa,

Note that this is an implementation detail and can be changed anytime later without an RFC anyway back-compatibly.

I wouldn't call it a purely implementation detail since it can have a considerable effect on the API (I mean conversion operations).

@gkoz,

Isn't representing durations of hundreds of years at odds with the goal of avoiding "human complications"?

I'm not sure what you mean. I only said that nanoseconds-based duration could support durations up to 2^64 nanoseconds which happens to be equal roughly to 450 years. Of course, internally it would be just nanoseconds.

@wycats,

Can you say what benefits a Duration type that is just u64 of nanos would have?

  1. It is conceptually simpler than timespec-like, so:
  2. arithmetic operations on it are much easier to implement,
  3. conversion to and from other fractions of second becomes natural.

Possible drawbacks (compared to timespec-like representation) are:

  1. not compatible with timespec directly - a very minor drawback, I think, as the conversion is several arithmetic operations and is not needed always and everywhere;
  2. smaller maximum duration size - I don't believe this is really a drawback because u64::MAX nanoseconds is absolutely sufficient for those purposes the duration type is going to be used.

I personally think that 3rd point is of utmost importance and together with 2 outweighs drawback by a very large margin. Again, such approach is simpler - I think this is also important. Java/Scala live with flat duration for a very long time very nicely and I don't know any other language/stack which is more suited to large-scale concurrent and network programming - the areas where this duration type is going to be used.

@netvl
Copy link

netvl commented Apr 29, 2015

@wycats,

it seemed clear that limiting such a type to 500 years in perpetuity wasn't the right choice for a type intended to grow over time.

But why? This duration type can't possibly be used for date/time library since it would need completely different and more complex approach, and for threading/networking purposes 500 years are more than enough.

@eternaleye
Copy link

This duration type can't possibly be used for date/time library since it would need completely different and more complex approach

I don't agree - I mean, the references to Joda-time make it pretty clear that this has essentially identical semantics to its Duration type. Largely because it did the sensible thing of separating physical delta time (Duration) from semantic interval time (Interval) from semantic point time (Instant).

@netvl
Copy link

netvl commented Apr 29, 2015

@eternaleye, Instant type does not make sense outside of date/time library and is not very useful for networking/threading purposes - and when we have a date/time library, it should provide necessary conversions anyway, for example, a duration until the given instance could be obtained like this:

let d: Duration = (futureInstance - Instance::now()).into();

Durations in concurrency/networking and durations in real-world date/time are different things, and we should not conflate them, especially given that no standard date/time library exists now and is not yet planned (AFAIK). Durations which are discussed here should be as simple as possible.

@eternaleye
Copy link

@netvl, I'm really not clear on what you're saying here.

The "more complicated" type you're talking about seems to be identical to what Joda-time calls an Interval, not a Duration.

What Joda-time calls a Duration is exactly such a simple type as you're describing: it is an integer count of uniform time units (seconds, nanoseconds, what-have-you) that avoids human issues like leap seconds because it counts time-that-passes, not time-between-points.

And I wasn't in any way saying an Instant type should be used in network/threading, so I'm confused as to why you bring it up - I was just listing how Joda-time splits time-handling responsibilities among the three types.

And the responsibilities it gives to Duration (that it's physical delta time, not semantic time of any kind) are the exact responsibilities that network/threading code wants in a timeout.

@wycats
Copy link
Contributor Author

wycats commented Apr 29, 2015

@eternaleye You said precisely what I was going to respond.

@netvl
Copy link

netvl commented Apr 29, 2015

The only thing I wanted to say is that because usage areas of this duration type (network/concurrency) and date/time library duration type (real world date/time processing) are almost disjoint, and so it does not make sense to unify them, now or in future, even if their internal representation is similar or identical. Consequently it is not necessary for this duration type to have all features the real-world duration type, for example, to allow representation of durations larger than 500 years.

@eternaleye
Copy link

@netvl, while your rationale is something I disagree with, I do think that making the Duration repr u64 (or i64) is a valid thing for a few reasons.

  1. The system APIs that deal with time in a duration-y way expect small values - thus, limiting Duration to i64::MAX would not meaningfully constrain usage (the system APIs that deal with time in an instant-y way should never create a Duration anyway...). (The fact that OS time in general uses one type for all three kinds of time makes this a bit harder to spot, but it's there.)
  2. If the API then has just a .nanos() method, that returns Option<some_integer_type>, we can always change the repr to something with a higher max value, because it can then return None on overflow.
  3. While timespec-format does make Unix usage more efficient, it makes Windows usage less so (admittedly, only slightly in both cases) - and while I'm a Linux guy through and through, one theme has been moving away from Unix-centrism in Rust.
  4. Timespec has relatively annoying alignment properties. It's 12 bytes (...except on systems with 32-bit time_t, in which case Rust doesn't match the system anyway), but 8-byte aligned on many architectures - so arrays will have padding.
  5. "Extending" a duration with another duration is something that has real uses, and that requires some relatively annoying math (sum nanos, div_rem by 10^9, add the div to seconds and set nanos to the rem, then sum seconds, etc.). Subtraction is worse.
  6. With u32 nanos constrained to 10^9, you have a semantic constraint not enforced by the underlying types, when with flat u64/i64 nanos that goes away.

@netvl
Copy link

netvl commented Apr 29, 2015

@eternaleye, very nice summary, thanks! I didn't even think about alignment issues.

@gkoz
Copy link

gkoz commented Apr 29, 2015

My original design (when the type was called Timeout) was indeed a newtype of u64 (of nanos). I eventually became unconvinced of the benefit of maintaining two separate types (a Timeout in 1.0/1.1 plus a Duration to land later) and decided to propose a scope-restricted Duration.

A Timeout that can get silently truncated or rounded in some APIs, has no need to store centuries but may need to support an "infinite" value seems dissimilar from an abstract Duration.

@eternaleye
Copy link

One thing I actually find myself wondering is whether .nanos() on a u64/i64-based Duration should actually be fn nanos<T: PrimInt>(&self) -> Option<T> (using PrimInt from rust-lang/num for clarity) and compare against T::MAX to decide whether to return None. Option resolves back-compat issues with extending the repr later, and the generic means that there'd be no need to add some .bigger_nanos() method - callers would just specify a larger type.

That would require a bit more in the way of generic integer programming than is available now, though.

@Centril Centril added T-libs-api Relevant to the library API team, which will review and decide on the RFC. A-time Proposals relating to time & dates. labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-time Proposals relating to time & dates. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.