Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

humanfriendly.Timer won't work properly if the system clock changes #13

Closed
mwgamble opened this issue Nov 15, 2016 · 1 comment
Closed

Comments

@mwgamble
Copy link

If the system clock changes while the Timer class is busy timing something, it won't track the elapsed time correctly. This could happen if daylight saving kicks in, or if someone travels across a border and their timezone changes, or for any number of other reasons, such as a NTP client kicking in and changing the time. A great example of this can be found in this video by Tom Scott: https://www.youtube.com/watch?v=QCwpVtqcqNE

The solution is to use a monotonic clock to track the time. Fortunately, python has built-in support for monotonic clocks: https://docs.python.org/3/library/time.html?highlight=time.monotonic#time.monotonic
There is also a polyfill available for when time.monotonic isn't available: https://pypi.python.org/pypi/monotonic

It would be great if Timer could be adjusted to use a monotonic clock so that it can properly track the elapsed time regardless of its conditions of use.

@xolox
Copy link
Owner

xolox commented Jan 16, 2017

Hi Matthew and thanks for the feedback! I was aware of time.monotonic() being available in Python 3.3 and higher, but I'd never encountered pypi.python.org/pypi/monotonic before. There's actually a sense of elegance to the monotonic package, in that it doesn't require compilation (binary code) and it's contained in a single Python module. I like!

Just now I released humanfriendly 2.3 which uses monotonic timers when available, whether via the new conditional monotonic dependency or time.monotonic(). If no monotonic clock is available it falls back to time.time(), but now only as a last resort :-).

I believe this resolves the problem you reported so I'm going to close this issue now. If you notice anything wrong, feel free to reopen this issue or open a new one.

@xolox xolox closed this as completed Jan 16, 2017
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

No branches or pull requests

2 participants