Skip to content

bpo-42085: Introduce dedicated entry in PyAsyncMethods for sending values #22780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
1st1 merged 8 commits into from
Nov 10, 2020
Merged

bpo-42085: Introduce dedicated entry in PyAsyncMethods for sending values #22780

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
cdd612e
Introduce dedicated entry in PyAsyncMethods for sending values
vladima Oct 19, 2020
7fb80fd
PR feedback
vladima Oct 19, 2020
d11ba24
Added NEWS
vladima Oct 19, 2020
4635bf1
Added am_send for generators
vladima Oct 20, 2020
6559a1e
Added Py_TPFLAGS_HAVE_SEND flag
vladima Oct 20, 2020
0269d6c
add if-def around Py_TPFLAGS_HAVE_SEND
vladima Oct 20, 2020
5006b27
Updated docs
vladima Oct 21, 2020
016f3e9
fix typo in the signature of sendfunc
vladima Oct 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions Include/abstract.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,11 +339,6 @@ PyAPI_FUNC(int) PyIter_Check(PyObject *);
PyAPI_FUNC(PyObject *) PyIter_Next(PyObject *);

#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000
typedef enum {
PYGEN_RETURN = 0,
PYGEN_ERROR = -1,
PYGEN_NEXT = 1,
} PySendResult;

/* Takes generator, coroutine or iterator object and sends the value into it.
Returns:
Expand Down
13 changes: 13 additions & 0 deletions Include/cpython/object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,23 @@ typedef struct {
objobjargproc mp_ass_subscript;
} PyMappingMethods;

#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000
Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cpython/object.h is only imported when Py_LIMITED_API is not defined, so this #if does not make sense. Also, moving the declaration of PySendResult here removes it from the limited API. It actually break the limited API because importing abstract.h with the limited API will be error.

typedef enum {
PYGEN_RETURN = 0,
PYGEN_ERROR = -1,
PYGEN_NEXT = 1,
} PySendResult;

typedef PySendResult (*sendfunc)(PyObject *iter, PyObject *value, PyObject **result);
#endif

typedef struct {
unaryfunc am_await;
unaryfunc am_aiter;
unaryfunc am_anext;
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000
sendfunc am_send;
Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not we add corresponding Py_TPFLAGS_HAVE_ flag for binary compatibility?

Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typeslots.h should be updated.

Copy link
Member

@1st1 1st1 Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not we add corresponding Py_TPFLAGS_HAVE_ flag for binary compatibility?

Do we support extensions compiled for higher versions of Python in lower versions of Python? If I compile something for 3.10 I wouldn't expect it to work in 3.9.

Copy link
Member

@1st1 1st1 Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we don't need a Py_TPFLAGS_HAVE_ flag and I don't remember adding one for the as_async slot when we implemented PEP 492.

Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if you compile something for 3.9 it is expected to work in 3.10 and do not read uninitialized slot.

Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps there is a bug in the PEP 492 implementation.

Copy link
Member

@1st1 1st1 Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, I see now, thanks for explaining! Yeah, in this case we do need a flag.

Copy link
Member

@1st1 1st1 Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps there is a bug in the PEP 492 implementation.

Well, I don't think we need to do anything now that as_async has been there since 3.5. But looking back, maybe we should have added a flag.

Copy link
Contributor

@scoder scoder Nov 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used an existing slot for PEP-492, so the ABI did not change.
I don't think we necessarily need a new flag. See https://bugs.python.org/issue32388

#endif
} PyAsyncMethods;

typedef struct {
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_sys.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1340,7 +1340,7 @@ def delx(self): del self.__x
check(int, s)
# class
s = vsize(fmt + # PyTypeObject
'3P' # PyAsyncMethods
'4P' # PyAsyncMethods
'36P' # PyNumberMethods
'3P' # PyMappingMethods
'10P' # PySequenceMethods
Expand Down
48 changes: 38 additions & 10 deletions Modules/_asynciomodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1484,7 +1484,8 @@ future_cls_getitem(PyObject *cls, PyObject *type)
static PyAsyncMethods FutureType_as_async = {
(unaryfunc)future_new_iter, /* am_await */
0, /* am_aiter */
0 /* am_anext */
0, /* am_anext */
0, /* am_send */
};

static PyMethodDef FutureType_methods[] = {
Expand Down Expand Up @@ -1602,37 +1603,55 @@ FutureIter_dealloc(futureiterobject *it)
}
}

static PyObject *
FutureIter_iternext(futureiterobject *it)
static PySendResult
FutureIter_am_send(futureiterobject *it, PyObject *arg, PyObject **result)
Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is arg ignored?

Copy link
Contributor Author

@vladima vladima Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, there is a comment in FutureIter_send, I can copy it here as well:

    /* Future.__iter__ doesn't care about values that are pushed to the
     * generator, it just returns self.result().
     */

{
PyObject *res;
FutureObj *fut = it->future;

if (fut == NULL) {
return NULL;
return PYGEN_ERROR;
}

if (fut->fut_state == STATE_PENDING) {
if (!fut->fut_blocking) {
fut->fut_blocking = 1;
Py_INCREF(fut);
return (PyObject *)fut;
*result = (PyObject *)fut;
return PYGEN_NEXT;
}
PyErr_SetString(PyExc_RuntimeError,
"await wasn't used with future");
return NULL;
return PYGEN_ERROR;
}

it->future = NULL;
res = _asyncio_Future_result_impl(fut);
if (res != NULL) {
/* The result of the Future is not an exception. */
(void)_PyGen_SetStopIterationValue(res);
Py_DECREF(res);
*result = res;
return PYGEN_RETURN;
}

Py_DECREF(fut);
return NULL;
return PYGEN_ERROR;
}

static PyObject *
FutureIter_iternext(futureiterobject *it)
{
PyObject *result;
switch (FutureIter_am_send(it, Py_None, &result)) {
case PYGEN_RETURN:
(void)_PyGen_SetStopIterationValue(result);
Py_DECREF(result);
return NULL;
case PYGEN_NEXT:
return result;
case PYGEN_ERROR:
return NULL;
default:
Py_UNREACHABLE();
}
}

static PyObject *
Expand Down Expand Up @@ -1721,12 +1740,21 @@ static PyMethodDef FutureIter_methods[] = {
{NULL, NULL} /* Sentinel */
};

static PyAsyncMethods FutureIterType_as_async = {
0, /* am_await */
0, /* am_aiter */
0, /* am_anext */
(sendfunc)FutureIter_am_send, /* am_send */
};


static PyTypeObject FutureIterType = {
PyVarObject_HEAD_INIT(NULL, 0)
"_asyncio.FutureIter",
.tp_basicsize = sizeof(futureiterobject),
.tp_itemsize = 0,
.tp_dealloc = (destructor)FutureIter_dealloc,
.tp_as_async = &FutureIterType_as_async,
.tp_getattro = PyObject_GenericGetAttr,
.tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC,
.tp_traverse = (traverseproc)FutureIter_traverse,
Expand Down
3 changes: 2 additions & 1 deletion Modules/_testcapimodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6142,7 +6142,8 @@ awaitObject_await(awaitObject *ao)
static PyAsyncMethods awaitType_as_async = {
(unaryfunc)awaitObject_await, /* am_await */
0, /* am_aiter */
0 /* am_anext */
0, /* am_anext */
0, /* am_send */
};


Expand Down
16 changes: 11 additions & 5 deletions Objects/genobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,9 @@ PyIter_Send(PyObject *iter, PyObject *arg, PyObject **result)
_Py_IDENTIFIER(send);
assert(arg != NULL);
assert(result != NULL);

if (Py_TYPE(iter)->tp_as_async != NULL && Py_TYPE(iter)->tp_as_async->am_send != NULL) {
return Py_TYPE(iter)->tp_as_async->am_send(iter, arg, result);
}
if (PyGen_CheckExact(iter) || PyCoro_CheckExact(iter)) {
return gen_send_ex2((PyGenObject *)iter, arg, result, 0, 0);
}
Expand Down Expand Up @@ -1031,7 +1033,8 @@ static PyMethodDef coro_methods[] = {
static PyAsyncMethods coro_as_async = {
Copy link
Member

@1st1 1st1 Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, with this PR regular generators no longer benefit from the faster PyIter_Send. I think we should implement am_send for regular generators too.

(unaryfunc)coro_await, /* am_await */
0, /* am_aiter */
0 /* am_anext */
0, /* am_anext */
0, /* am_send */
};

PyTypeObject PyCoro_Type = {
Expand Down Expand Up @@ -1413,7 +1416,8 @@ static PyMethodDef async_gen_methods[] = {
static PyAsyncMethods async_gen_as_async = {
0, /* am_await */
PyObject_SelfIter, /* am_aiter */
(unaryfunc)async_gen_anext /* am_anext */
(unaryfunc)async_gen_anext, /* am_anext */
0, /* am_send */
};


Expand Down Expand Up @@ -1676,7 +1680,8 @@ static PyMethodDef async_gen_asend_methods[] = {
static PyAsyncMethods async_gen_asend_as_async = {
PyObject_SelfIter, /* am_await */
0, /* am_aiter */
0 /* am_anext */
0, /* am_anext */
0, /* am_send */
};


Expand Down Expand Up @@ -2084,7 +2089,8 @@ static PyMethodDef async_gen_athrow_methods[] = {
static PyAsyncMethods async_gen_athrow_as_async = {
PyObject_SelfIter, /* am_await */
0, /* am_aiter */
0 /* am_anext */
0, /* am_anext */
0, /* am_send */
};


Expand Down