GeezerVison

Nice! Do you have pyperformance benchmark results?

Nope, will run them.

This branch has conflicts that must be resolved
[...]
Python/generated_cases.c.h

I tried to regenerate Python/generated_cases.c.h by porting the make rule:

to the Windows cmd. However, I get an exception:

D:\Oleg\cpython>set PYTHONPATH=D:\Oleg\cpython

D:\Oleg\cpython>python Tools/cases_generator/generate_cases.py --emit-line-directives Python/generated_cases.c.h.new Python/opcode_metadata.h.new
Running Release|x64 interpreter...
Traceback (most recent call last):
  File "D:\Oleg\cpython\Tools\cases_generator\generate_cases.py", line 1259, in <module>
    main()
  File "D:\Oleg\cpython\Tools\cases_generator\generate_cases.py", line 1250, in main
    a.parse()  # Raises SyntaxError on failure
    ^^^^^^^^^
  File "D:\Oleg\cpython\Tools\cases_generator\generate_cases.py", line 557, in parse
    self.parse_file(filename, instrs_idx)
  File "D:\Oleg\cpython\Tools\cases_generator\generate_cases.py", line 568, in parse_file
    with open(filename) as file:
         ^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: 'Python/generated_cases.c.h.new'

D:\Oleg\cpython>python Tools/cases_generator/generate_cases.py --emit-line-directives Python/generated_cases.c.h Python/opcode_metadata.h
Running Release|x64 interpreter...
Traceback (most recent call last):
  File "D:\Oleg\cpython\Tools\cases_generator\generate_cases.py", line 1259, in <module>
    main()
  File "D:\Oleg\cpython\Tools\cases_generator\generate_cases.py", line 1250, in main
    a.parse()  # Raises SyntaxError on failure
    ^^^^^^^^^
  File "D:\Oleg\cpython\Tools\cases_generator\generate_cases.py", line 557, in parse
    self.parse_file(filename, instrs_idx)
  File "D:\Oleg\cpython\Tools\cases_generator\generate_cases.py", line 583, in parse_file
    raise psr.make_syntax_error(
  File "Python/generated_cases.c.h", line 4658
    }
    ^
SyntaxError: Couldn't find '// BEGIN BYTECODES //' in Python/generated_cases.c.h

D:\Oleg\cpython>python
Running Release|x64 interpreter...
Python 3.12.0a5+ (tags/v3.12.0a5-186-gf300a1fa4c:f300a1fa4c, Apr 30 2023, 22:46:31) [MSC v.1929 64 bit (AMD64)] on win32

How should I call it correctly?

@arhadthedev it would be better to make a new separate issue for difficulties regenerating cases on Windows, rather than conflating it into this PR, which isn't really related.

FYI you should be able to run the script without any parameters and it will use the right files.

Also I thought it is run automatically by the Windows build.bat script.

PyPerformance results: https://gist.github.com/mpage/b8f712396755c3fee277f84352058608

Looks neutral except for unpack sequence (1.23x faster) and unpickle (1.30x slower). I'm surprised that this change would affect either of these benchmarks. Are they considered to be reliable?

Just chatted with @mpage and there are a few things we could do to streamline this more:

1. The FRAME_OWNED_BY_GENERATOR pre-check shouldn't be necessary for correctness, we can probably just eliminate it?
2. Since we are doing a runtime check here based on characteristics of the code object (is it an async def) it would make more sense to push that into the compiler. We could extract this into a dedicated SET_AWAITER opcode that the compiler emits only in async defs; then no runtime check is needed.
3. Alternatively if we don't want it in a separate opcode, we could at least specialize away the check by having two versions of SEND_GEN, one where the caller is known to be an async def and one where it isn't.
4. We could decide to have this feature enabled only by a runtime flag (since it's for improved debugging/profiling). In that case we probably wouldn't want it handled in the compiler, since then we'd need different sets of pycs for flag on/off.

My inclination is for #2 (do it in the compiler, new opcode.) Would welcome comments from other reviewers (e.g. @markshannon, @gvanrossum, @willingc).

This adds considerable overhead to the fast path of generators, generator expressions and creating coroutines.
It would be more efficient to link the tasks.
So instead of

T0    +---> T1
|     |     |
C0    |     C2
|     |     |
v     |     v
C1    |     C3
|     |
+-----|

We would have

T0  ---> T1
|        |
C0       C2
|        |
v        v
C1       C3

The original could then be reconstructed as needed.

I guess this is a similar question to Mark's... this seems like something that you could do much more simply directly in asyncio, without touching the core interpreter? When a task does await some_task, then it invokes asyncio.Task.__await__, which is arbitrary code that asyncio controls -- wouldn't it be simpler for that code to make a note about the relationship between the tasks? In some cases it might even let you chain together relationships/"causality" through regular Futures, not just tasks.

Also, I might be missing something, but I think the code here gives the wrong answer in cases where a task has multiple waiters, which is allowed by asyncio semantics:

async def wait_on(task):
    await task

async def main():
    sleep_task = asyncio.ensure_future(asyncio.sleep(10))
    task1 = asyncio.ensure_future(wait_on(sleep_task))
    task2 = asyncio.ensure_future(wait_on(sleep_task))

I think in this case the link from task1 -> sleep_task would get overwritten and lost?

This adds considerable overhead to the fast path of generators, generator expressions and creating coroutines. It would be more efficient to link the tasks. So instead of

The original could then be reconstructed as needed.

It's worth mentioning that the choice to maintain the async stacks on coroutines was to make it as simple as possible to reconstruct the stack from an eBPF probe. I think that linking tasks should be doable, it'll mean that we have to recreate the logic of _PyGen_yf in a probe, but that isn't terribly complicated.

Where were you thinking that we would update the links between tasks?

I guess this is a similar question to Mark's... this seems like something that you could do much more simply directly in asyncio, without touching the core interpreter? When a task does await some_task, then it invokes asyncio.Task.__await__, which is arbitrary code that asyncio controls -- wouldn't it be simpler for that code to make a note about the relationship between the tasks? In some cases it might even let you chain together relationships/"causality" through regular Futures, not just tasks.

It probably doesn't matter in practice, but since __await__ is an ordinary method there's nothing that guarantees that whatever calls __await__ will await the awaitable that is returned (wow that was a mouthful!).

Also, I might be missing something, but I think the code here gives the wrong answer in cases where a task has multiple waiters, which is allowed by asyncio semantics:

async def wait_on(task):
    await task

async def main():
    sleep_task = asyncio.ensure_future(asyncio.sleep(10))
    task1 = asyncio.ensure_future(wait_on(sleep_task))
    task2 = asyncio.ensure_future(wait_on(sleep_task))

I think in this case the link from task1 -> sleep_task would get overwritten and lost?

Yep, this is a limitation of the implementation, the last awaiter wins. This has worked out OK for us so far.

I'm not going to be able to give this enough attention any time soon. Would it be okay to consider this for 3.13 instead of 3.12? 3.12 feature freeze (beta 1) was supposed to be today, and while the RM has postponed that by two weeks, he also gave guidance that this is not meant as an encouragement for new features (the SC ruled on a few pending PEPs, allowing some, postponing others to 3.13).

I was already assuming this was under discussion for 3.13, not a candidate for 3.12.

I'm having another stab at this taking into account what was discussed here. For more details see my comment here: #91048 (comment).

Thanks for this, we went with gh-124640 instead.