Skip to content

Comments

Use Swift's time types#962

Merged
ktoso merged 5 commits intoapple:mainfrom
yim-lee:iss955
Jun 9, 2022
Merged

Use Swift's time types#962
ktoso merged 5 commits intoapple:mainfrom
yim-lee:iss955

Conversation

@yim-lee
Copy link
Member

@yim-lee yim-lee commented Jun 9, 2022

Resolves #955

@yim-lee yim-lee requested a review from ktoso June 9, 2022 04:31
public static func now() -> Deadline {
uptimeNanoseconds(Deadline.Value(DispatchTime.now().uptimeNanoseconds))
public static var distantFuture: ContinuousClock.Instant {
.now() + Duration.effectivelyInfinite
Copy link
Member

Choose a reason for hiding this comment

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

are these correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm I think so (and there are tests)? It used to be Deadline(Int64.max).

Copy link
Member

@ktoso ktoso Jun 9, 2022

Choose a reason for hiding this comment

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

I checked and these may end up with problematic edge cases...

It used to be that "distant future" was the absolute max value possible, but now that isn't it:

  5> (Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max)).components
$R4: (seconds: Int64, attoseconds: Int64) = {
  seconds = 18446744073
  attoseconds = 709551614000000000
}
  6> (Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max)).components
$R5: (seconds: Int64, attoseconds: Int64) = {
  seconds = 27670116110
  attoseconds = 564327421000000000
}
  7> (Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max)).components
$R6: (seconds: Int64, attoseconds: Int64) = {
  seconds = 36893488147
  attoseconds = 419103228000000000
}
  8> (Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max)).components
$R7: (seconds: Int64, attoseconds: Int64) = {
  seconds = 46116860184
  attoseconds = 273879035000000000
}
  9> (Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max) + Duration.nanoseconds(Int64.max)).components
$R8: (seconds: Int64, attoseconds: Int64) = {
  seconds = 55340232221
  attoseconds = 128654842000000000
}

I think we may need to switch to Duration? and use nil as "potentially infinite"?

In other words we need to take care that the isEffectivelyInfinite works well... it is same as not having a timeout in most cases to be honest, so let's move to Duration? if that can work out?

Copy link
Member

Choose a reason for hiding this comment

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

Strike all that I tried it out and it works out since we cap inside the .nanoseconds

Thank you!

Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

Looks great, only concern about potentially needing to mark extensions on Duration and the typealias internal so folks don't get upset we're adding public API to "not our type".

@ktoso ktoso merged commit ee78097 into apple:main Jun 9, 2022
@yim-lee yim-lee deleted the iss955 branch June 9, 2022 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace our own TimeAmount with Swift.Duration

2 participants