Repository navigation
Implement types._GeneratorWrapper in C #133372
Description
Feature or enhancement
Proposal:
I've found an old TODO line in types module and decided to give it a try:
https://github.com/python/cpython/blob/main/Lib/types.py#L253
My PR with this change will be linked soon.
Has this already been discussed elsewhere?
No response given
Links to previous discussion of this feature:
No response
Activity
sobolevn commented on May 4, 2025
Please, before diving to deep into it: explain what we will achieve by doing so. For example: performance (and in what cases). Also: take extra care about potential compatibility changes.
pythonGH-133372: C implementation of types._GeneratorWrapper is added.
efimov-mikhail commented on May 4, 2025
Actually, base PR is already present, I've linked it.
In my opinion, this change is kinda similar to two things that we already have in codebase.
First is basic collections that have implementations both in Python and C, OrderedDict for example.
Pure Python implementation will not be removed but faster C implementation is added and it will be used if possible.
Second is PyCoroWrapper in genobject.c which wraps generator object.
IMO, _GeneratorWrapper is very similar to this class and can be implemented in C for performance.
I don't think that performance gain is big, mainly because _GeneratorWrapper is only used in types.coroutine decorator.
And that decorator is kinda old-style and not used much.
But this is still an improvement in very basic class for async programming.
I also slightly improve pure Python implementation of _GeneratorWrapper: gi_suspended and cr_suspended properties are added.
IMO, they were missed by mistake.
Speaking about compatibility, I can see no problems, because C version is very close to pure Python version.
Only exception texts aren't equal and setting arbitrary values to __name__ / __qualname__ is prohibited as with general functions and generators.
efimov-mikhail commented on May 4, 2025
Moreover, there is another "TODO: Implement this in C." line in this file:
cpython/Lib/types.py
Line 308 in a247dd3
It seems that types.coroutine also should be implemented both in C and in Python.
I can make this change in separate PR if this one will be accepted.
ZeroIntensity commented on May 4, 2025
I tried this a few months ago on my own, and from that experience, I'm really concerned about this change.
picnixz commented on May 4, 2025
How fast are we in terms of import time however? Here it's interesting to see if we can improve the import time of types.
sobolevn commented on May 4, 2025
I also fear that we might introduce some bugs with the rewrite. Since @types.coroutine is very outdated, I don't think that we should really touch it :(
Unless we see a really good reasoning.
AA-Turner commented on May 4, 2025
Would it be worth considering deprecating @types.coroutine?
In the top 10,000 PyPI projects, it's only used in 91: amazon-ion, aspidites, async-generator, esphome, langgraph, nuitka, pycapnp, telnetlib3, and trio.
Of these, the uses in aspidites, nuitka, and pycapnp are test-only.
A
Footnotes
excluding asyncio and taskgroup, which are stdlib backports ↩
sobolevn commented on May 4, 2025
I think we can do that for 3.15, it would mark 10 versions of async def addition.
But, asyncio team should decide on it :)
AA-Turner commented on May 4, 2025
cc @kumaraditya303
kumaraditya303 commented on May 5, 2025
asyncio doesn't support generator based coroutines anymore so optimizing this isn't worth it. I think the comment should be removed as it is not relevant nowadays.
efimov-mikhail commented on May 5, 2025
IMO, this is very good question, and we could consider it in the details.
FYI, 4 years ago there were reasons why not to deprecate it:
#81102 (comment)
I should learn these code snippets deeply to better understand the current situation.
AA-Turner commented on May 5, 2025
also cc @njsmith. Both the examples cited by Nathaniel are 'simple' cases where all types.coroutine does is add a flag to co_flags, so another alternative would be to consider deprecating wrapping generator-like functions & _GeneratorWrapper, but keep the flag-mutation code.
A
efimov-mikhail commented on May 9, 2025
I've started a discussion about deprecation:
https://discuss.python.org/t/proposal-deprecate-types-coroutine-decorator-and-types-generatorwrapper-class/91367
kumaraditya303 commented on May 21, 2025
I don't there is any benefit of changing or deprecating it, there are valid use cases for it and I think it's best to just remove the comment of implementing it in C and close this issue.
gh-133372: remove out of date todos from types module about generator…
pythongh-133372: remove out of date todos from types module about gen…
pythongh-133372: remove out of date todos from types module about gen…
pythongh-133372: remove out of date todos from types module about gen…