Add BRANCH_TAKEN and BRANCH_NOT_TAKEN events to sys.monitoring · Issue #122548 · python/cpython · GitHub

Feature or enhancement

Proposal:

Add BRANCH_TAKEN and BRANCH_NOT_TAKEN events to sys.monitoring
Deprecate the old BRANCH event (as it will be redundant)
Monitoring BRANCH events implicitly monitors both the BRANCH_TAKEN and BRANCH_NOT_TAKEN events
Disabling a BRANCH_TAKEN event will implicitly disable the matching BRANCH event, but only for the taken branch
Disabling a BRANCH_NOT_TAKEN event will implicitly disable the matching BRANCH event, but only for the not-taken branch
Disabling a BRANCH event will implicitly disable the both the matching BRANCH_TAKEN and BRANCH_NOT_TAKEN events

This is fully backwards compatible.

Has this already been discussed elsewhere?

I have already discussed this feature proposal on Discourse

Links to previous discussion of this feature:

Discussion which also links to prior discussions.

Linked PRs

### Tasks
- [ ] https://github.com/python/cpython/issues/123044
- [ ] https://github.com/python/cpython/issues/123048
- [ ] https://github.com/python/cpython/issues/123050
- [ ] https://github.com/python/cpython/issues/128375
- [ ] https://github.com/python/cpython/issues/128533
- [ ] https://github.com/python/cpython/issues/128534

The interactions between BRANCH events and the BRANCH_TAKEN and BRANCH_NOT_TAKEN pair of events is going to be awkward to implement and unnecessarily inefficient, given there is no use case for combining the old and new approach.

So, here's the updated proposal:

Add BRANCH_TAKEN and BRANCH_NOT_TAKEN events to sys.monitoring
Deprecate the old BRANCH event (as it will be redundant)
Tools should use either the old BRANCH event, or the new BRANCH_TAKEN and BRANCH_NOT_TAKEN pair. Not both.
The behavior of BRANCH, BRANCH_TAKEN and BRANCH_NOT_TAKEN events is not defined if both are used.

This is still fully backwards compatible, and will allow us to implement BRANCH events on top of the new events without negatively impacting their performance.

If I try to just use the old BRANCH event, I get an error:

  File "/Users/juan/project/slipcover/pep669.py", line 92, in enable_events
    sys.monitoring.set_local_events(sys.monitoring.COVERAGE_ID, co,
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                    sys.monitoring.events.LINE|
                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
                                    sys.monitoring.events.BRANCH)
                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: invalid local event set 0x40020

Fixed.
In future, could you report issues with PRs on the PR? It gets hard to track them otherwise.
Thanks.

Fixed. In future, could you report issues with PRs on the PR? It gets hard to track them otherwise. Thanks.

Ah, sorry about that! Ironically, that's what I intended by reporting it here -- but it was the code in #122564 that I saw that on.

Since monitoring shouldn't impact performance when not used, we don't want to restrict the bytecode compiler in its optimizations, or how it lays out code.

With that in mind, I think we need to drop the "taken"/"not taken" labels and use something like "left" and "right".
That way there is less of an implication that a particular branch direction corresponds to a particular source construct.

In the code:

if cond:
     x
y

It could be surprising if the branch from cond to x is marked "taken" rather than "not taken", but less surprising if it is marked "right" instead of "left".

I therefore proposed renaming BRANCH_NOT_TAKEN to BRANCH_LEFT and BRANCH_TAKEN to BRANCH_RIGHT.
The triples in co_branches will then be origin, left, right.

I therefore proposed renaming BRANCH_NOT_TAKEN to BRANCH_LEFT and BRANCH_TAKEN to BRANCH_RIGHT. The triples in co_branches will then be origin, left, right.

Ok! I've modified my code, so as far as I'm concerned, you can remove the taken/not_taken names.

GH-122548: Implement branch taken and not taken events for sys.monito…

GH-122548: Correct magic number comment (GH-128115)

Continuing from #122564 (comment) and #122564 (comment)

There is no branch from line 1 to 2, the "left" branch is from line 1 to line 1.

Maybe first we should decide: from the user's point of view, does line 1 have two possible destinations (line 2 and line 3) or does it not? Should line 1 be considered a branch for the purposes of branch coverage?

If we can't map the collected data back to a user-understandable interpretation, then we haven't finished the feature. Can you help me puzzle through this? I'd like to get efficient branch coverage measurement. So far, we seem to be talking about different things (byte codes vs lines) and we aren't meeting in the middle.

pythonGH-122548: Implement branch taken and not taken events for sys.…

pythonGH-122548: Correct magic number comment (pythonGH-128115)

If we can't map the collected data back to a user-understandable interpretation...

See #123050 (comment)

(Moved here from #128445 (comment))

Two more examples of branch event surprises. run_sysmon.py is used to see the sys.monitoring events. These are using commit cbe6c7c from PR #128445.

ifdebug.py

for value in [True, False]:
    if value:
        if __debug__:
            x = 4
    else:
        x = 6

Disassembled:

% python3 -m dis -O ifdebug.py
  0          0       RESUME                   0

  1          2       LOAD_CONST               0 ((True, False))
             4       GET_ITER
      L1:    6       FOR_ITER                18 (to L3)
            10       STORE_NAME               0 (value)

  2         12       LOAD_NAME                0 (value)
            14       TO_BOOL
            22       POP_JUMP_IF_FALSE        6 (to L2)
            26       NOT_TAKEN

  3         28       NOT_TAKEN

  4         30       LOAD_SMALL_INT           4
            32       STORE_NAME               1 (x)
            34       JUMP_BACKWARD           16 (to L1)

  6   L2:   38       LOAD_SMALL_INT           6
            40       STORE_NAME               1 (x)
            42       JUMP_BACKWARD           20 (to L1)

  1   L3:   46       END_FOR
            48       POP_ITER
            50       LOAD_CONST               1 (None)
            52       RETURN_VALUE

Events:

3.14.0a3+ (heads/pr/128445:cbe6c7c085c, Jan  4 2025, 05:39:38) [Clang 16.0.0 (clang-1600.0.26.6)]
PY_START: ifdebug.py@0 #0
LINE: ifdebug.py #1
BRANCH_LEFT: ifdebug.py@6->10 #1->1
LINE: ifdebug.py #2
BRANCH_LEFT: ifdebug.py@22->28 #2->3
LINE: ifdebug.py #3
BRANCH_LEFT: ifdebug.py@28->30 #3->4        ****
LINE: ifdebug.py #4
JUMP: ifdebug.py@34->6 #4->1
LINE: ifdebug.py #1
BRANCH_RIGHT: ifdebug.py@22->38 #2->6
LINE: ifdebug.py #6
JUMP: ifdebug.py@42->6 #6->1
BRANCH_RIGHT: ifdebug.py@6->50 #1->1
PY_RETURN: ifdebug.py@52 #1

The starred line is a BRANCH event from a NOT_TAKEN instruction.

1176_async.py

import asyncio

async def async_gen():
    yield 4

async def async_test():
    async for i in async_gen():
        print(i + 8)

asyncio.run(async_test())

Disassembled:

% python3 -m dis -O 1176_async.py
  0          0       RESUME                   0

  1          2       LOAD_SMALL_INT           0
             4       LOAD_CONST               0 (None)
             6       IMPORT_NAME              0 (asyncio)
             8       STORE_NAME               0 (asyncio)

  3         10       LOAD_CONST               1 (<code object async_gen at 0x103781c50, file "1176_async.py", line 3>)
            12       MAKE_FUNCTION
            14       STORE_NAME               1 (async_gen)

  6         16       LOAD_CONST               2 (<code object async_test at 0x1037c5b80, file "1176_async.py", line 6>)
            18       MAKE_FUNCTION
            20       STORE_NAME               2 (async_test)

 10         22       LOAD_NAME                0 (asyncio)
            24       LOAD_ATTR                6 (run)
            44       PUSH_NULL
            46       LOAD_NAME                2 (async_test)
            48       PUSH_NULL
            50       CALL                     0
            58       CALL                     1
            66       POP_TOP
            68       LOAD_CONST               0 (None)
            70       RETURN_VALUE

Disassembly of <code object async_gen at 0x103781c50, file "1176_async.py", line 3>:
   3          0       RETURN_GENERATOR
              2       POP_TOP
       L1:    4       RESUME                   0

   4          6       LOAD_SMALL_INT           4
              8       CALL_INTRINSIC_1         4 (INTRINSIC_ASYNC_GEN_WRAP)
             10       YIELD_VALUE              0
             12       RESUME                   5
             14       POP_TOP
             16       LOAD_CONST               0 (None)
             18       RETURN_VALUE

  --   L2:   20       CALL_INTRINSIC_1         3 (INTRINSIC_STOPITERATION_ERROR)
             22       RERAISE                  1
ExceptionTable:
  L1 to L2 -> L2 [0] lasti

Disassembly of <code object async_test at 0x1037c5b80, file "1176_async.py", line 6>:
   6           0       RETURN_GENERATOR
               2       POP_TOP
        L1:    4       RESUME                   0

   7           6       LOAD_GLOBAL              1 (async_gen + NULL)
              16       CALL                     0
              24       GET_AITER
        L2:   26       GET_ANEXT
              28       LOAD_CONST               0 (None)
        L3:   30       SEND                     3 (to L6)
        L4:   34       YIELD_VALUE              1
        L5:   36       RESUME                   3
              38       JUMP_BACKWARD_NO_INTERRUPT 5 (to L3)
        L6:   40       END_SEND
        L7:   42       STORE_FAST               0 (i)

   8          44       LOAD_GLOBAL              3 (print + NULL)
              54       LOAD_FAST                0 (i)
              56       LOAD_SMALL_INT           8
              58       BINARY_OP                0 (+)
              62       CALL                     1
              70       POP_TOP
              72       JUMP_BACKWARD           25 (to L2)

   7    L8:   76       CLEANUP_THROW
        L9:   78       JUMP_BACKWARD_NO_INTERRUPT 20 (to L6)
       L10:   80       END_ASYNC_FOR
              82       LOAD_CONST               0 (None)
              84       RETURN_VALUE

  --   L11:   86       CALL_INTRINSIC_1         3 (INTRINSIC_STOPITERATION_ERROR)
              88       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

Events:

3.14.0a3+ (heads/pr/128445:cbe6c7c085c, Jan  4 2025, 05:39:38) [Clang 16.0.0 (clang-1600.0.26.6)]
PY_START: 1176_async.py@0 #0
LINE: 1176_async.py #1
LINE: 1176_async.py #3
LINE: 1176_async.py #6
LINE: 1176_async.py #10
PY_START: 1176_async.py@4 #6
LINE: 1176_async.py #7
PY_START: 1176_async.py@4 #3
LINE: 1176_async.py #4
LINE: 1176_async.py #8
12
JUMP: 1176_async.py@72->26 #8->7
LINE: 1176_async.py #7
PY_RESUME: 1176_async.py@12 #4
PY_RETURN: 1176_async.py@18 #4
PY_RETURN: 1176_async.py@84 #7
PY_RETURN: 1176_async.py@70 #10

Here there are no BRANCH events at all. I would expect some for the async for line, analogous to how synchronous for is evented.

pythonGH-122548: Implement branch taken and not taken events for sys.…

pythonGH-122548: Correct magic number comment (pythonGH-128115)

@markshannon Any word on the if __debug__ and async problems I mentioned above? Should they be new issues?

if __debug__ was fixed by #128533, I believe. Please let us know if you are still seeing issues.
The async for issue is tracked in #128534.

Both issues are listed in the "tasks" section above.

Sorry, I am getting confused. I know I said "#3->4" was wrong, but now I'm thinking I need to see that branch reported. Shouldn't PEP626 require that it be reported?

PEP 626 says nothing about branches, just line events.
Since __debug__ is a constant, like True or False, there is no branch as far as the interpreter is concerned.
Without -O, if __debug__: is treated the same as if True:

Thanks, I see what you mean about the __debug__ constant being optimized away. I'm trying to figure out how to accommodate that. Coverage.py has a rudimentary understanding of this kind of optimization. For example, it knows that while True: isn't a branch, but thinks that while not False: is a branch. I'm not sure how to mirror Python's optimizations effectively...

I think the only thing remaining before coverage.py can do efficient branch coverage is the async issue. When should I expect an update?

#130569 adds branch monitoring for async for loops.

@nedbat I think that's everything done.

Is there anything else you think is needed, either code or documentation?

I think we are set on the code. Not sure about the docs, TBH.

3.14 has been released with this feature

Add BRANCH_TAKEN and BRANCH_NOT_TAKEN events to sys.monitoring · Issue #122548 · python/cpython · GitHub

Repository navigation

Add BRANCH_TAKEN and BRANCH_NOT_TAKEN events to sys.monitoring #122548

Feature or enhancement

Proposal:

Has this already been discussed elsewhere?

Links to previous discussion of this feature:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Add BRANCH_TAKEN and BRANCH_NOT_TAKEN events to sys.monitoring · Issue #122548 · python/cpython · GitHub

Repository navigation

Add BRANCH_TAKEN and BRANCH_NOT_TAKEN events to sys.monitoring #122548

Description

Feature or enhancement

Proposal:

Has this already been discussed elsewhere?

Links to previous discussion of this feature:

Linked PRs

Activity

markshannon commented on Aug 1, 2024

jaltmayerpizzorno commented on Aug 14, 2024

markshannon commented on Aug 15, 2024

jaltmayerpizzorno commented on Aug 15, 2024

markshannon commented on Aug 16, 2024

jaltmayerpizzorno commented on Aug 16, 2024

nedbat commented on Dec 22, 2024

markshannon commented on Dec 31, 2024

nedbat commented on Jan 6, 2025

nedbat commented on Feb 1, 2025

markshannon commented on Feb 3, 2025

nedbat commented on Feb 4, 2025

markshannon commented on Feb 6, 2025

nedbat commented on Feb 7, 2025

nedbat commented on Feb 20, 2025

markshannon commented on Feb 26, 2025

markshannon commented on Feb 27, 2025

nedbat commented on Mar 3, 2025

markshannon commented on Jan 5, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions