Merge pull request #2587 from bdarnell/opt

ioloop: Micro-optimize IOLoop.add_future

Author: bdarnell

tornado/ioloop.py

@@ -33,6 +33,7 @@
 import asyncio
 import concurrent.futures
 import datetime
+import functools
 import logging
 import numbers
 import os
@@ -676,10 +677,25 @@ def add_future(
         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,

tornado/test/iostream_test.py

@@ -946,29 +946,29 @@ def test_start_tls_smtp(self):
     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):
         # Test that server_hostname parameter to start_tls is being used.
         # 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):