[Python-Dev] Review request: issue 27350, compact ordered dict

2016-08-09 Thread INADA Naoki 
Hi, devs.

I've implemented compact and ordered dictionary [1], which PyPy
implemented in 2015 [2].

Since it is my first large patch, I would like to have enough time for
review cycle by Python 3.6 beta1.

Could someone review it?

[1] http://bugs.python.org/issue27350
[2] 
https://morepypy.blogspot.jp/2015/01/faster-more-memory-efficient-and-more.html

-- 
INADA Naoki  
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-08-09 Thread Nick Coghlan 
On 9 August 2016 at 06:18, Guido van Rossum  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: most of the cost is going to be in
the fact that instead of a single function call for __exit__ and __enter__,
we have a function call *and* a generator frame resumption, and in the case
of __exit__, the expected flow includes catching StopIteration. The object
creation is also marginally more expensive, since it's indirect through a
factory function rather than calling the type directly.

There's also currently some introspection of "func" being done in __init__
as a result of issue #19330 that should probably be moved out to the
contextmanager decorator and passed in from the closure as a separate
argument rather than being figured out on each call to
_GeneratorContextManager.__init__.

So I don't see any obvious reason we shouldn't be able to get the standard
library version at least to a similar level of performance to Chris's
simpler alternatives. There are also some potential object allocation
efficiencies we could consider, like using __slots__ to eliminate the
__dict__ allocation (that does have backwards compatibility implications,
but may be worth it if it buys a non-trivial speed improvement).

Beyond that, while I think a C accelerator may be worth trying, I'm not
sure how much it will actually gain us, as it seems plausible that a
function call + generator frame resumption will inherently take twice as
long as just doing a function call.

Regards,
Nick.

-- 
Nick Coghlan   |   [email protected]   |   Brisbane, Australia
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-08-09 Thread Nick Coghlan 
On 9 August 2016 at 23:26, Nick Coghlan  wrote:

> On 9 August 2016 at 06:18, Guido van Rossum  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.

Cheers,
Nick.

-- 
Nick Coghlan   |   [email protected]   |   Brisbane, Australia
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

Re: [Python-Dev] Python parser performance optimizations

2016-08-09 Thread Artyom Skrobov 
Hello,

This is a monthly ping to get a review on http://bugs.python.org/issue26415 -- 
"Excessive peak memory consumption by the Python parser".

Following the comments from July, the patches now include updating Misc/NEWS 
and compiler.rst to describe the change.

The code change itself is still the same as a month ago.


From: Artyom Skrobov
Sent: 07 July 2016 15:44
To: [email protected]; [email protected]; [email protected]; 
[email protected]
Cc: nd
Subject: RE: Python parser performance optimizations

Hello,

This is a monthly ping to get a review on http://bugs.python.org/issue26415 -- 
"Excessive peak memory consumption by the Python parser".
The first patch of the series (an NFC refactoring) was successfully committed 
earlier in June, so the next step is to get the second patch, "the payload", 
reviewed and committed.

To address the concerns raised by the commenters back in May: the patch doesn't 
lead to negative memory consumption, of course. The base for calculating 
percentages is the smaller number of the two; this is the same style of 
reporting that perf.py uses. In other words, "200% less memory usage" is a 
threefold shrink.

The absolute values, and the way they were produced, are all reported under the 
ticket.


From: Artyom Skrobov
Sent: 26 May 2016 11:19
To: '[email protected]'
Subject: Python parser performance optimizations

Hello,

Back in March, I've posted a patch at http://bugs.python.org/issue26526 -- "In 
parsermodule.c, replace over 2KLOC of hand-crafted validation code, with a DFA".

The motivation for this patch was to enable a memory footprint optimization, 
discussed at http://bugs.python.org/issue26415
My proposed optimization reduces the memory footprint by up to 30% on the 
standard benchmarks, and by 200% on a degenerate case which sparked the 
discussion.
The run time stays unaffected by this optimization.

Python Developer's Guide says: "If you don't get a response within a few days 
after pinging the issue, then you can try emailing 
[email protected] asking for someone to 
review your patch."

So, here I am.
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-08-09 Thread Giampaolo Rodola' 
On Mon, Aug 8, 2016 at 11:59 PM, Chris Angelico  wrote:

> On Tue, Aug 9, 2016 at 7:14 AM, Wolfgang Maier
>  wrote:
> > Right, I think a fairer comparison would be to:
> >
> > class ctx2:
> > def __enter__(self):
> > self.it = iter(self)
> > return next(self.it)
> >
> > def __exit__(self, *args):
> > try:
> > next(self.it)
> > except StopIteration:
> > pass
> >
> > def __iter__(self):
> > yield
> >
> > With this change alone the slowdown diminishes to ~ 1.7x for me. The
> rest is
> > probably the extra overhead for being able to pass exceptions raised
> inside
> > the with block back into the generator and such.
>
> I played around with a few other variants to see where the slowdown
> is. They all work out pretty much the same as the above; my two
> examples are both used the same way as contextlib.contextmanager is,
> but are restrictive on what you can do.
>
> import timeit
> import contextlib
> import functools
>
> class ctx1:
> def __enter__(self):
> pass
> def __exit__(self, *args):
> pass
>
> @contextlib.contextmanager
> def ctx2():
> yield
>
> class SimplerContextManager:
> """Like contextlib._GeneratorContextManager but way simpler.
>
> * Doesn't reinstantiate itself - just reinvokes the generator
> * Doesn't allow yielded objects (returns self)
> * Lacks a lot of error checking. USE ONLY AS DIRECTED.
> """
> def __init__(self, func):
> self.func = func
> functools.update_wrapper(self, func)
> def __call__(self, *a, **kw):
> self.gen = self.func(*a, **kw)
> return self
> def __enter__(self):
> next(self.gen)
> return self
> def __exit__(self, type, value, traceback):
> if type is None:
> try: next(self.gen)
> except StopIteration: return
> else: raise RuntimeError("generator didn't stop")
> try: self.gen.throw(type, value, traceback)
> except StopIteration: return True
> # Assume any instance of the same exception type is a proper
> reraise
> # This is way simpler than contextmanager normally does, and costs
> us
> # the ability to detect exception handlers that coincidentally
> raise
> # the same type of error (eg "except ZeroDivisionError:
> print(1/0)").
> except type: return False
>
> # Check that it actually behaves correctly
> @SimplerContextManager
> def ctxdemo():
> print("Before yield")
> try:
> yield 123
> except ZeroDivisionError:
> print("Absorbing 1/0")
> return
> finally:
> print("Finalizing")
> print("After yield (no exception)")
>
> with ctxdemo() as val:
> print("1/0 =", 1/0)
> with ctxdemo() as val:
> print("1/1 =", 1/1)
> #with ctxdemo() as val:
> #print("1/q =", 1/q)
>
> @SimplerContextManager
> def ctx3():
> yield
>
> class TooSimpleContextManager:
> """Now this time you've gone too far."""
> def __init__(self, func):
> self.func = func
> def __call__(self):
> self.gen = self.func()
> return self
> def __enter__(self):
> next(self.gen)
> def __exit__(self, type, value, traceback):
> try: next(self.gen)
> except StopIteration: pass
>
> @TooSimpleContextManager
> def ctx4():
> yield
>
> class ctx5:
> def __enter__(self):
> self.it = iter(self)
> return next(self.it)
>
> def __exit__(self, *args):
> try:
> next(self.it)
> except StopIteration:
> pass
>
> def __iter__(self):
> yield
>
> t1 = timeit.timeit("with ctx1(): pass", setup="from __main__ import ctx1")
> print("%.3f secs" % t1)
>
> for i in range(2, 6):
> t2 = timeit.timeit("with ctx2(): pass", setup="from __main__
> import ctx%d as ctx2"%i)
> print("%.3f secs" % t2)
> print("slowdown: -%.2fx" % (t2 / t1))
>
>
> My numbers are:
>
> 0.320 secs
> 1.354 secs
> slowdown: -4.23x
> 0.899 secs
> slowdown: -2.81x
> 0.831 secs
> slowdown: -2.60x
> 0.868 secs
> slowdown: -2.71x
>
> So compared to the tight custom-written context manager class, all the
> "pass it a generator function" varieties look pretty much the same.
> The existing contextmanager factory has several levels of indirection,
> and that's probably where the rest of the performance difference comes
> from, but there is some cost to the simplicity of the gen-func
> approach.
>
> My guess is that a C-implemented version could replace piles of
> error-handling code with simple pointer comparisons (hence my
> elimination of it), and may or may not be able to remove some of the
> indirection. I'd say it'd land about in the same range as the other
> examples here. Is that worth it?
>
> ChrisA
> ___
> Python-Dev mailing list
> [email protected]
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe: https://ma

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

2016-08-09 Thread Giampaolo Rodola' 
On Tue, Aug 9, 2016 at 3:30 PM, Nick Coghlan  wrote:

> On 9 August 2016 at 23:26, Nick Coghlan  wrote:
>
>> On 9 August 2016 at 06:18, Guido van Rossum  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
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-08-09 Thread Chris Angelico 
On Wed, Aug 10, 2016 at 4:43 AM, Giampaolo Rodola'  wrote:
> -return self.__class__(self.func, self.args, self.kwds)
> +func, args, kwds = self.funcak
> +return self.__class__(func, args, kwds)

return self.__class__(*self.funcak)

>  @wraps(func)
>  def helper(*args, **kwds):
>  return _GeneratorContextManager(func, args, kwds)
> +
> +update_wrapper(helper, func)

Isn't update_wrapper exactly what @wraps is doing for you?

ChrisA
___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com

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

2016-08-09 Thread Serhiy Storchaka 

On 09.08.16 00:59, Chris Angelico wrote:

class TooSimpleContextManager:
"""Now this time you've gone too far."""
def __init__(self, func):
self.func = func
def __call__(self):
self.gen = self.func()
return self
def __enter__(self):
next(self.gen)
def __exit__(self, type, value, traceback):
try: next(self.gen)
except StopIteration: pass

[...]
> My numbers are:
>
> 0.320 secs
> 1.354 secs
> slowdown: -4.23x
> 0.899 secs
> slowdown: -2.81x
> 0.831 secs
> slowdown: -2.60x
> 0.868 secs
> slowdown: -2.71x

I have got a slowdown 1.74x with TooSimpleContextManager with following 
implementation of __enter__ and __exit__:


def __enter__(self):
for res in self.gen:
return res
raise RuntimeError("generator didn't yield")
def __exit__(self, type, value, traceback):
for _ in self.gen:
raise RuntimeError("generator didn't stop")


___
Python-Dev mailing list
[email protected]
https://mail.python.org/mailman/listinfo/python-dev
Unsubscribe: 
https://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com