Skip to content

gh-135953: Implement sampling tool under profile.sample #135998

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

Open
wants to merge 40 commits into
base: main
Choose a base branch
from

Conversation

lkollar
Copy link
Contributor

@lkollar lkollar commented Jun 26, 2025

No description provided.

@lkollar lkollar changed the title Implement sampling tool under profile.sample gh-135953: Implement sampling tool under profile.sample Jun 26, 2025
@lkollar lkollar force-pushed the sampling-profiler branch 4 times, most recently from 0828aa3 to 57e3152 Compare June 27, 2025 20:09
@lkollar lkollar changed the title gh-135953: Implement sampling tool under profile.sample gh-135953: Implement sampling tool under profile.sample Jul 3, 2025
lkollar added 5 commits July 3, 2025 21:35
This allows adding a new 'sample' submodule and enables invoking the sampling profiler through 'python -m profile.sample', while retaining backwards compatibility when using 'profile'.
Implement a statistical sampling profiler that can profile external Python processes by PID. Uses the _remote_debugging module and converts the results to pstats-compatible format for analysis.
This variant overrides how column headers are printed to avoid conflating call counts with sample counts. The SampledStats results are stored in the exact same format as Stats, but since the results don't represent call counts but sample counts, the column headers are different to account for this. This ensure that using the pstats browser instantiates the right object to handle the correct columns, add a factory function which can instantiate the correct class. As the Stats class can only handle either a filename or an object which provides the 'stats' attribute in a pre-parsed format, this provides a StatsLoaderShim to avoid marshalling the data twice (once to check the marker and once in the Stats module if we were to pass the file name).
Implements collapsed stack trace format output for the sampling profiler. This format represents complete call stacks as semicolon- delimited strings with sample counts, making it compatible with external flamegraph generation tools like flamegraph.pl. The format uses filename:function:line notation and stores call trees during sampling for efficient post-processing into the collapsed format.
@lkollar lkollar force-pushed the sampling-profiler branch 2 times, most recently from a019e6a to a0be753 Compare July 3, 2025 21:09
@lkollar lkollar force-pushed the sampling-profiler branch from a0be753 to aeca768 Compare July 3, 2025 21:58
@pablogsal pablogsal requested a review from ambv July 6, 2025 17:38
@pablogsal pablogsal self-assigned this Jul 6, 2025
@@ -47,8 +49,14 @@ def sample(self, collector, duration_sec=10):
try:
stack_frames = self.unwinder.get_stack_trace()
collector.collect(stack_frames)
except (RuntimeError, UnicodeDecodeError, OSError):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong: the profiler can and will fail naturally with OSError

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to modify the windows part of remote debugging in dbe2c0a to not raise a generic OSError so we can catch it properly

@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 260d934 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135998%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@pablogsal
Copy link
Member

pablogsal commented Jul 10, 2025

@lkollar seems this fails on the buildbot with some weird errors:

test test_cprofile crashed -- Traceback (most recent call last): File "/home/buildbot/buildarea/pull_request.cstratak-fedora-stable-aarch64.clang-installed/build/target/lib/python3.15/test/libregrtest/single.py", line 210, in _runtest_env_changed_exc _load_run_test(result, runtests) ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^ File "/home/buildbot/buildarea/pull_request.cstratak-fedora-stable-aarch64.clang-installed/build/target/lib/python3.15/test/libregrtest/single.py", line 155, in _load_run_test test_mod = importlib.import_module(module_name) File "/home/buildbot/buildarea/pull_request.cstratak-fedora-stable-aarch64.clang-installed/build/target/lib/python3.15/importlib/__init__.py", line 88, in import_module return _bootstrap._gcd_import(name[level:], package, level) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "<frozen importlib._bootstrap>", line 1398, in _gcd_import File "<frozen importlib._bootstrap>", line 1371, in _find_and_load File "<frozen importlib._bootstrap>", line 1342, in _find_and_load_unlocked File "<frozen importlib._bootstrap>", line 938, in _load_unlocked File "<frozen importlib._bootstrap_external>", line 762, in exec_module File "<frozen importlib._bootstrap>", line 491, in _call_with_frames_removed File "/home/buildbot/buildarea/pull_request.cstratak-fedora-stable-aarch64.clang-installed/build/target/lib/python3.15/test/test_cprofile.py", line 7, in <module> import cProfile File "/home/buildbot/buildarea/pull_request.cstratak-fedora-stable-aarch64.clang-installed/build/target/lib/python3.15/cProfile.py", line 11, in <module> import profile as _pyprofileModuleNotFoundError: No module named 'profile' 

I think we are missing the subdir in

LIBSUBDIRS= asyncio \

@pablogsal
Copy link
Member

pablogsal commented Jul 10, 2025

Seems we are not protecting also against FreeBSD: https://buildbot.python.org/#/builders/1228/builds/718/steps/6/logs/stdio

We need to:

  • Skip the tests if FreeBSD as we do in test_external_inspection.
  • Properly raise in the C code when creating a stack sampler object instead of reaching the unreachable code
@lkollar lkollar requested a review from erlend-aasland as a code owner July 10, 2025 08:28
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit a33d166 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135998%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 0235127 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135998%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 90260a6 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135998%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 5683b76 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F135998%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants