FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-128534: Instrument branches for `async for` loops. by markshannon · Pull Request #130569 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

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

Merged
markshannon merged 3 commits into
python:mainfrom
faster-cpython:instrument-branches-async-for
Feb 27, 2025
ConversationCommits (3)ChecksFiles changed
Merged

GH-128534: Instrument branches for async for loops.#130569
markshannon merged 3 commits into
python:mainfrom
faster-cpython:instrument-branches-async-for

Conversation

markshannon commented Feb 26, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

bedevere-app Bot mentioned this pull request Feb 26, 2025
Closed
markshannon mentioned this pull request Feb 26, 2025
Closed
markshannon merged commit 2a18e80 into python:main Feb 27, 2025
markshannon deleted the instrument-branches-async-for branch February 27, 2025 11:07

nedbat commented Mar 1, 2025

Copy link
Copy Markdown
Member

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 commented Mar 2, 2025

Copy link
Copy Markdown
Member

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 commented Mar 2, 2025

Copy link
Copy Markdown
Member

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 commented Mar 2, 2025

Copy link
Copy Markdown
Member

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

markshannon commented Mar 3, 2025

Copy link
Copy Markdown
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 commented Mar 3, 2025

Copy link
Copy Markdown

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)).

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

iritkatriel iritkatriel approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL