From: Ben Darnell <ben@bendarnell.com>
Date: Sun, 3 Feb 2019 23:01:53 +0000 (-0500)
Subject: ioloop: Micro-optimize IOLoop.add_future
X-Git-Tag: v6.0.0b1~1^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F2587%2Fhead;p=thirdparty%2Ftornado.git

ioloop: Micro-optimize IOLoop.add_future

Asyncio Futures always schedule their callbacks on a future iteration
of the IOLoop, so routing the callbacks through IOLoop.add_callback
again was redundant and causing unnecessary delays in callback
execution.
---

diff --git a/tornado/ioloop.py b/tornado/ioloop.py
index 14f73c83e..8e2bb8af1 100644
--- a/tornado/ioloop.py
+++ b/tornado/ioloop.py
@@ -33,6 +33,7 @@ per `unittest` case.
 import asyncio
 import concurrent.futures
 import datetime
+import functools
 import logging
 import numbers
 import os
@@ -676,10 +677,25 @@ class IOLoop(Configurable):
         awaitables (unlike most of Tornado where the two are
         interchangeable).
         """
-        assert is_future(future)
-        future_add_done_callback(
-            future, lambda future: self.add_callback(callback, future)
-        )
+        if isinstance(future, Future):
+            # Note that we specifically do not want the inline behavior of
+            # tornado.concurrent.future_add_done_callback. We always want
+            # this callback scheduled on the next IOLoop iteration (which
+            # asyncio.Future always does).
+            #
+            # Wrap the callback in self._run_callback so we control
+            # the error logging (i.e. it goes to tornado.log.app_log
+            # instead of asyncio's log).
+            future.add_done_callback(
+                lambda f: self._run_callback(functools.partial(callback, future))
+            )
+        else:
+            assert is_future(future)
+            # For concurrent futures, we use self.add_callback, so
+            # it's fine if future_add_done_callback inlines that call.
+            future_add_done_callback(
+                future, lambda f: self.add_callback(callback, future)
+            )
 
     def run_in_executor(
         self,
diff --git a/tornado/test/iostream_test.py b/tornado/test/iostream_test.py
index 413141a02..295cdd58d 100644
--- a/tornado/test/iostream_test.py
+++ b/tornado/test/iostream_test.py
@@ -946,12 +946,12 @@ class TestIOStreamStartTLS(AsyncTestCase):
     def test_handshake_fail(self):
         server_future = self.server_start_tls(_server_ssl_options())
         # Certificates are verified with the default configuration.
-        client_future = self.client_start_tls(server_hostname="localhost")
         with ExpectLog(gen_log, "SSL Error"):
+            client_future = self.client_start_tls(server_hostname="localhost")
             with self.assertRaises(ssl.SSLError):
                 yield client_future
-        with self.assertRaises((ssl.SSLError, socket.error)):
-            yield server_future
+            with self.assertRaises((ssl.SSLError, socket.error)):
+                yield server_future
 
     @gen_test
     def test_check_hostname(self):
@@ -959,16 +959,16 @@ class TestIOStreamStartTLS(AsyncTestCase):
         # The check_hostname functionality is only available in python 2.7 and
         # up and in python 3.4 and up.
         server_future = self.server_start_tls(_server_ssl_options())
-        client_future = self.client_start_tls(
-            ssl.create_default_context(), server_hostname="127.0.0.1"
-        )
         with ExpectLog(gen_log, "SSL Error"):
+            client_future = self.client_start_tls(
+                ssl.create_default_context(), server_hostname="127.0.0.1"
+            )
             with self.assertRaises(ssl.SSLError):
                 # The client fails to connect with an SSL error.
                 yield client_future
-        with self.assertRaises(Exception):
-            # The server fails to connect, but the exact error is unspecified.
-            yield server_future
+            with self.assertRaises(Exception):
+                # The server fails to connect, but the exact error is unspecified.
+                yield server_future
 
 
 class WaitForHandshakeTest(AsyncTestCase):