From: Claudio Freire <klaussfreire@gmail.com>
Date: Wed, 2 Sep 2015 17:32:46 +0000 (-0300)
Subject: Remove synchronization for add_callback in the ioloop thread
X-Git-Tag: v4.3.0b1~35^2~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=89f59cac08892fd266789cbcdfa60f452d4e54a8;p=thirdparty%2Ftornado.git

Remove synchronization for add_callback in the ioloop thread

It is not necessary, since it will never need to invoke
wake, and as noted in add_callback_from_signal it would pose
no race risk, and it is a considerable performance penalty.
---

diff --git a/tornado/ioloop.py b/tornado/ioloop.py
index 87d4168e4..388d91b12 100644
--- a/tornado/ioloop.py
+++ b/tornado/ioloop.py
@@ -909,20 +909,32 @@ class PollIOLoop(IOLoop):
         self._cancellations += 1
 
     def add_callback(self, callback, *args, **kwargs):
-        with self._callback_lock:
-            if self._closing:
-                raise RuntimeError("IOLoop is closing")
-            list_empty = not self._callbacks
+        if thread.get_ident() != self._thread_ident:
+            with self._callback_lock:
+                if self._closing:
+                    raise RuntimeError("IOLoop is closing")
+                list_empty = not self._callbacks
+                self._callbacks.append(functools.partial(
+                    stack_context.wrap(callback), *args, **kwargs))
+                if list_empty:
+                    # If we're in the IOLoop's thread, we know it's not currently
+                    # polling.  If we're not, and we added the first callback to an
+                    # empty list, we may need to wake it up (it may wake up on its
+                    # own, but an occasional extra wake is harmless).  Waking
+                    # up a polling IOLoop is relatively expensive, so we try to
+                    # avoid it when we can.
+                    self._waker.wake()
+        else:
+            # If we're on the IOLoop's thread, we don't need the lock,
+            # since we don't need to wake anyone, just add the callback.
+            # Blindly insert into self._callbacks.
+            # This is safe because the GIL makes list.append atomic.
+            # One subtlety is that if the thread is interrupting another
+            # thread holding the _callback_lock block in IOLoop.start, 
+            # we may modify either the old or new version of self._callbacks,
+            # but either way will work.
             self._callbacks.append(functools.partial(
                 stack_context.wrap(callback), *args, **kwargs))
-            if list_empty and thread.get_ident() != self._thread_ident:
-                # If we're in the IOLoop's thread, we know it's not currently
-                # polling.  If we're not, and we added the first callback to an
-                # empty list, we may need to wake it up (it may wake up on its
-                # own, but an occasional extra wake is harmless).  Waking
-                # up a polling IOLoop is relatively expensive, so we try to
-                # avoid it when we can.
-                self._waker.wake()
 
     def add_callback_from_signal(self, callback, *args, **kwargs):
         with stack_context.NullContext():