[Python-Dev] Rewrite @contextlib.contextmanager in C

[Giampaolo Rodola']

On Tue, Aug 9, 2016 at 3:30 PM, Nick Coghlan <ncoghlan at gmail.com> wrote:

> On 9 August 2016 at 23:26, Nick Coghlan <ncoghlan at gmail.com> wrote:
>
>> On 9 August 2016 at 06:18, Guido van Rossum <guido at python.org> wrote:
>>
>>> I think Nick would be interested in understanding why this is the case.
>>> What does the decorator do that could be so expensive?
>>>
>>
>> Reviewing https://hg.python.org/cpython/file/default/Lib/contextlib.py
>> #l57, Chris's analysis seems plausible to me
>>
>
> Sorry Wolfgang - I missed that Chris was expanding on a comparison you
> initially made!
>
> Either way, I agree that aspect does make up the bulk of the difference in
> speed, so moving to C likely wouldn't help much. However, the speed
> difference relative to the simpler warppers is far less defensible - I
> think there are some opportunities for improvement there, especially around
> moving introspection work out of _GeneratorContextManager.__init__ and
> into the contextmanager decorator itself.
>

By moving the __doc__ introspection out of __init__ and by introducing
__slots__ I got from -4.37x to -3.16x (__slot__ speedup was about +0.3x).
Chris' SimplerContextManager solution is faster because it avoids the
factory function but that is necessary for supporting the decoration of
methods. Here's a PoC:


diff --git a/Lib/contextlib.py b/Lib/contextlib.py
index 7d94a57..45270dd 100644
--- a/Lib/contextlib.py
+++ b/Lib/contextlib.py
@@ -2,7 +2,7 @@
 import abc
 import sys
 from collections import deque
-from functools import wraps
+from functools import wraps, update_wrapper

 __all__ = ["contextmanager", "closing", "AbstractContextManager",
            "ContextDecorator", "ExitStack", "redirect_stdout",
@@ -57,25 +57,18 @@ class ContextDecorator(object):
 class _GeneratorContextManager(ContextDecorator, AbstractContextManager):
     """Helper for @contextmanager decorator."""

+    __slots__ = ['gen', 'funcak']
+
     def __init__(self, func, args, kwds):
         self.gen = func(*args, **kwds)
-        self.func, self.args, self.kwds = func, args, kwds
-        # Issue 19330: ensure context manager instances have good
docstrings
-        doc = getattr(func, "__doc__", None)
-        if doc is None:
-            doc = type(self).__doc__
-        self.__doc__ = doc
-        # Unfortunately, this still doesn't provide good help output when
-        # inspecting the created context manager instances, since pydoc
-        # currently bypasses the instance docstring and shows the docstring
-        # for the class instead.
-        # See http://bugs.python.org/issue19404 for more details.
+        self.funcak = func, args, kwds

     def _recreate_cm(self):
         # _GCM instances are one-shot context managers, so the
         # CM must be recreated each time a decorated function is
         # called
-        return self.__class__(self.func, self.args, self.kwds)
+        func, args, kwds = self.funcak
+        return self.__class__(func, args, kwds)

     def __enter__(self):
         try:
@@ -157,6 +150,8 @@ def contextmanager(func):
     @wraps(func)
     def helper(*args, **kwds):
         return _GeneratorContextManager(func, args, kwds)
+
+    update_wrapper(helper, func)
     return helper


-- 
Giampaolo - http://grodola.blogspot.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20160809/02114056/attachment.html>