Replaced internal TimeService with simple formatted RFC822 time cachi…

…ng (#2176)

Thanks for @greg-barnett for the code I picked-up.

aiohttp/helpers.py

@@ -16,7 +16,6 @@
 from collections import namedtuple
 from math import ceil
 from pathlib import Path
-from time import gmtime
 from urllib.parse import quote
 from urllib.request import getproxies
 
@@ -610,67 +609,30 @@ def is_ip_address(host):
                         .format(host, type(host)))
 
 
-class TimeService:
+_cached_current_datetime = None
+_cached_formatted_datetime = None
 
-    def __init__(self, loop, *, interval=1.0):
-        self._loop = loop
-        self._interval = interval
-        self._time = time.time()
-        self._loop_time = loop.time()
-        self._count = 0
-        self._strtime = None
-        self._cb = loop.call_at(self._loop_time + self._interval, self._on_cb)
-
-    def close(self):
-        if self._cb:
-            self._cb.cancel()
-
-        self._cb = None
-        self._loop = None
-
-    def _on_cb(self, reset_count=10*60):
-        if self._count >= reset_count:
-            # reset timer every 10 minutes
-            self._count = 0
-            self._time = time.time()
-        else:
-            self._time += self._interval
 
-        self._strtime = None
-        self._loop_time = ceil(self._loop.time())
-        self._cb = self._loop.call_at(
-            self._loop_time + self._interval, self._on_cb)
+def rfc822_formatted_time():
+    global _cached_current_datetime
+    global _cached_formatted_datetime
 
-    def _format_date_time(self):
+    now = int(time.time())
+    if now != _cached_current_datetime:
         # Weekday and month names for HTTP date/time formatting;
         # always English!
-        # Tuples are contants stored in codeobject!
+        # Tuples are constants stored in codeobject!
         _weekdayname = ("Mon", "Tue", "Wed", "Thu", "Fri", "Sat", "Sun")
-        _monthname = (None,  # Dummy so we can use 1-based month numbers
+        _monthname = ("",  # Dummy so we can use 1-based month numbers
                       "Jan", "Feb", "Mar", "Apr", "May", "Jun",
                       "Jul", "Aug", "Sep", "Oct", "Nov", "Dec")
 
-        year, month, day, hh, mm, ss, wd, y, z = gmtime(self._time)
-        return "%s, %02d %3s %4d %02d:%02d:%02d GMT" % (
+        year, month, day, hh, mm, ss, wd, y, z = time.gmtime(now)
+        _cached_formatted_datetime = "%s, %02d %3s %4d %02d:%02d:%02d GMT" % (
             _weekdayname[wd], day, _monthname[month], year, hh, mm, ss
         )
-
-    def time(self):
-        return self._time
-
-    def strtime(self):
-        s = self._strtime
-        if s is None:
-            self._strtime = s = self._format_date_time()
-        return self._strtime
-
-    @property
-    def loop_time(self):
-        return self._loop_time
-
-    @property
-    def interval(self):
-        return self._interval
+        _cached_current_datetime = now
+    return _cached_formatted_datetime
 
 
 def _weakref_handle(info):

aiohttp/test_utils.py

@@ -556,19 +556,12 @@ def make_mocked_request(method, path, headers=None, *,
     if payload is sentinel:
         payload = mock.Mock()
 
-    time_service = mock.Mock()
-    time_service.time.return_value = 12345
-    time_service.strtime.return_value = "Tue, 15 Nov 1994 08:12:31 GMT"
-
     @contextmanager
     def timeout(*args, **kw):
         yield
 
-    time_service.timeout = mock.Mock()
-    time_service.timeout.side_effect = timeout
-
     req = Request(message, payload,
-                  protocol, payload_writer, time_service, task, loop,
+                  protocol, payload_writer, task, loop,
                   client_max_size=client_max_size)
 
     match_info = UrlMappingMatchInfo({}, mock.Mock())

aiohttp/web.py

@@ -268,7 +268,7 @@ def cleanup(self):
     def _make_request(self, message, payload, protocol, writer, task,
                       _cls=web_request.Request):
         return _cls(
-            message, payload, protocol, writer, protocol._time_service, task,
+            message, payload, protocol, writer, task,
             self._loop,
             client_max_size=self._client_max_size)
 

aiohttp/web_protocol.py

@@ -50,8 +50,6 @@ class RequestHandler(asyncio.streams.FlowControlMixin, asyncio.Protocol):
     status line, bad headers or incomplete payload. If any error occurs,
     connection gets closed.
 
-    :param time_service: Low resolution time service
-
     :param keepalive_timeout: number of seconds before closing
                               keep-alive connection
     :type keepalive_timeout: int or None
@@ -107,7 +105,6 @@ def __init__(self, manager, *, loop=None,
         self._loop = loop if loop is not None else asyncio.get_event_loop()
 
         self._manager = manager
-        self._time_service = manager.time_service
         self._request_handler = manager.request_handler
         self._request_factory = manager.request_factory
 
@@ -163,10 +160,6 @@ def __repr__(self):
             self.__class__.__name__, meth, path,
             'connected' if self.transport is not None else 'disconnected')
 
-    @property
-    def time_service(self):
-        return self._time_service
-
     @property
     def keepalive_timeout(self):
         return self._keepalive_timeout
@@ -368,9 +361,7 @@ def _process_keepalive(self):
 
         # all handlers in idle state
         if len(self._request_handlers) == len(self._waiters):
-            # time_service.loop_time is ceiled to 1.0, so we check 2 intervals
-            now = self._time_service.loop_time
-            if (now + self._time_service.interval * 2) > next:
+            if self._loop.time() > next:
                 self.force_close(send_last_heartbeat=True)
                 return
 
@@ -491,7 +482,7 @@ def start(self, message, payload, handler):
                         if self._keepalive and not self._close:
                             # start keep-alive timer
                             if keepalive_timeout is not None:
-                                now = self._time_service.loop_time
+                                now = self._loop.time()
                                 self._keepalive_time = now
                                 if self._keepalive_handle is None:
                                     self._keepalive_handle = loop.call_at(
@@ -554,7 +545,7 @@ def handle_error(self, request, status=500, exc=None, message=None):
     def handle_parse_error(self, writer, status, exc=None, message=None):
         request = BaseRequest(
             ERROR, EMPTY_PAYLOAD,
-            self, writer, self._time_service, None, self._loop)
+            self, writer, None, self._loop)
 
         resp = self.handle_error(request, status, exc, message)
         yield from resp.prepare(request)

aiohttp/web_request.py

@@ -61,7 +61,7 @@ class BaseRequest(collections.MutableMapping, HeadersMixin):
     POST_METHODS = {hdrs.METH_PATCH, hdrs.METH_POST, hdrs.METH_PUT,
                     hdrs.METH_TRACE, hdrs.METH_DELETE}
 
-    def __init__(self, message, payload, protocol, writer, time_service, task,
+    def __init__(self, message, payload, protocol, writer, task,
                  loop,
                  *, client_max_size=1024**2,
                  state=None,
@@ -81,7 +81,6 @@ def __init__(self, message, payload, protocol, writer, time_service, task,
         self._post = None
         self._read_bytes = None
 
-        self._time_service = time_service
         self._state = state
         self._cache = {}
         self._task = task
@@ -134,7 +133,6 @@ def clone(self, *, method=sentinel, rel_url=sentinel,
             self._payload,
             self._protocol,
             self._writer,
-            self._time_service,
             self._task,
             self._loop,
             state=self._state.copy(),
@@ -402,11 +400,6 @@ def keep_alive(self):
         """Is keepalive enabled by client?"""
         return not self._message.should_close
 
-    @property
-    def time_service(self):
-        """Time service"""
-        return self._time_service
-
     @reify
     def cookies(self):
         """Return request cookies.

aiohttp/web_response.py

@@ -11,7 +11,8 @@
 from multidict import CIMultiDict, CIMultiDictProxy
 
 from . import hdrs, payload
-from .helpers import HeadersMixin, SimpleCookie, sentinel
+from .helpers import (HeadersMixin, SimpleCookie, rfc822_formatted_time,
+                      sentinel)
 from .http import RESPONSES, SERVER_SOFTWARE, HttpVersion10, HttpVersion11
 
 
@@ -375,7 +376,7 @@ def _start(self, request,
                     keep_alive = False
 
         headers.setdefault(CONTENT_TYPE, 'application/octet-stream')
-        headers.setdefault(DATE, request.time_service.strtime())
+        headers.setdefault(DATE, rfc822_formatted_time())
         headers.setdefault(SERVER, SERVER_SOFTWARE)
 
         # connection header

aiohttp/web_server.py

@@ -1,7 +1,6 @@
 """Low level HTTP server."""
 import asyncio
 
-from .helpers import TimeService
 from .web_protocol import RequestHandler
 from .web_request import BaseRequest
 
@@ -17,7 +16,6 @@ def __init__(self, handler, *, request_factory=None, loop=None, **kwargs):
         self._loop = loop
         self._connections = {}
         self._kwargs = kwargs
-        self.time_service = TimeService(self._loop)
         self.requests_count = 0
         self.request_handler = handler
         self.request_factory = request_factory or self._make_request
@@ -35,15 +33,13 @@ def connection_lost(self, handler, exc=None):
 
     def _make_request(self, message, payload, protocol, writer, task):
         return BaseRequest(
-            message, payload, protocol, writer,
-            protocol.time_service, task, self._loop)
+            message, payload, protocol, writer, task, self._loop)
 
     @asyncio.coroutine
     def shutdown(self, timeout=None):
         coros = [conn.shutdown(timeout) for conn in self._connections]
         yield from asyncio.gather(*coros, loop=self._loop)
         self._connections.clear()
-        self.time_service.close()
 
     def __call__(self):
         return RequestHandler(self, loop=self._loop, **self._kwargs)

changes/2176.removal

@@ -0,0 +1,2 @@
+Removed TimeService in favor of simple caching.
+TimeService also had a bug where it lost about 0.5 seconds per second.

tests/test_helpers.py

@@ -398,53 +398,6 @@ def test_is_ip_address_invalid_type():
         helpers.is_ip_address(object())
 
 
-# ----------------------------------- TimeService ----------------------
-
-
-@pytest.fixture
-def time_service(loop):
-    return helpers.TimeService(loop, interval=0.1)
-
-
-class TestTimeService:
-
-    def test_ctor(self, time_service):
-        assert time_service._cb is not None
-        assert time_service._time is not None
-        assert time_service._strtime is None
-
-    def test_stop(self, time_service):
-        time_service.close()
-        assert time_service._cb is None
-        assert time_service._loop is None
-
-    def test_double_stopping(self, time_service):
-        time_service.close()
-        time_service.close()
-        assert time_service._cb is None
-        assert time_service._loop is None
-
-    def test_time(self, time_service):
-        t = time_service._time
-        assert t == time_service.time()
-
-    def test_strtime(self, time_service):
-        time_service._time = 1477797232
-        assert time_service.strtime() == 'Sun, 30 Oct 2016 03:13:52 GMT'
-        # second call should use cached value
-        assert time_service.strtime() == 'Sun, 30 Oct 2016 03:13:52 GMT'
-
-    def test_recalc_time(self, time_service, mocker):
-        time = time_service._loop.time()
-        time_service._time = time
-        time_service._strtime = 'asd'
-        time_service._count = 1000000
-        time_service._on_cb()
-        assert time_service._strtime is None
-        assert time_service._time > time
-        assert time_service._count == 0
-
-
 # ----------------------------------- TimeoutHandle -------------------
 
 def test_timeout_handle(loop):

tests/test_streams.py

@@ -12,19 +12,13 @@ class TestStreamReader(unittest.TestCase):
     DATA = b'line1\nline2\nline3\n'
 
     def setUp(self):
-        self.time_service = None
         self.loop = asyncio.new_event_loop()
         asyncio.set_event_loop(None)
 
     def tearDown(self):
         self.loop.close()
 
     def _make_one(self, *args, **kwargs):
-        if 'timeout' in kwargs:
-            self.time_service = helpers.TimeService(self.loop, interval=0.01)
-            self.addCleanup(self.time_service.close)
-            kwargs['timer'] = self.time_service.timeout(kwargs.pop('timeout'))
-
         return streams.StreamReader(loop=self.loop, *args, **kwargs)
 
     def test_create_waiter(self):

tests/test_web_websocket_functional.py

@@ -737,8 +737,6 @@ def test_heartbeat_no_pong(loop, test_client, ceil):
     @asyncio.coroutine
     def handler(request):
         nonlocal cancelled
-        request._time_service._interval = 0.1
-        request._time_service._on_cb()
 
         ws = web.WebSocketResponse(heartbeat=0.05)
         yield from ws.prepare(request)