Skip to content

Commit

Permalink
ioloop: Micro-optimize IOLoop.add_future
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bdarnell committed Feb 3, 2019
1 parent 975e916 commit 64de184
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 13 deletions.
24 changes: 20 additions & 4 deletions tornado/ioloop.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import asyncio
import concurrent.futures
import datetime
import functools
import logging
import numbers
import os
Expand Down Expand Up @@ -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,
Expand Down
18 changes: 9 additions & 9 deletions tornado/test/iostream_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down

0 comments on commit 64de184

Please sign in to comment.