apple/swift-metrics

[Question] Best Practice for Timer w/o Foundation.Date

Mordil opened this issue · 5 comments

While working with SwiftMetrics to implement a Timer I was left wondering what's the best way to capture the start/end without reaching for Foundation.Date?

My first instinct was to use NIO.TimeAmount or NIO.NIODeadline as they have the semantics I'm after of just grabbing references to .now() and finding the difference (and having the proper types for Timer.recordNanoseconds(_:) - but the documentation and usage of NIODeadline as a point in time is dirty.

So I looked into Dispatch.DispatchTimeInterval, but those APIs work with UInt64 rather than the desired Int64 to pass to Timer.recordNanoseconds(_:). This would require handling overflow and conversion.

In all other cases with Timer, things are generally handled pretty well for me (due to generics), but in the base situation of recordNanoseconds I'm left to have to handle this myself.

According to this thread #5 I feel like the best option is to check for possible overflow and then default to max.

For context, my use case is to grab .now() before executing an I/O operation, and then finding the new .now() when my callback is first executed.

ktoso commented

Let's see what @weissi thinks about using NIO's types as time source but personally I don't think it's a bad idea -- TimeAmount fitting more than Deadline though.

For what it's worth, let start = DispatchTime.now().uptimeNanoseconds could also be a good source for this, even though it's UInt64, you could:

According to this thread #5 I feel like the best option is to check for possible overflow and then default to max.

(This happens to be the time source I use nowadays, though hidden under some types of my own).

My recommendation would be DispatchTime.now() in the face of ambiguity, but I don't mind use of NIO's time types either.

+1

Do we think it'd be worth it to provide a generic overload of recordNanoseconds that handles the overflow & default to max logic, very similar to all the other higher units of time?

Also, isn't there a concern of fatal errors with all of these methods that convert the BinaryInteger to Int64?

Running this in the Swift REPL triggers a fatal error:

let overflow = UInt64.max
func overflowTest<DataType: BinaryInteger>(_ value: DataType) {
    let converted = Int64(value) // Fatal error: Not enough bits to represent the passed value
    print(converted)
}

I opened two PRs to address what I mentioned in my last comment.

  • #29 to cover guarding against UInt64 to Int64 conversion overflow
  • #30 to cover working with UInt64 APIs & Timer.recordNanoseconds