Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow specifying the function to use to get the current timestamp #558

Closed
wants to merge 1 commit into from

3 participants

@mgenti

In our application where we use Tornado we routinely synchronize our system clock with an NTP server. Since this application is on an embedded device our system clock is very sloppy. If the time sync happens while the Tornado application is running, events may not run as expected. To help with that we have the IOLoop use a function that returns a timestamp that is monotonic.

This particular commit allows a user to specify the function to use to get the timestamp but by default uses time.time.

@mgenti mgenti Pass in a timestamp function to use
When creating an IOLoop instance, allow specifying the function to use
to get the current timestamp.
c981dfd
@bdarnell
Owner

This seems like a good idea, especially with the new monotonic clock functions coming in python 3.3 (pep 418). However, backwards compatibility is a problem - the absolute form of IOLoop.add_timeout won't work on an IOLoop that's using something other than time.time. In order to make this change we'd need to basically deprecate the common practice of add_timeout(time.time() + offset). We'd need to make sure that tornado itself uses the relative form where appropriate, and expose the time function from IOLoop so it can be used to construct absolute timestamps where needed (as in your change to PeriodicCallback). This sort of change would probably need to wait for tornado 3.0.

@mgenti

I would hope that any user that goes to the trouble to specify a different timestamp function would know not to call add_timeout with time.time(). ;)

@bdarnell
Owner

Yes, but sometimes tornado calls add_timeout for you (e.g. in the httpclients), and when it does it currently uses time.time().

@apenwarr

See also: #583

@bdarnell bdarnell referenced this pull request from a commit
@bdarnell bdarnell Add time_func parameter to IOLoop, and make it possible to use time.m…
…onotonic.

This means that calls to IOLoop.add_timeout that pass a number must be
updated to use IOLoop.time instead of time.time.

There are still some places where we use time.time in the code, but they
are either places where wall time is desired, or non-critical deltas (e.g.
printing elapsed time at the end of a request).

Thanks to apenwarr and mgenti for pull requests and discussion relating to
this change. (#558 and #583)
20deb5c
@bdarnell
Owner

I've committed a time.monotonic change drawing on both pull requests. Like mgenti's version, I added a time_func argument to IOLoop's constructor. Instead of a monotonic argument to add_timeout, I simply said that all calls to add_timeout must be relative to the IOLoop's clock. This is going to be harder to audit for, but since use of a monotonic clock is opt-in I don't think it's too unreasonable (but I'm open to feedback on this point; it's not too late to change). Thanks to both of you for the pull requests and feedback.

@bdarnell bdarnell closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 3, 2012
  1. @mgenti

    Pass in a timestamp function to use

    mgenti authored
    When creating an IOLoop instance, allow specifying the function to use
    to get the current timestamp.
This page is out of date. Refresh to see the latest.
Showing with 8 additions and 7 deletions.
  1. +8 −7 tornado/ioloop.py
View
15 tornado/ioloop.py
@@ -107,7 +107,7 @@ def connection_ready(sock, fd, events):
# Global lock for creating global IOLoop instance
_instance_lock = threading.Lock()
- def __init__(self, impl=None):
+ def __init__(self, impl=None, timefunc=time.time):
self._impl = impl or _poll()
if hasattr(self._impl, 'fileno'):
set_close_exec(self._impl.fileno())
@@ -120,6 +120,7 @@ def __init__(self, impl=None):
self._stopped = False
self._thread_ident = None
self._blocking_signal_threshold = None
+ self.timefunc = timefunc
# Create a pipe that we send bogus data to when we want to wake
# the I/O loop when it is idle
@@ -271,7 +272,7 @@ def start(self):
self._run_callback(callback)
if self._timeouts:
- now = time.time()
+ now = self.timefunc()
while self._timeouts:
if self._timeouts[0].callback is None:
# the timeout was cancelled
@@ -379,7 +380,7 @@ def add_timeout(self, deadline, callback):
Instead, you must use `add_callback` to transfer control to the
IOLoop's thread, and then call `add_timeout` from there.
"""
- timeout = _Timeout(deadline, stack_context.wrap(callback))
+ timeout = _Timeout(deadline, stack_context.wrap(callback), self.timefunc)
heapq.heappush(self._timeouts, timeout)
return timeout
@@ -441,11 +442,11 @@ class _Timeout(object):
# Reduce memory overhead when there are lots of pending callbacks
__slots__ = ['deadline', 'callback']
- def __init__(self, deadline, callback):
+ def __init__(self, deadline, callback, timefunc):
if isinstance(deadline, (int, long, float)):
self.deadline = deadline
elif isinstance(deadline, datetime.timedelta):
- self.deadline = time.time() + _Timeout.timedelta_to_seconds(deadline)
+ self.deadline = timefunc() + _Timeout.timedelta_to_seconds(deadline)
else:
raise TypeError("Unsupported deadline %r" % deadline)
self.callback = callback
@@ -485,7 +486,7 @@ def __init__(self, callback, callback_time, io_loop=None):
def start(self):
"""Starts the timer."""
self._running = True
- self._next_timeout = time.time()
+ self._next_timeout = self.io_loop.timefunc()
self._schedule_next()
def stop(self):
@@ -506,7 +507,7 @@ def _run(self):
def _schedule_next(self):
if self._running:
- current_time = time.time()
+ current_time = self.io_loop.timefunc()
while self._next_timeout <= current_time:
self._next_timeout += self.callback_time / 1000.0
self._timeout = self.io_loop.add_timeout(self._next_timeout, self._run)
Something went wrong with that request. Please try again.