Stefan Behnel added the comment:
As mentioned, the fannkuch benchmark benefits quite a bit from the fast path,
by 8%. It was a while ago when I tested, though, so I don't have exact numbers
ATM.
--
___
Python tracker
<http://bugs.py
Stefan Behnel added the comment:
Here's another idea. There could be two implementations of "slice", one that
uses Python object indices (as currently) and one that has Py_ssize_t indices
(and properties for the start/stop/step attributes). Similar to what Unicode
strings do,
Stefan Behnel added the comment:
Thanks for the review, Serhiy. There isn't really a reason not to 'normalise'
the Python step object to None when 1 is passed from C code, so I updated the
patch to do that.
Regarding on-demand creation of the Python values, the problem is th
New submission from Stefan Behnel:
The median tends to give a better idea about benchmark results than an average
as it inherently ignores outliers.
--
components: Benchmarks
files: show_median.patch
keywords: patch
messages: 231239
nosy: pitrou, scoder
priority: normal
severity
Stefan Behnel added the comment:
Fair enough, patch updated.
--
Added file: http://bugs.python.org/file37207/show_median.patch
___
Python tracker
<http://bugs.python.org/issue22
Stefan Behnel added the comment:
I reran the benchmarks with the fast path in _PyEval_SliceIndex() and the
results are not conclusive. There is no clear indication that it improves the
performance.
The problem is that most slices have no step, many slices are open ended on at
least one side
Stefan Behnel added the comment:
> why you pass through fast path only if -1<= Py_SIZE(v) <=1?
It's usually enough, it's the fastest fast path, and it doesn't even need
error handling.
Any slicing where the slice calculation time matters is going to be of
fairly limit
Stefan Behnel added the comment:
Regarding the patch below, isn't most of this redundant? ISTM that simply
calling PyErr_SetString(...) should do all of this, including the exception
chaining.
diff -r 23ab1197df0b Objects/genobject.c
--- a/Objects/genobject.c Wed Nov 19 13:21:40
Stefan Behnel added the comment:
Ah, right - chaining only happens automatically when the exception has already
been caught and moved to sys.exc_info.
There's a _PyErr_ChainExceptions(), though, which does it for you. You should
be able to say
PyErr_Fetch(&x,&y,&z)
Stefan Behnel added the comment:
Public underscore C functions are there for exactly the purpose of not
duplicating functionality across *internal* core runtime code, without making
them an official part of the C-API for *external* code. (I understand that it's
a POC patch, though, so wh
Stefan Behnel added the comment:
FYI, here's the set of tests that I've written for my implementation in Cython:
https://github.com/cython/cython/blob/b4383a540a790a5553f19438b3fc22b11858d665/tests/run/generators_pep479.pyx
--
___
Pyth
New submission from Stefan Behnel:
distutils uses this code to validate the input in "Command.__init__()":
# late import because of mutual dependence between these classes
from distutils.dist import Distribution
if not isinstance(dist, Distribution):
Stefan Behnel added the comment:
Yes, pretty much the same problem.
--
___
Python tracker
<http://bugs.python.org/issue23114>
___
___
Python-bugs-list mailin
Stefan Behnel added the comment:
The same applies to the error "dist must be a Distribution instance" in cmd.py.
See issue 23114.
--
nosy: +scoder
___
Python tracker
<http://bugs.python.o
Stefan Behnel added the comment:
Closing as not worth doing.
--
resolution: -> rejected
status: open -> closed
___
Python tracker
<http://bugs.python.org/i
Stefan Behnel added the comment:
Serhiy, I suggest you look at the code that Cython generates for its functions.
It has been extensively profiled and optimised (years ago), so generating the
same code for the argument clinic should yield the same performance.
And while I don't have
Stefan Behnel added the comment:
FTR, lxml's Element class has addnext() and addprevious() methods which are
commonly used for this purpose. But ET can't adopt those due to its different
tree model.
I second Martin's comment that ET.append() is a misleading name. It suggests
Stefan Behnel added the comment:
I added a couple of review comments to patch 6, but since no-one has responded
so far, I guess they simply haven't been noticed. So I'll just repeat them here.
1)
getawaitablefunc / aiternextfunc / getaiterfunc
Is there a reason why these need to
Changes by Stefan Behnel :
--
nosy: +scoder
___
Python tracker
<http://bugs.python.org/issue23275>
___
___
Python-bugs-list mailing list
Unsubscribe:
Stefan Behnel added the comment:
Another question: is it ok if Cython implements and uses the "tp_as_async" slot
in all Py3.x versions (3.2+)? It shouldn't hurt, but it would speed up
async/await in Cython at least under Py3.x. Only Py2.6/7 would then have to
resort to cal
Stefan Behnel added the comment:
> It *is* correct, see PEP 492. Awaitable is either a coroutine *or* an object
> with an __await__ method.
"coroutine", yes. But "Coroutine"? Shouldn't the Coroutine ABC then require
"__await__" to be implemented
Stefan Behnel added the comment:
> Can't your Coroutine object return itself from its __await__, and implement
> __next__? Like genobject in CPython simply returns self from its __iter__.
That was my first try, sure, and it mostly worked. It has a drawback,
though: it'
Stefan Behnel added the comment:
BTW, given that "iter(iterator)" works and returns the iterator, should we also
allow "await x.__await__()" to work? I guess that would be tricky to achieve
given that __await__() is only required to return any kind of arbitrary
Iterator,
Stefan Behnel added the comment:
>> Yield-from iterates, and a coroutine is not supposed to be iterable, only
>> awaitable (at least, that's what all error messages tell me when I try it).
>> So why should "yield from" work on them? What if foo() was not an Ite
Stefan Behnel added the comment:
> Properly handling other types which are convertible to floats, such as
> Decimal and Fraction, is outside the scope of this issue.
... and outside of the scope of the math module in general. It's inherently
floating
Stefan Behnel added the comment:
Thanks Yury, I'll give it a try ASAP.
--
___
Python tracker
<http://bugs.python.org/issue24017>
___
___
Python-bugs-list m
Stefan Behnel added the comment:
I'm seeing crashes with this assertion in Cython test modules:
python: ./Python/importdl.c:75: get_encoded_name: Assertion
`(((PyObject*)(encoded))->ob_refcnt) == 1' failed.
The problem seems to be that the module name is only one character lon
Stefan Behnel added the comment:
Patch LGTM and it fixes the problem (tried it on my side). Please make sure it
gets applied in time for beta 2.
--
___
Python tracker
<http://bugs.python.org/issue24
Stefan Behnel added the comment:
Tried it, works for me. Thanks!
--
___
Python tracker
<http://bugs.python.org/issue24017>
___
___
Python-bugs-list mailin
Stefan Behnel added the comment:
I just noticed that I hadn't used the real "types.coroutine" in my Py3.5 tests
when reporting back in issue 24017. When I pass a Cython generator through it,
I get
"""
Traceback (most recent call last):
File "tests/run/te
Stefan Behnel added the comment:
One failing test in "test_coroutines": test_func_5. The reason is that the
GeneratorWrapper is not iterable (and there is no reason it shouldn't be, given
that it wraps a Generator). That was my fault, I had already added an __iter__
method but
Stefan Behnel added the comment:
BTW, it's not only for compiled generators but also for normal Python functions
that construct Python generators internally and return them, or that delegate
the generator creation in some way. With this change, it's enough to decorate
the constructo
Stefan Behnel added the comment:
Ok, now the problem with *this* patch is that __iter__ and __await__ are
special methods that are being looked up on the type, not the instance.
Similarly __next__, I think, as it also has its own (type) slot. But I think
you're right that __next__ is
Stefan Behnel added the comment:
Your latest patch works for me.
--
___
Python tracker
<http://bugs.python.org/issue24316>
___
___
Python-bugs-list mailin
Stefan Behnel added the comment:
IMHO less clear and less correct than the previous suggestions.
--
___
Python tracker
<http://bugs.python.org/issue24079>
___
___
Stefan Behnel added the comment:
Seems like a good idea to explain "text" and "tail" in one section, though.
That makes "tail" easier to find for those who are not used to this kind of
split (and that's basically everyone who needs t
Stefan Behnel added the comment:
I would still like to see a patch in Py3.5 that only flips the bit in C when
"_types" is available and otherwise falls back to the existing Python code that
creates a new code object. This part is much cleaner and faster to do in C than
in the curr
Stefan Behnel added the comment:
I found one more place in asyncio.coroutines, around line 190 in the
coroutine() decorator:
if inspect.isgeneratorfunction(func):
coro = func
else:
@functools.wraps(func)
def coro(*args, **kw):
res = func(*args, **kw
Stefan Behnel added the comment:
Cython functions that return a generator aren't special in any way, so it's
actually correct that isgeneratorfunction() always returns False. I mean, I
could set the CO_COROUTINE flag on the code object that we fake, but that
wouldn't help m
Stefan Behnel added the comment:
Hmm, I just noticed that this only started failing when I disabled the asyncio
module patching for 3.5beta1+, assuming that it now all works. A side effect of
that was that also the inspect module is no longer patched into returning True
from isgenerator() for
Stefan Behnel added the comment:
... except that the returned object may not actually be a Cython generator but
a GeneratorWrapper, now that the patch from issue 24316 is in place. :)
Then I guess we're back to the point where duck-typing and calling __await__()
is a good
Stefan Behnel added the comment:
Works for me.
Why is the AwaitableABC type check needed, in addition to looking up the
relevant method? IIUC, the type check will just do a lookup of the same method
if the type hasn't been registered as Awaitable explicitly.
--
status: pe
Stefan Behnel added the comment:
Hmm, but IMHO a) the new syntax isn't just for asyncio and b) awaiting a Future
seems like a *very* reasonable thing to do. I think opening a new ticket for
this is a good idea.
--
___
Python tracker
Stefan Behnel added the comment:
Looks good to me.
--
___
Python tracker
<http://bugs.python.org/issue24079>
___
___
Python-bugs-list mailing list
Unsubscribe:
Stefan Behnel added the comment:
(Copying my review comment here so that it doesn't get lost as it's more of a
general design thing than a specific review comment.)
Having Future know and deal with asyncio seems wrong to me. asyncio should be
able to deal *somehow* with a Future tha
Stefan Behnel added the comment:
I agree that the "extra bit" is a quirk. Ideally, Coroutine and Generator would
share a common base class that has "send", "throw" and "close". Additionally, a
Coroutine would be an Awaitable whereas a Generator is an I
Stefan Behnel added the comment:
Looks good, simplifies the code visibly and makes the implementation quite
similar to the one I wrote for Cython.
I like the fact that it separates generators from coroutines at an isinstance()
level. The fact that "async def" functions currently hav
Stefan Behnel added the comment:
See issue 24400 regarding a split of yield-based generators and async-def
coroutines at a type level.
--
___
Python tracker
<http://bugs.python.org/issue24
Stefan Behnel added the comment:
I added some review comments. The main thing is that the coroutine type should
be awaitable.
For reference, here's my current implementation for Cython. It's a bit more
involved as it needs to support Python 2.6+. I may also add some special c
Stefan Behnel added the comment:
> I currently do isinstance(x, types.GeneratorType), which will fail if x
> is actually a GeneratorWrapper. I can change this to use
> collections.abc.Generator when it exists, but then I'd have to have some
> conditional logic to switch betwee
Stefan Behnel added the comment:
Here are two patches that fix this case, one with special casing, one without.
Please choose and apply one.
--
Added file: http://bugs.python.org/file39691/fix_stopiteration_value_slow.patch
___
Python tracker
<h
Changes by Stefan Behnel :
Added file: http://bugs.python.org/file39692/fix_stopiteration_value.patch
___
Python tracker
<http://bugs.python.org/issue23996>
___
___
Pytho
Stefan Behnel added the comment:
I agree that a typedef is a good idea. It doesn't cost anything but gives
us more freedom for future changes.
--
___
Python tracker
<http://bugs.python.org/is
Stefan Behnel added the comment:
No. It's more that it feels wrong to spend actual time on the second most
common case that can occur instead of just handling it in no time at all. The
third case that it's really required to instantiate the StopIteration exception
(if user code di
New submission from Stefan Behnel:
A Cython user noticed a memory leak when C-inheriting from "int".
http://thread.gmane.org/gmane.comp.python.cython.devel/15689
The Cython code to reproduce this is simply this:
cdef class ExtendedInt(int): pass
for j in xrang
Changes by Stefan Behnel :
--
nosy: +scoder
___
Python tracker
<http://bugs.python.org/issue24450>
___
___
Python-bugs-list mailing list
Unsubscribe:
Stefan Behnel added the comment:
No problem for Cython either.
The change in issue 24400 that makes coroutines real Awaitables also removes
surprises for a "cr_await" return value being a coroutine and previously *not*
an Awaitable.
The contract for "gi_yieldfrom" is on
Changes by Stefan Behnel :
--
nosy: +pitrou, serhiy.storchaka
___
Python tracker
<http://bugs.python.org/issue24469>
___
___
Python-bugs-list mailing list
Unsub
Stefan Behnel added the comment:
> 1) The intended solution is to require that int subclasses override tp_free.
In the way I showed? By calling either PyObject_GC_Del() or PyObject_Del()
directly? I'll happily do that (Py2.7 and Py2 "int" being dead ends makes
that pretty
Stefan Behnel added the comment:
I added three more comments to the review. The rest looks good to me, too.
--
___
Python tracker
<http://bugs.python.org/issue24
Stefan Behnel added the comment:
> I originally planned to make the next Cython release patch the Generator
> and Coroutine ABCs into collections.abc, but I now think it would be worth
> uploading an "abc_backports" package to PyPI instead that does that and on
> which asyn
Stefan Behnel added the comment:
> isawaitable(), however, should continue using abc.Awaitable, since it only
> checks for __await__ presence on the type (or should we just drop it?)
I'd really remove it. It's not referring to an actual type, so it doesn't fit
the p
Stefan Behnel added the comment:
This is not purely about speeding up the code. It's also about avoiding to
replace the code object of a function, which is essentially a big and clumsy
hack only to achieve setting a flag. Some tools, namely line_profiler, use the
current code object as a
Stefan Behnel added the comment:
> 1. abc.Coroutine and abc.Awaitable will guarantee that objects that implement
> them have '__await__' and it's safe to access it (that means that they will
> fail for generator-based coroutines).
Absolutely. That was the main the
Stefan Behnel added the comment:
> the undecorated form will never be visible to any code other than the
> decorator
Assuming that 1) it's the first and/or only decorator, 2) it's used to
decorate a generator function that returns its own generator and 3) it's
really us
Stefan Behnel added the comment:
It's generally worth running the benchmark suite for this kind of optimisation.
Being mostly Python code, it should benefit quite clearly from dictionary
improvements, but it should also give an idea of how much of an improvement
actual Python code (an
Stefan Behnel added the comment:
> Your benchmarks are not affected by this change see the other issue.
Then you ran them, I assume? Could you show the output here that you got?
> They are also not representative of every workload out there.
Certainly not, but they do provide a c
Stefan Behnel added the comment:
I'm witnessing a crash in the C implementation during garbage collection.
Interestingly, it only shows in the Py3.6 branch, not in Py3.5 (both latest).
Here's the gdb session:
Program received signal SIGSEGV, Segmentation fault.
lru_cache_tp_trav
Stefan Behnel added the comment:
It's not actually my own code using the lru_cache here. From a quick grep
over the code tree, the only potentially related usage I found was in
Python's fnmatch module, on the "_compile_pattern()" function. Commenting
that out then made the cr
Stefan Behnel added the comment:
test_fnmatch.py also passes, BTW.
--
___
Python tracker
<http://bugs.python.org/issue14373>
___
___
Python-bugs-list mailin
Stefan Behnel added the comment:
It looks like perfectly valid syntax to me and I cannot see why it should be
semantically rejected. I disagree that it shouldn't be changed. I think it's a
(minor) bug that should eventually get fixed.
--
nos
Stefan Behnel added the comment:
Hmm, right. I see your point and also the analogy with "yield". I could live
with (maybe) giving it a better error message suggesting to use parentheses for
clarity and otherwise keeping it a S
Stefan Behnel added the comment:
Benchmark results look good to me (although a header line is missing) and seem
to match my expectations. Sounds like we should allow this change.
--
___
Python tracker
<http://bugs.python.org/issue23
Stefan Behnel added the comment:
Thanks for updating the micro-benchmark. Just FYI (and sorry for hijacking this
ticket), I ran it through Cython. Here are the numbers:
Cython 0.23 (latest master)
binary(21) * 3: total 1.609s
abinary(21) * 3: total 1.514s
CPython 3.5 (latest branch)
binary(21
Stefan Behnel added the comment:
> Hm, I think there's little need for new exceptions...
While I agree with Yuri that the names are a bit awkward, I actually second
this. The StopIteration is almost an implementation detail of how the return
value is passed on to become the (Future) r
Stefan Behnel added the comment:
Agreed that it's misleading. Note that the section refers to the *following*
example, not the one that readers just went through.
I would actually start that paragraph with the reference to XPath (because
that's the difference to el.iter()), and
Stefan Behnel added the comment:
Note that the expected usage is not as a function but as a decorator. That
should be stated in the docs as well. IMHO, users should only do two things
with whatever the result is: either use it as a Generator (as before), or pass
it as an argument to "
Stefan Behnel added the comment:
could we apply this patch, please?
--
___
Python tracker
<http://bugs.python.org/issue24079>
___
___
Python-bugs-list mailin
Stefan Behnel added the comment:
Here's a complete drop-in replacement.
"""
More specific constraints on which elements to look for and where to look for
them in the tree can be expressed in :ref:`XPath `.
:meth:`Element.iterfind` iterates over all elements matching suc
Stefan Behnel added the comment:
> The proposed patch downplays that generality.
That is completely intentional. Almost all readers of the documentation will
first need to understand the difference between text and tail before they can
go and think about any more advanced use cases that w
Stefan Behnel added the comment:
Personally, I would prefer getting the improved version applied over
bikeshedding for another couple of months. But maybe that's just me.
--
___
Python tracker
<http://bugs.python.org/is
Stefan Behnel added the comment:
Whenever you feel a need for using range(len(...)), you are almost always doing
something wrong. The problem in your code is that you are modifying the tree
while iterating over it. However, please ask this question on the mailing list
as the bug tracker is
Stefan Behnel added the comment:
Funny. I just thought about this yesterday and came up with pretty much the
same idea. +1 for the patch.
I don't think there are more classes to support here. Quite the contrary,
other tools should try to integrate via concurrent.futures.Future in
Stefan Behnel added the comment:
It would be nice to have this applied to 3.5 even. I'm aware that this is a
new feature that shouldn't go in any more at this point, but if it could
get added in 3.5.1 at least, I think it would fill a rather clear and
annoying gap when it com
Stefan Behnel added the comment:
> I find it questionable to mix await and threads, as I have said several
> times in the discussion about Nick Coghlan's proposal to introduce
> helper functions with a similar function.
There are a couple of use cases in the context of asyncio,
Stefan Behnel added the comment:
The fix wasn't applied yet, so the current code in 3.4 and later branches is
still incorrect. Any of the last two patches ("*_value") will fix it, with my
preference on the last one.
--
versio
Stefan Behnel added the comment:
Regarding tests, it looks like iteration isn't currently tested at the C
level at all. At least, the xx test modules don't have any types that use
it. I can write one up next week, or add it to one of the existing types
(Xxo_Type?). Unlikely that I
Stefan Behnel added the comment:
> Also, the uptake for SNs are nearly zero.
That suggests to me that pointing users to it could help. It's currently hidden
in the types module and if I didn't know it existed, I could end up looking in
collections, and failing to find it there,
Stefan Behnel added the comment:
The "can store arbitrary objects" sentence is now duplicated, and still way too
visible. I have to read three sentences until it tells me what I need to know.
--
___
Python tracker
<http://bugs.python.o
Stefan Behnel added the comment:
I think the first two sentences can simply be removed to fix this, without loss
of readability or information.
--
___
Python tracker
<http://bugs.python.org/issue24
Stefan Behnel added the comment:
Please upload your patches as separate, uncompressed files for review.
PGO was already proposed here, but nothing came out of it:
https://bugs.python.org/issue17781
I suggest rejecting that old ticket and sticking with this one since it has an
actual patch
Stefan Behnel added the comment:
Issue 24915 suggests PGO and comes with an actual patch. I suggest rejecting
this ticket as too broad.
--
nosy: +scoder
___
Python tracker
<http://bugs.python.org/issue17
Stefan Behnel added the comment:
Patch and test look correct. They fix a bug that produces incorrect output, so
I vote for backporting them. Most code won't see the difference as whitespace
control characters are rare in attribute values. And code that uses them will
benefit from correc
Stefan Behnel added the comment:
> The only thing that was a bit mystifying were the errors during the
> initial profile run. There is so much that floats by in the terminal
> window that I completely missed the warnings about errors during the
> test run not being anything to worry
Stefan Behnel added the comment:
Could I get a little note in the collections.abc section that the new ABCs
"Generator", "Coroutine" and "Awaitable" are available as backports in the
"backports_abc" package on PyPI?
https://pypi.python.org/pypi/backpo
Stefan Behnel added the comment:
Thanks, Yury!
--
___
Python tracker
<http://bugs.python.org/issue25082>
___
___
Python-bugs-list mailing list
Unsubscribe:
Stefan Behnel added the comment:
> _collections sounds cool, but the flip side is any python without the C
> implemntation would still have the slower startup, right?
I wouldn't bother too much with that, certainly not given the order we are
talking about here. Jython's startu
Stefan Behnel added the comment:
Let's say the change minimises the dependencies. That is a reasonable goal, too.
--
___
Python tracker
<http://bugs.python.org/is
Stefan Behnel added the comment:
Would there be a way to expose these internals rather than hiding them?
--
nosy: +scoder
___
Python tracker
<http://bugs.python.org/issue25
Stefan Behnel added the comment:
Understood and agreed. Second patch looks good to me.
Cython calls PyThreadState_GET() in pretty much every helper function that
deals with exceptions, but I doubt that the potential speed difference is going
to be relevant in the real world. And we target
1101 - 1200 of 1287 matches
Mail list logo