Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Avoid use of CFAbsoluteTime as much as possible. #87

Merged
merged 2 commits into from
Nov 27, 2014

Conversation

kgoodier
Copy link
Contributor

A helper class SPDYStopwatch is introduced here to abstract common
time management duties from the rest of the code.

Since CFAbsoluteTimeGetCurrent() is affected by clock and timezone
changes, it's use should be avoided. mach_absolute_time() is unaffected
by clock changes, but is susceptible to overflow issues (which can be
worked around) and doesn't increment while the system is asleep (which
cannot be worked around). However, it has more appropriate behavior.

Our use of CFAbsoluteTimeGetCurrent() for the timers is unavoidable,
since Cocoa timers are based on it. This seems like it would present
undesirable behavior for our deferrable and connect timers if the clock
were to change, but there's not much to do about it.

A helper class SPDYStopwatch is introduced here to abstract common
time management duties from the rest of the code.

Since CFAbsoluteTimeGetCurrent() is affected by clock and timezone
changes, it's use should be avoided. mach_absolute_time() is unaffected
by clock changes, but is susceptible to overflow issues (which can be
worked around) and doesn't increment while the system is asleep (which
cannot be worked around). However, it has more appropriate behavior.

Our use of CFAbsoluteTimeGetCurrent() for the timers is unavoidable,
since Cocoa timers are based on it. This seems like it would present
undesirable behavior for our deferrable and connect timers if the clock
were to change, but there's not much to do about it.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.49%) when pulling 7c1a4f1 on kgoodier:review/absolute-time into 02160a4 on twitter:develop.

After discussion, we decided to revert back to using CFAbsoluteTime for
timing operations. There are pros and cons of each approach, and it
boiled down to basically "keep doing what we were doing since it wasn't
obviously broken". This will keep the metadata timing values consistent.

The things being measured -- latency, connected time -- are really
wall-time measurements, and having them affected by sleep time is odd.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) when pulling d6d7a01 on kgoodier:review/absolute-time into 02160a4 on twitter:develop.

goaway added a commit that referenced this pull request Nov 27, 2014
Avoid use of CFAbsoluteTime as much as possible.
@goaway goaway merged commit bb02228 into twitter-archive:develop Nov 27, 2014
@kgoodier kgoodier deleted the review/absolute-time branch January 8, 2015 22:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants