benfred / py-spy

Sampling profiler for Python programs
MIT License
12.13k stars 401 forks source link

Python 3.11 --native profiling fails because of bpo-45256 #634

Closed krfricke closed 7 months ago

krfricke commented 7 months ago

bpo-45256 removed the 1-to-1 mapping between the CPython stack frames (_PyEval_EvalFrame) and Python stack frames.

This breaks the --native extension for Python 3.11. In particular, even the simplest call stacks can't be merged anymore:

import time
import os

def wait(sleep_time):
    time.sleep(sleep_time)

print("PID", os.getpid())
wait(30)
Process 624584: python wait.py
Python v3.11.5 (...)

Thread 624584 (idle)
    wait (wait.py:5)
    <module> (wait.py:8)

> py-spy dump --pid 624584 -n
Process 624584: python wait.py
Python v3.11.5 (...)

Error: Failed to merge native and python frames (Have 1 native and 2 python)

In previous Python versions (until 3.10.x), each line in the python stack trace corresponded to one _PyEval_EvalFrame (and _PyEval_EvalFrameDefault) frame in CPython, but this is not the case anymore. For a program like the one above, there is only one _PyEval_EvalFrame.

Unfortunately, it doesn't seem straightforward to fix this. In the simplest cases (like here), where there's only 1 native frame, we could probably just insert the full python stack trace at that location. However, in more complicated situations (e.g. when calling a native extension, which then executed python code), this wouldn't suffice.

The challenge is to identify which groups of python frames belong to which _PyEval_EvalFrame. I've digged a bit into this, but couldn't come up with a good way so far.

Note that this issue has been raised previously, e.g. in https://github.com/benfred/py-spy/issues/617, https://github.com/benfred/py-spy/issues/608, https://github.com/benfred/py-spy/issues/558. I'm opening this issue to consolidate these with some relevant background.

I'm happy to contribute, but my knowledge of CPython internals is limited, so a few pointers would be appreciated.

trim21 commented 2 months ago

looks like this PR is not released yet?