Skip to content

GH-128534: Instrument branches for async for loops. #130569

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.

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
merged 3 commits into from
Feb 27, 2025

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Feb 26, 2025

@markshannon markshannon merged commit 2a18e80 into python:main Feb 27, 2025
69 checks passed
@markshannon markshannon deleted the instrument-branches-async-for branch February 27, 2025 11:07
@nedbat
Copy link
Member

nedbat commented Mar 1, 2025

I think I need some help understanding the bytecode generated. This Python:

    16 │ async def doit():
    17 │     async for letter in AsyncIteratorWrapper("abc"):
    18 │         print(letter)
    19 │     print(".")

Generates this bytecode:

16:?-16:?              0       RETURN_GENERATOR
16:?-16:?              2       POP_TOP
16:0-16:0       L1:    4       RESUME                   0

17:24-17:44            6       LOAD_GLOBAL              1 (AsyncIteratorWrapper + NULL)
17:45-17:50           16       LOAD_CONST               0 ('abc')
17:24-17:51           18       CALL                     1
17:24-17:51           26       GET_AITER
17:4-18:21      L2:   28       GET_ANEXT
17:4-18:21            30       LOAD_CONST               1 (None)
17:4-18:21      L3:   32       SEND                     3 (to L6)
17:4-18:21      L4:   36       YIELD_VALUE              1
17:4-18:21      L5:   38       RESUME                   3
17:4-18:21            40       JUMP_BACKWARD_NO_INTERRUPT 5 (to L3)
17:4-18:21      L6:   42       END_SEND
17:4-18:21      L7:   44       NOT_TAKEN
17:14-17:20           46       STORE_FAST               0 (letter)

18:8-18:13            48       LOAD_GLOBAL              3 (print + NULL)
18:14-18:20           58       LOAD_FAST                0 (letter)
18:8-18:21            60       CALL                     1
18:8-18:21            68       POP_TOP
18:8-18:21            70       JUMP_BACKWARD           23 (to L2)

17:4-18:21      L8:   74       CLEANUP_THROW
17:4-18:21      L9:   76       JUMP_BACKWARD_NO_INTERRUPT 18 (to L6)
17:24-17:51    L10:   78       END_ASYNC_FOR

19:4-19:9             80       LOAD_GLOBAL              3 (print + NULL)
19:10-19:13           90       LOAD_CONST               2 ('.')
19:4-19:14            92       CALL                     1
19:4-19:14           100       POP_TOP
19:4-19:14           102       LOAD_CONST               1 (None)
19:4-19:14           104       RETURN_VALUE

  --           L11:  106       CALL_INTRINSIC_1         3 (INTRINSIC_STOPITERATION_ERROR)
  --                 108       RERAISE                  1
ExceptionTable:
  L1 to L2 -> L11 [0] lasti
  L2 to L4 -> L10 [1]
  L4 to L5 -> L8 [3]
  L5 to L7 -> L10 [1]
  L7 to L8 -> L11 [0] lasti
  L8 to L9 -> L10 [1]
  L10 to L11 -> L11 [0] lasti

How can execution get to offset 74 (CLEANUP_THROW)? Do I need to examine the exception table?

My code understands there is a possible branch from line 17 to 18 because at offset 32, it follows the jump to L6 and looks at offsets 42, 44, 46, 48, which gets to line 18. But it doesn't find the branch from 17 to 19. Offset 32 flows to 36, 38, 40, and back to 32, never leaving line 17. Does this question make sense?

@nedbat
Copy link
Member

nedbat commented Mar 2, 2025

More information: the two branch events I get are 44 to 46 (NOT_TAKEN to STORE_FAST) and 32 to 80 (SEND to LOAD_GLOBAL due to an exception). I expected the 44->46 event to start from offset 32, since that's the jump-possible instruction that preceded it. Is the event correct? For 32->80 I guess I need to use the exception table. Are SEND instructions special in this way because they can raise StopIteration? Are there other instructions I would need to treat this way?

@nedbat
Copy link
Member

nedbat commented Mar 2, 2025

Also, I don't see any documented interface to the exception table, though maybe I missed something. If I need to interpret it, what should I do?

@nedbat
Copy link
Member

nedbat commented Mar 2, 2025

Hmm, never mind, I've figured out what to do about these, and I think this is going to work.

@markshannon
Copy link
Member Author

You shouldn't need to understand the bytecode or exception table.
If you do, we are missing something.

In fact, it looks like we are missing the branches from co_branches. I've reopened the issue.

@jaltmayerpizzorno
Copy link

You shouldn't need to understand the bytecode or exception table. If you do, we are missing something.

In fact, it looks like we are missing the branches from co_branches. I've reopened the issue.

We need a way to map branch destinations back to source code... if co_positions is it, there are some cases still where the results don't make sense to me (see the discussion #123050 (comment)).

seehwan pushed a commit to seehwan/cpython that referenced this pull request Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants