SE-0329 (Second Review): Clock, Instant, and Duration

Evolution Proposal Reviews
1

Hello, Swift community.

The second review of SE-0329: Clock, Instant, Duration begins now and runs through January 17th, 2022.

The first review of this proposal produced a lot of feedback from the community, resulting in the proposal being returned for revision. The author has been working on those revisions in a pitch thread. The revised proposal contains a detailed log of those changes, but to briefly summarize, the proposal no longer includes promoting Date into the standard library, the .hours and .minutes specifications have been removed from Duration, some names have been improved, and some details of the protocols have been variously altered, generalized, or made more concrete.

This is the second review of this proposal. After the first review, the Core Team accepted the value of this proposal in principle and asked only for revisions to the details being proposed. Therefore, in this review, we are no longer asking for feedback about whether the problem is significant enough to warrant a change to Swift, and I've removed that question from the feedback template below.

Reviews are an important part of the Swift evolution process. All review feedback should either be on this forum thread or, if you would like to keep your feedback private, directly to the review manager. If you do email me directly, please put "SE-0329" somewhere in the subject line.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at:

https://github.com/apple/swift-evolution/blob/master/process.md

As always, thank you for contributing to Swift.

John McCall
Review Manager

9 Likes
2

On another quick read through:
I note that much of the prose was not updated for the change of adding a DurationProtocol and Duration associated type on InstantProtocol.
The actual API seems reasonable, but the Task.sleep(for: Duration) function should directly specify whether it is suspending or continuous.

3

The proposal prose is a bit long due to the nuance of the topic so to clarify; that particular short-hand is Continuous. Duration exists still as a concrete type in addition to being an associated type on InstantProtocol.

I am in the process of writing some of the initial drafts of documentation for this - and the default shorthands are particularly one point that will definitely be part of that.

4

We can update the proposal, too, just so that people who use it as documentation aren't misled.

4 Likes
5

I am very pleased with how this proposal has ultimately turned out. As I commented on the revised pitch, I think it fits much better now with the feel and direction of Swift and gets many major decisions correct. I've done some study of similar features in other languages and feel that this proposal is comparable to contemporary temporal libraries. I put extensive effort into the pitch stages and read through the latest revisions.

Specific comments/minor suggestions:

  • I agree with comments during the pitch phase that nil tolerance is confusable with zero tolerance, and I don't think that simply documenting the meaning well is a good substitute for a more intuitive API design. Here's one way I can see of handling it:

Add infinity as a static requirement of DurationProtocol and hence to Duration; the protocol can leave each conforming type to choose how it wants to represent the value internally, and for Duration it could be some unrepresentable bit pattern (can seconds and nanoseconds have discordant sign when representing a finite duration, for example?) or simply (seconds: .max, nanoseconds: .max) if there are no unrepresentable bit patterns—this takes away only an infinitesimal amount of the representable finite range of Duration. Then, change the parameter from tolerance: Duration? = nil instead to tolerance: Duration = .infinity—this reads naturally to convey the intended meaning.

  • I'm still confused as to why DurationProtocol requires only (Self, Int) -> Self arithmetic but Duration additionally offers (Self, Double) -> Self arithmetic, when both allow dividing two duration values and return a Double. This means that, for a non-Duration type, users can't turn around and multiply a duration by that Double result without casting, but Duration has no such limit. Since multiplying by a Double is evidently useful for the currency type Duration, I would suggest making it a requirement for all types that conform to DurationProtocol. Relatedly, since multiplication is commutative, I think it'd be semantically appropriate to offer (Int, Self) -> Int and (Double, Self) -> Int overloads also.
  • I like the convenience API sleep(for:), but in some minor bikeshedding, it's also perfectly clear (when read as colloquial English) to omit the preposition. "Sleep 10 seconds" and "sleep for 10 seconds" mean the same thing, and moreover the parameter type Duration reinforces the message. Therefore, the Swift API naming guideline to "omit needless words" would give preference to sleep(_:). It's also nice, I think, that Task.sleep(nanoseconds: 10) can be modernized to just Task.sleep(.nanoseconds(10)). On a related note, to clarify that the clock is continuous, perhaps the API could be declared sleep(_: ContinuousClock.Duration)—this implies it might be helpful also to have as part of the Clock protocol typealias Duration = Instant.Duration [edit: this typealias may also help those implementing conforming types that use a custom Instant.Duration not to accidentally write APIs that take arguments of the currency type].
9 Likes
6

I don't think I'm well versed enough in time APIs to have a strong opinion on this topic, but the proposed solution seem pretty solid to me!

WRT clocks in the standard library, would a clock relative to 1970 for unix times make sense to include in the standard library or would it make more sense to include this clock in swift-system. I ask specifically because I'm working on a wrapper for stat(2), but would like to provide a nicer API over timespec for mtime (and friends).

1 Like
7

IIRC the version I saw before used 32 bits for nanosecond field, what is the reason to switch to 64 bits for this field?

Are durations equatable? Are these durations equal?

Duration(seconds: 0, nanoseconds: 1000 * 10^9 + 1)
Duration(seconds: 1, nanoseconds: 999 * 10^9 + 1)
Duration(seconds: 2, nanoseconds: 998 * 10^9 + 1)
Duration(seconds: 1000, nanoseconds: 1)
8

Note that this version of the proposal doesn’t specify the underlying storage at all; the 64-bit properties are read-only and are not stated to be stored properties. There is no initializer like what you write the above and the user doesn’t have access to any APIs which would allow alternative representations of the same duration.

4 Likes
9

Any thoughts on what idiomatic usage of the built-in clocks would be for consumers?

The sample code in the proposal abstracts away the specific concrete Clock type:

try await someClock.sleep(until: .now.advanced(by: .seconds(3))

Would we expect folks to write something like this in practice?

try await ContinuousClock().sleep(until: .now.advanced(by: .seconds(3))
10

This would be equivalently expressed as

try await Task.sleep(for: .seconds(3)) // omitting `for` if my suggestion is adopted

As a side note, it was indicated in the description of revisions that sleep(for:) would use a continuous clock, but the text under review describes how sleep(nanoseconds:) uses different clocks on Darwin and Linux and does not specify that sleep(for:) uses strictly the continuous clock—I think this needs to be clarified.

6 Likes
11

I still think that the optional tolerance parameters lead to an unfortunate API where

sleep(..., tolerance: .none)

doesn't mean "no tolerance" but "default tolerance" which is quite confusing. This confusion could be easily avoided by making the parameter non-optional and requiring some .default or so value.

20 Likes
12

The naming of the optional parameter will occur any place that a type such as Duration is listed - for example if an Int? parameter is labeled as "count" the same issue occurs someFunction(count: .none) is equally as misleading, yet a nil parameter of an Int is quite meaningful and useful.

That being said; an indefinite duration can be made out of the underpinnings, and that indefinite value could be a requirement of the DurationProtocol. It would just semantically mean a value that is greater than any other reasonable duration. Which I think is sensible for any DurationProtocol adoption.

1 Like
13

That was proposed initially however due to the issues surrounding Date and it's naming that was decided to not be placed into the standard library. Sinking Date (the type that represents what you are talking about; a fixed point in time that is relative to an epoch) was considered - however the name was determined to be unsavory since it can easily be conflated with a calendrical date when the Calendar type is not present. Additionally there were some strong concerns with regards to leap seconds. The end result is that a clock and instant that functions as you are wanting will exist as a modification to Foundation - UTCClock.

14

as @xwu correctly pointed out: those initializers don't exist (and I presume you are meaning 1e9 not 10 xor 9).

let a: Duration = .seconds(0) + .nanoseconds(1000 * 1000000000 + 1)
let b: Duration = .seconds(1) + .nanoseconds(999 * 1000000000 + 1)
let c: Duration = .seconds(2) + .nanoseconds(998 * 1000000000 + 1)
let d: Duration = .seconds(1000) + .nanoseconds(1)

Those are all equal to each other; and share the same memory pattern. If you are interested in the backing storage the initial draft of the proposal can be found here: Clock/Instant/Duration by phausler · Pull Request #40609 · apple/swift · GitHub

3 Likes
15

Any thoughts on defining something like:

enum DurationTolerance<Duration : DurationProtocol> {
   case default
   case zero
   case coalescedWithin(Duration) 
}

and then passing that enum as the parameter to sleep(tolerance:)? That removes the .none ambiguity of Optional and gives some more specific names to the other cases (case names are just an example, but I do think the none ambiguity should be avoided if possible).

2 Likes
16

That isn't too bad; the only issue with that approach is that it would cost at least 2 bits more than the stored duration; which I think it is reasonable to ask for types that implement duration to specify an indefinite.

Given the two choices of wrapping an enum or requiring a specific value representing an indefinite tolerance. I am leaning to the latter. Which I think is a reasonable condition on acceptance for this.

17

Could we not work-around the leap seconds issue by introducing a SystemClock, as the system's notion of the current time, with whatever leap seconds handling it has. I don't know of any other languages' standard libraries which provide access to monotonic clocks and uptime clocks but not the system's reckoning of the current time. Do you know of any?

We could avoid the naming issues for now by simply referring to it as SystemClock.Instant. It could later be given a nicer name, such as Timestamp.

There are a lot of libraries which parse timestamps and need the ability to return those values as a convenient type. For example, async-http-client parses cookie expiration times, and UniqueID is able to extract the embedded timestamps from time-ordered UUIDs.

Whilst Foundation has split off FoundationNetworking and FoundationXML as separate modules, Date still lives in the main Foundation library, alongside types such as Calendar and DateFormatter which themselves depend on ICU. With the standard library now having dropped ICU, importing Foundation just to have Date as a currency type comes with a higher cost than ever before.

If this doesn't make it in to the standard library, it is inevitable that 3rd-party packages will step in to fill the gap. But that is not a better outcome for the Swift ecosystem, IMO - we would be better-served by a currency type which lives in the standard library. Providing those currency types is, after all, one of the standard library's main jobs.

IMO, defining the Clock protocol but leaving the system clock to other packages is like defining the Collection protocol but asking developers to bring their own Array.

2 Likes
18

A handful of minor issues aside, I think the proposal is great. I particularly appreciate the improvements over the initial review.

I only see three rough edges here:

  • Echoing @xwu's comment, I think adding a Duration type alias to Clock and then defining Task's convenience method as sleep(_: ContinuousClock.Duration) would significantly improve the clarity of the API — developers would see which clock is being used from the type signature without having to refer to external documentation.

  • I find the default behaviour of Clock.sleep allowing a clock-defined tolerance very surprising. I don't think I've ever come across a sleep-like API that doesn't default to "as close to the requested duration as possible". I definitely see the benefits of giving the implementation some leeway, but on the other hand I expect developers new to this API to be surprised by that, and then it'll become one more oddity that they'd need to keep track of as they switch between APIs.

    IIUC, I think there's even an example of that in the proposal text itself — the deprecation warnings for the existing Task.sleep methods should instead suggest that they be replaced with sleep(for: _, tolerance: .seconds(0)) to maintain their current behaviour. Otherwise, callers will be getting a default tolerance where previously they weren't.

  • I also find the tolerance: .none construct confusing. I'd instead suggest something along these lines for that functionality:

    protocol Clock {
  func sleep(until deadline: Instant, tolerance: Instant.Duration) async throws 

  var defaultTolerance: Instant.Duration { get }
}

extension Clock {
  func sleep(until deadline: Instant) async {
    sleep(until: deadline, tolerance: .defaultTolerance)
  }
}

    The key point being that a Clock implementation would be required to provide its default tolerance in addition to its minimum resolution, and then that value is used as a default value rather than Optional.none.

    In any case, I think having a Clock make its default tolerance programmatically visible would be a great improvement.

I think so, yes.

I've worked with timing APIs in C, Java, C#, Obj-C, and Swift for task scheduling and timeout handling since the late 90s. I think this proposal compares very favourably to any other timing API I've used.

I've followed both pitch threads (and their related proposals) very closely, but only managed to skim the original review thread.

19

One more point on tolerances — whichever way that discussion ends up, I think it'd be helpful to include a static .zero property on Duration.

20

That is already a thing: AdditiveArithmetic has a requirement of .zero

1 Like