Showing content from http://mail.python.org/pipermail/python-dev/attachments/20160811/fc242a85/attachment.html below:
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 10 August 2016 at 04:43, Giampaolo Rodola' <span dir="ltr"><<a href="mailto:g.rodola@gmail.com" target="_blank">g.rodola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Tue, Aug 9, 2016 at 3:30 PM, Nick Coghlan <span dir="ltr"><<a href="mailto:ncoghlan@gmail.com" target="_blank">ncoghlan@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On 9 August 2016 at 23:26, Nick Coghlan <span dir="ltr"><<a href="mailto:ncoghlan@gmail.com" target="_blank">ncoghlan@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span>On 9 August 2016 at 06:18, Guido van Rossum <span dir="ltr"><<a href="mailto:guido@python.org" target="_blank">guido@python.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I think Nick would be interested in understanding why this is the case. What does the decorator do that could be so expensive?<br></div></blockquote><div><br></div></span><div>Reviewing <a href="https://hg.python.org/cpython/file/default/Lib/contextlib.py#l57" target="_blank">https://hg.python.org/cpython/<wbr>file/default/Lib/contextlib.py<wbr>#l57</a>, Chris's analysis seems plausible to me</div></div></div></div></blockquote><div><br></div></span><div>Sorry Wolfgang - I missed that Chris was expanding on a comparison you initially made!<br><br>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.__ini<wbr>t__ and into the contextmanager decorator itself.<br></div></div></div></div></blockquote><div><br></div></span><div>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).</div></div></div></div></blockquote><div><br></div><div>The draft changes aren't preserving the semantics (which may suggest we have some missing test cases), so we can't take timing numbers just yet. <br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Chris' <span style="font-size:12.8px">SimplerContextManager solution is faster because it avoids the factory function but that is necessary for supporting the decoration of methods. </span><span style="font-size:12.8px">Here's a PoC:</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px"><br></span></div><div><div><span style="font-size:12.8px">diff --git a/Lib/contextlib.py b/Lib/contextlib.py</span></div><div><span style="font-size:12.8px">index 7d94a57..45270dd 100644</span></div><div><span style="font-size:12.8px">--- a/Lib/contextlib.py</span></div><div><span style="font-size:12.8px">+++ b/Lib/contextlib.py</span></div><div><span style="font-size:12.8px">@@ -2,7 +2,7 @@</span></div><div><span style="font-size:12.8px"> import abc</span></div><div><span style="font-size:12.8px"> import sys</span></div><div><span style="font-size:12.8px"> from collections import deque</span></div><div><span style="font-size:12.8px">-from functools import wraps</span></div><div><span style="font-size:12.8px">+from functools import wraps, update_wrapper</span></div><div><span style="font-size:12.8px"> </span></div><div><span style="font-size:12.8px"> __all__ = ["contextmanager", "closing", "AbstractContextManager",</span></div><div><span style="font-size:12.8px">      "ContextDecorator", "ExitStack", "redirect_stdout",</span></div><div><span style="font-size:12.8px">@@ -57,25 +57,18 @@ class ContextDecorator(object):</span></div><div><span style="font-size:12.8px"> class _GeneratorContextManager(<wbr>ContextDecorator, AbstractContextManager):</span></div><div><span style="font-size:12.8px">   """Helper for @contextmanager decorator."""</span></div><div><span style="font-size:12.8px"> </span></div><div><span style="font-size:12.8px">+   __slots__ = ['gen', 'funcak']</span></div><div><span style="font-size:12.8px">+</span></div></div></div></div></div></blockquote><div><div><br></div><div>Note that this is is still keeping the
__dict__ allocation (you'd have to make ContexDecorator and
AbstractContextManager use __slots__ as well to eliminate it).<br><br></div><div>While there's likely some speedup just from the dict->descriptor shift, let's see how far we can improve *without* taking the __slots__ step, and then consider the question of adopting __slots__ (and the backwards compatibility implications if we take it as far as eliminating __dict__) separately.<br></div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><span style="font-size:12.8px">   def __init__(self, func, args, kwds):</span></div><div><span style="font-size:12.8px">     self.gen = func(*args, **kwds)</span></div><div><span style="font-size:12.8px">-     self.func, self.args, self.kwds = func, args, kwds</span></div><div><span style="font-size:12.8px">-     # Issue 19330: ensure context manager instances have good docstrings</span></div><div><span style="font-size:12.8px">-     doc = getattr(func, "__doc__", None)</span></div><div><span style="font-size:12.8px">-     if doc is None:</span></div><div><span style="font-size:12.8px">-       doc = type(self).__doc__</span></div><div><span style="font-size:12.8px">-     self.__doc__ = doc</span></div><div><span style="font-size:12.8px">-     # Unfortunately, this still doesn't provide good help output when</span></div><div><span style="font-size:12.8px">-     # inspecting the created context manager instances, since pydoc</span></div><div><span style="font-size:12.8px">-     # currently bypasses the instance docstring and shows the docstring</span></div><div><span style="font-size:12.8px">-     # for the class instead.</span></div><div><span style="font-size:12.8px">-     # See <a href="http://bugs.python.org/issue19404" target="_blank">http://bugs.python.org/<wbr>issue19404</a> for more details.</span></div><div><span style="font-size:12.8px">+     self.funcak = func, args, kwds</span></div></div></div></div></div></blockquote><div><br></div><div>This actually gives me an idea - we should compare the performance of the current code with that of using a functools.partial object defined in the decorator function, so only the partial object needs to be passed in to _GeneratorContextManager, and we can drop the args and kwds parameters entirely.<br><br></div><div>That should move some more of the work out of the per-use code in _GeneratorContextManager.__init__ and into the per-definition code in the contextmanager decorator, and "self.gen = func()" should then be faster than "self.gen = func(*args, **kwds)".<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div><span style="font-size:12.8px"></span></div><div><span style="font-size:12.8px">   def _recreate_cm(self):</span></div><div><span style="font-size:12.8px">     # _GCM instances are one-shot context managers, so the</span></div><div><span style="font-size:12.8px">     # CM must be recreated each time a decorated function is</span></div><div><span style="font-size:12.8px">     # called</span></div><div><span style="font-size:12.8px">-     return self.__class__(self.func, self.args, self.kwds)</span></div><div><span style="font-size:12.8px">+     func, args, kwds = self.funcak</span></div><div><span style="font-size:12.8px">+     return self.__class__(func, args, kwds)</span></div><div><span style="font-size:12.8px"> </span></div><div><span style="font-size:12.8px">   def __enter__(self):</span></div><div><span style="font-size:12.8px">     try:</span></div><div><span style="font-size:12.8px">@@ -157,6 +150,8 @@ def contextmanager(func):</span></div><div><span style="font-size:12.8px">   @wraps(func)</span></div><div><span style="font-size:12.8px">   def helper(*args, **kwds):</span></div><div><span style="font-size:12.8px">     return _GeneratorContextManager(func, args, kwds)</span></div><div><span style="font-size:12.8px">+</span></div><div><span style="font-size:12.8px">+   update_wrapper(helper, func)</span></div><div><span style="font-size:12.8px">   return helper</span></div></div></div></div></div></blockquote><div><br></div><div>As Chris noted, this probably isn't having the effect you want - to move the introspection code out to the decorator, you still need to pull __doc__ from the function (at decoration time) and set it on the _GeneratorContextManager instance (at instance creation time).<br><br></div><div>However, we're getting down to discussing proposed patch details now, so it's probably time to file an enhancement issue on the tracker and move further discussion there :)<br><br></div><div>Cheers,<br></div><div>Nick.<br><br></div><div>P.S. I head down to Melbourne for PyCon Australia tomorrow, so I probably won't be too responsive until the conference sprints start on Monday.<br clear="all"></div></div><br>-- <br><div class="gmail_signature" data-smartmail="gmail_signature">Nick Coghlan  |  <a href="mailto:ncoghlan@gmail.com" target="_blank">ncoghlan@gmail.com</a>  |  Brisbane, Australia</div>
</div></div>
RetroSearch is an open source project built by @garambo
| Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4