Skip to content

NetworkClock: fix duration arithmetic and improve descriptions - #125

Open
rpaulo wants to merge 1 commit into
apple:mainfrom
rpaulo:network-clock-fixes
Open

NetworkClock: fix duration arithmetic and improve descriptions#125
rpaulo wants to merge 1 commit into
apple:mainfrom
rpaulo:network-clock-fixes

Conversation

@rpaulo

@rpaulo rpaulo commented Aug 31, 2026

Copy link
Copy Markdown
Contributor
  • Fix float scaling, division, and shift operators to avoid premature integer truncation and preserve precision
  • Add hours/days factories, coarse-unit descriptions, fractional microseconds, and roundedUpMilliseconds helper
  • Make Instant subtraction return a NetworkDuration and render Instants using fine (sub-minute) units
  • Cache nanosecond-per-unit constants and expand test coverage

@rpaulo rpaulo added the ⚠️ semver/major Breaks existing public API. label Aug 31, 2026
@rpaulo
rpaulo force-pushed the network-clock-fixes branch from cfd5092 to 8a01ebe Compare August 31, 2026 23:49
- Fix float scaling, division, and shift operators to avoid premature
  integer truncation and preserve precision
- Add hours/days factories, coarse-unit descriptions, fractional
  microseconds, and roundedUpMilliseconds helper
- Make Instant subtraction return a NetworkDuration and render Instants
  using fine (sub-minute) units
- Cache nanosecond-per-unit constants and expand test coverage
@rpaulo
rpaulo force-pushed the network-clock-fixes branch from 8a01ebe to af3309b Compare September 1, 2026 00:24

@agnosticdev agnosticdev left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I would recommend going through and adding & to all of the places where its safe to do so since these values are often used in the hot path. It doesn't seem like much but it removes a surprising amount of instructions when you do so.

@rnro rnro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good overall, I agree with Matt's comment about &+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ semver/major Breaks existing public API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants