[issue41756] Do not always use exceptions to return result from coroutine

[Petr Viktorin]

Petr Viktorin  added the comment:

that is, bpo-44727

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Petr Viktorin]

Petr Viktorin  added the comment:

Hello!
This change added an enum to the stable ABI:
typedef enum {
PYGEN_RETURN = 0,
PYGEN_ERROR = -1,
PYGEN_NEXT = 1,
} PySendResult;

Adding new values to enums might break the stable ABI in some (admittedly rare) 
cases; so usually it's better to avoid enums and stick with int and #ifdef'd 
values.
This particular one looks like it won't be extended and won't cause problems, 
but still, switching to int would make stable ABI easier to work with.
Is anyone against switching to int?

See pbo-44727 for details.

--
nosy: +petr.viktorin

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Andrew Svetlov]

Change by Andrew Svetlov :


--
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Andrew Svetlov]

Andrew Svetlov  added the comment:

Can we close the issue?

--
nosy: +asvetlov

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:


New changeset cfb0f57ff876ab3d04ff144f19eda58844981643 by Vladimir Matveev in 
branch 'master':
bpo-41756: Export PyGen_Send and wrap it in if-defs (#22677)
https://github.com/python/cpython/commit/cfb0f57ff876ab3d04ff144f19eda58844981643


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Change by Vladimir Matveev :


--
pull_requests: +21649
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/22677

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> Also, in Include/abstract.h it should only be available for limited C API >= 
> 3.10:

Vladimir, could you please submit a PR?

> We now should discuss the behavior of PySend_Iter() if value is NULL. It is 
> not documented, the current behavior differs from the behavior for non-NULL 
> value, and it is used in the ceval loop.

IMO: I'd keep the behavior and just document it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Change by Yury Selivanov :


--
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Few things I forget about. The new C API function should be exported in 
PC/python3dll.c.

Also, in Include/abstract.h it should only be available for limited C API >= 
3.10:

#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A
...
#endif

We now should discuss the behavior of PySend_Iter() if value is NULL. It is not 
documented, the current behavior differs from the behavior for non-NULL value, 
and it is used in the ceval loop. We should document this case explicitly and 
maybe change the behavior if it would be more appropriate.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

With the latest PR now merged this issue can be closed. Please reopen if there 
are any other action items left.

--
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Change by Vladimir Matveev :


--
pull_requests: +21639
pull_request: https://github.com/python/cpython/pull/22663

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Mark Shannon]

Mark Shannon  added the comment:

What's the difference between removing it and making it properly private (i.e. 
static)?

It's an irrelevant detail whether the code is inlined or in a helper function.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Stefan Behnel]

Stefan Behnel  added the comment:

> Don't forget to remove PyGen_Send()

That's the function that allows sending data into a generator. It's also used 
internally by "PyIter_Send()". Are you really suggesting to remove it, or to 
make it underscore-private again? (As it was before.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Mark Shannon]

Mark Shannon  added the comment:

Vladimir,
Thanks for adding PyIter_Send().

Don't forget to remove PyGen_Send() :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:


New changeset 037245c5ac46c3436f617a1f5d965929754be239 by Vladimir Matveev in 
branch 'master':
bpo-41756: Add PyIter_Send function (#22443)
https://github.com/python/cpython/commit/037245c5ac46c3436f617a1f5d965929754be239


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Change by Vladimir Matveev :


--
pull_requests: +21473
stage: resolved -> patch review
pull_request: https://github.com/python/cpython/pull/22443

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Vladimir Matveev  added the comment:

No, I don't think so but I can definitely make one. A few questions first:
- having PySendResult as a result type of PyIterSend seems ok, however prefix 
for each concrete value (PYGEN_*) is not aligned with the prefix of the 
function itself (PyIter_)
- should it also deal with tstate->c_tracefunc (probably not) or just be 
something like
```
PySendResult
PyIter_Send(PyObject *iter, PyObject *arg, PyObject **result)
{
_Py_IDENTIFIER(send);
assert(result != NULL);

if (PyGen_CheckExact(iter) || PyCoro_CheckExact(iter)) {
return PyGen_Send((PyGenObject *)iter, arg, result);
}

if (arg == Py_None && Py_TYPE(iter)->tp_iternext != NULL) {
*result = Py_TYPE(iter)->tp_iternext(iter);
}
else {
*result = _PyObject_CallMethodIdOneArg(iter, _send, arg);
}

if (*result == NULL) {
if (_PyGen_FetchStopIterationValue(result) == 0) {
return PYGEN_RETURN;
}
return PYGEN_ERROR;
}
return PYGEN_NEXT;
}
```

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

No, I meant a function which combines PyGen_Send, tp_iternext and 
_PyGen_FetchStopIterationValue. Was not it in your PR?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Vladimir Matveev  added the comment:

Serhiy, AFAIR PyIter_Send in my PR appear only as a rename from placeholder 
`Name_TBD` and it still was specific to PyGenObjects. Do you mean something 
that was listed in https://bugs.python.org/msg377007 ?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I agree that we should add PyIter_Send() (the name is discussable, but 
PyIter_Send LGTM) which should support iterators and objects with the send() 
method. It would be much more useful for user, and can replace PyGen_Send() in 
the interpreter core code. But merging the PR with PyGen_Send() was not a 
mistake. It was good to split the changes on smaller steps easier to review.

Vladimir, do you mind to create a new PR for PyIter_Send()? I seen its 
implementation in one of intermediate versions of your PR.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:


New changeset 6c33385e3a1dce31d7b9037eebfc584075795dba by Serhiy Storchaka in 
branch 'master':
bpo-41756: Refactor gen_send_ex(). (GH-22330)
https://github.com/python/cpython/commit/6c33385e3a1dce31d7b9037eebfc584075795dba


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> If you disagree with me, please say why, don't just merge the PR.

Apologies, Mark. I didn't intend to merge something bypassing your opinion; 
just missed your comment between reviewing multiple PRs in a few unrelated 
repos. I'm sorry.

On the actual naming subject, you proposed:

> `PySendResult PyIter_Send(PyObject *obj, PyObject *arg, PyObject **result);`

The problem with using this name is that ideally we should also support 
non-native coroutine and generator implementations (i.e. resolve the "send" 
attribute and call it using Python calling convention). Ideally we should have 
two C APIs: one low-level supporting only native objects and a high level one, 
supporting all kinds of them.

Can we perhaps add both `PyGen_Send()` and `PyCoro_Send()` for now that would 
only accept generators and coroutines respectively? After that we can discuss 
adding a more generic `PyIter_Send`?


> Would you revert the PR, please.

Since this is in 3.10/master that nobody uses right now except us (Python core 
devs), can we just issue a follow up PR to fix whatever is there to fix? I'd 
like to avoid the churn of reverting, and again, I apologize for pushing this a 
bit hastily.  Let me know if you actually want a revert and I'll do that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Mark Shannon]

Mark Shannon  added the comment:

Yury,

Why was the PR merged with a new API function `PyGen_Send`?

I explicitly said that any new API function should *not* start with `PyGen`, 
nor should any function rely on generators and async "coroutines" sharing the 
same memory layout.

If you disagree with me, please say why, don't just merge the PR.

The name `PyGen` is misleading as it can handle coroutines as well as 
generators.
There is no performance advantage to only handling these two types.
Worse, it requires that a `PyCoroObject` can always be cast to a `PyGenObject`, 
preventing better layout of either object in the future.


Would you revert the PR, please.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

Vladimir, could you please submit a PR to update 3.10/whatsnew? Need to mention 
both the new C API and the new perf boost in relevant sections.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

PR 22330 refactors gen_send_ex(), making it similar to the new PyGen_Send().

--
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +21374
pull_request: https://github.com/python/cpython/pull/22330

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

Thanks, Vladimir! Also huge thanks to Mark, Serhiy, and Stefan. If there are 
any nits to fix let's do that in follow up PRs.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:


New changeset 2b05361bf7cbbd76035206fd9befe87f37489f1e by Vladimir Matveev in 
branch 'master':
bpo-41756: Introduce PyGen_Send C API (GH-22196)
https://github.com/python/cpython/commit/2b05361bf7cbbd76035206fd9befe87f37489f1e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Vladimir Matveev  added the comment:

Yes, it should be

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> Sounds like a good middleground to start: add ``PySendResult `` and 
> `PySendResult PyGen_Send(PyGenObject*, PyObject* PyObject**)` specific to 
> generators and coroutines. 

Yes, it seems that everybody agreed on that. I can give the PR another review 
-- is it ready?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Vladimir Matveev  added the comment:

Sounds like a good middleground to start: add ``PySendResult `` and 
`PySendResult PyGen_Send(PyGenObject*, PyObject* PyObject**)` specific to 
generators and coroutines. Subsequent changes could introduce `PySendResult 
PyIter_Send(PyObject*, PyObject*, PyObject**)` that would be more generic (call 
tp_next, invoke "send" or maybe in the future use dedicated slot for "send" 
operator so i.e. asyncio.Future or Cython coroutines could benefit from the 
same optimization)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> I would also have preferred a more type specific function, but yeah, as long 
> as the types for which the function would normally be used are special cased 
> early enough in the implementation, it makes no big difference.

Maybe add two API funcs: PyGen_Send (specific to generators & coroutines) and 
PyIter_Send?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

We introduced _PyObject_LookupAttr() and _PyObject_GetMethod() for similar 
purposes. And they have similar signatures. Although they all are private.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Stefan Behnel]

Stefan Behnel  added the comment:

BTW, just to give this a house number, I remember having measured a performance 
improvement of up to 70% at some point when switching from "generators always 
raise a StopIteration at the end" to "generators just return NULL" in Cython. 
For short-running generators and coroutines, this can make a big difference.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Stefan Behnel]

Stefan Behnel  added the comment:

I would also have preferred a more type specific function, but yeah, as long as 
the types for which the function would normally be used are special cased early 
enough in the implementation, it makes no big difference.

Fine with me, too.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

I would be fine even with a generator-specific API as a first step, for 
simplicity. But the end goal is to support all generator-like objects. It is 
much more useful for end users and does not have drawbacks.

Enum for result seems not necessary (other functions with three-state result 
just return -1, 0 or 1), but if other core developers want it -- OK.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> Does it sound correct?

It does, but given the amount of back and forth on this, I'd wait for Serhiy 
and Stefan to confirm if they're OK.

IMO the `PyIter_Send` name is OK (given how generic the implementation will be) 
and returning an enum, while a tad unconventional for the C API is way more 
convenient for the developer.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Vladimir Matveev  added the comment:

so to summarize:

Proposed function signature:
```
PySendResult PyIter_Send(PyObject *obj, PyObject *arg, PyObject **result);
```

For generators/coroutines function will delegate to specialized implementation 
that does not raise StopIteration exception
For types that provide `tp_iternext` if arg is Py_None function call invoke 
`Py_TYPE(obj)->tp_iternext(obj)`
For all other cases function will try to call `send` method

Regarding of the case function will not raise StopIteration and will always 
return pair status/result.

Does it sound correct?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> I guess `PyIter_Send` would imply that this function should work for all 
> inputs (like in https://bugs.python.org/msg377007) which also sounds 
> reasonable.

I like `PyIter_Send` (and why I initially proposed it myself) because it will 
also work with the "send" slot defined on some types if we end up adding one.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Vladimir Matveev  added the comment:

I guess `PyIter_Send` would imply that this function should work for all inputs 
(like in https://bugs.python.org/msg377007) which also sounds reasonable.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Mark Shannon]

Mark Shannon  added the comment:

I agree with Serhiy, that `PyGen_` is a bad prefix.

Unless a function takes generator objects and *only* generators objects, then 
it shouldn't have a `PyGen` prefix.

The API function is the C equivalent of obj.send(val).
The first parameter is an object.

Coroutines do not inherit from generators.
That the the C implementations are so coupled is an unfortunate historical 
accident, and may well be changed.

Regardless of how this is implemented internally, any API function's signature 
should reflect the equivalent Python code.
And please use an enum, not an int, as the return value. It's a huge boon for 
readability.

I would suggest:
`PySendResult PyIter_Send(PyObject *obj, PyObject *arg, PyObject **result);`

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

There are other abstract object APIs: PyNumber, PySequence, PyMapping, etc. In 
particularly PyIter_Next() works with the iterator protocol, there is no single 
iterator class. Seems PyGen_* API is related to concrete class, but we can 
introduce new namespace for the generator protocol.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> I think it should be specific to generators and coroutines. Calling 
> `PyObject_CallMethodIdOneArg(coro, _send, arg);` and interpreting 
> exceptions to emulate the low level API seems a bit too much.

To add to my point: typically higher-level APIs go under the `PyObject_*` 
namespace, whereas `Py{Type}_*` is more concrete. So I'd make `PyGen_Send` to 
only work with `PyGen` and `PyCoro`.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> Since coroutines inherit the generator protocol more or less, I think 
> "PyGen_Send()" is a more suitable name, better than "PyCoro_Send()".

OK, +1. PyGen_Send it is.

> Also should it be specific to generators/coroutines and accept PyGenObject*

I think it should be specific to generators and coroutines. Calling 
`PyObject_CallMethodIdOneArg(coro, _send, arg);` and interpreting 
exceptions to emulate the low level API seems a bit too much.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Vladimir Matveev  added the comment:

Also should it be specific to generators/coroutines and accept PyGenObject* or 
should it try to handle multiple cases and expose the result for them in 
uniform way, i.e.
```
if (PyGen_CheckExact(gen) || PyCoro_CheckExact(gen)) {
   // use coroutine/generator specific code that avoids raising exceptions
   *result = ...
   return PYGEN_RETURN;
}
PyObject *ret;
if (arg == Py_None) {
  ret = Py_TYPE(gen)->tp_iternext(gen);
}
else {
  ret = _PyObject_CallMethodIdOneArg(coro, _send, arg);
}
if (ret != NULL) {
  *result = ret;
  return PYGEN_YIELD;
}
if (_PyGen_FetchStopIterationValue(result) == 0) {
  return PYGEN_RETURN;
}
return PYGEN_ERROR;
```

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Stefan Behnel]

Stefan Behnel  added the comment:

I'm happy to see this moving forward.

Not convinved of the "PyIter_Send()" name, though. I don't consider this part 
of the iterator protocol. It's specific to generators and coroutines. Cython 
would probably guard its usage by "PyGen_CheckExact()" or 
"PyCoro_CheckExact()", and not use it for arbitrary iterators.

Since coroutines inherit the generator protocol more or less, I think 
"PyGen_Send()" is a more suitable name, better than "PyCoro_Send()".

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

> As for returned value, I propose to return -1 in case of error, 1 for yielded 
> value and 0 for returned value (i.e. define PYGEN_RETURN = 0, PYGEN_YIELD = 1 
> and PYGEN_ERROR = -1, but without exposing public names).

Sure, that works.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

As for returned value, I propose to return -1 in case of error, 1 for yielded 
value and 0 for returned value (i.e. define PYGEN_RETURN = 0, PYGEN_YIELD = 1 
and PYGEN_ERROR = -1, but without exposing public names).

It would be uniform with other C API: many functions return -1 on error (if 
they return int and can fail), and PyDict_Next() and _PySet_NextEntry() return 
1 for every yielded item, and 0 if the iteration has been finished.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

Mark, Stefan,

I don't want this to be stale so I propose to move with my suggestions:

1. We make the new API public. Mark, if you have objections to that - please 
elaborate with some details. IMO, the corresponding Python API is long public 
and there's no harm in exposing a C version of it. Especially given the fact 
that uvloop, cython, and even asyncio itself will be relying on that API.

2. I propose to name the new API `PyIter_Send`. Motivation: it will work with 
both generators and coroutines and plays nicely with `PyIter_Next`.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

@Mark
> Mark Shannon wrote: I don't think [the C-API function] should be public, as a 
> possible further improvement is to stop passing exceptions through a side 
> channel, but in result. Maybe we don't want to do that, but lets' not add to 
> the (already rather large) C-API.

Yeah, we can add it as a "private" function, I'm not entirely opposed to that. 
But... it would be great if Cython and C code could still depend on it and use 
it. And then... why should it be private? The corresponding Python API 
"gen.send()" and "gen.throw()" is public, why can't the C API be public too?

We will not fundamentally change generators (it would be a major backwards 
incompatible change), so committing to a good C API sounds reasonable.

@Mark
> Remember that PyIter_Next() is pretty much the same, though, and it has the 
> standard "return PyObject*" interface. These two would diverge then.

Maybe we should call it `_PyIter_Send()`?  While `.send()` is mostly about 
coroutines, regular generators have the method too, and it would be weird to 
call `_PyCoro_Send` on a generator object.

@Vladimir
> PYGEN_ERROR  | NULL | Regular PyErr_* functions should be used to work 
> with error case

Correct.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Vladimir Matveev  added the comment:

If I understand proposed shape of API correctly - it was not supposed to return 
exception via "result" so contract for new `PyGen_Send` function is something 
like:

Return value | result   | Comment
-
PYGEN_RETURN | not NULL | Returned value
PYGEN_NEXT   | not NULL | Yielded value
PYGEN_ERROR  | NULL | Regular PyErr_* functions should be used to work with 
error case

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Stefan Behnel]

Stefan Behnel  added the comment:

Copying some of the design discussion from the PR here 
(https://github.com/python/cpython/pull/22196/files#r486730457), because it 
belongs into the ticket.

Yury Selivanov proposed to add a new C-API function for this (naming changes by 
me):

typedef enum {PYGEN_RETURN, PYGEN_ERROR, PYGEN_NEXT} PyGenSendStatus;

PyGenSendStatus PyGen_Send(PyGenObject *gen, PyObject *arg, PyObject 
**result);

Mark Shannon and I agreed that the status code should be the return value, with 
some confusion whether "PyGen_" or "PyCoro_" would be appropriate prefixes.

Mark Shannon wrote: I don't think [the C-API function] should be public, as a 
possible further improvement is to stop passing exceptions through a side 
channel, but in result. Maybe we don't want to do that, but lets' not add to 
the (already rather large) C-API.

However, I think this will be demanded and used by extensions, including Cython 
implemented ones, so it seems better to make them use a public function than a 
private one.

Let's continue these lines of discussion here.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Stefan Behnel]

Stefan Behnel  added the comment:

Big +1 from me, too, for the same reasons Yury gave.

--
nosy: +scoder

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Yury Selivanov]

Yury Selivanov  added the comment:

Big +1 from me. This is something I always wanted to do myself (since the time 
of PEP 492 & 525 implementations) and I think this is a necessary change. It's 
great that this isn't just a C API UX improvement but also yields a big perf 
improvement.

--
nosy: +Mark.Shannon, lukasz.langa, vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

Change by Vladimir Matveev :


--
keywords: +patch
pull_requests: +21255
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/22196

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41756] Do not always use exceptions to return result from coroutine

[Vladimir Matveev]

New submission from Vladimir Matveev :

Currently async functions are more expensive to use comparing to their sync 
counterparts. A simple microbenchmark shows that difference could be quite 
significant:
```
import time

def f(a):
if a == 0:
return 0
return f(a - 1)

async def g(a):
if a == 0:
return 0
return await g(a - 1)

N = 10
C = 200

t0 = time.time()
for _ in range(N):
f(C)
t1 = time.time()
for _ in range(N):
try:
g(C).send(None)
except StopIteration:
pass
t2 = time.time()

print(f"Sync functions: {t1 - t0} s")
print(f"Coroutines: {t2 - t1} s")
```
Results from master on my machine:

Sync functions: 2.8642687797546387 s
Coroutines: 9.172159910202026 s

NOTE: Due to viral nature of async functions their number in codebase could 
become quite significant so having hundreds of them in a single call stack is 
not something uncommon.

One of reasons of such performance gap is that async functions always return 
its results via raising StopIteration exception which is not cheap. This can be 
avoided if in addition to `_PyGen_Send` always return result via exception we 
could have another function that will allow us to distinguish whether value 
that was returned from generator is a final result (return case) of whether 
this is yielded value.
In linked PR I've added function `_PyGen_SendNoStopIteration` with this 
behavior and updated ceval.c and _asynciomodule.c to use it instead of 
`_PyGen_Send` which resulted in a measurable difference:

Sync functions: 2.8861589431762695 s
Coroutines: 5.730362176895142 s

--
messages: 376698
nosy: v2m, yselivanov
priority: normal
severity: normal
status: open
title: Do not always use exceptions to return result from coroutine
type: performance
versions: Python 3.10

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com