GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-135953: Profile a module or script with sampling profiler by lkollar · Pull Request #136777 · python/cpython

Conversation

@lkollar

@lkollar lkollar commented Jul 19, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Add -m and filename arguments to the sampling profiler to launch the specified Python program in a subprocess and start profiling it. Previously only a PID was accepted, this can now be done by passing -p PID.

  • Issue: Expose internal stack introspection APIs as a statistical runtime analysis tool #135953

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot mentioned this pull request Jul 19, 2025
Expose internal stack introspection APIs as a statistical runtime analysis tool #135953
Open
@lkollar
lkollar force-pushed the sample-module branch 2 times, most recently from 5a351da to 1bf048a Compare July 19, 2025 14:07
@lkollar
lkollar marked this pull request as ready for review July 19, 2025 14:59
@bedevere-app bedevere-app Bot added the awaiting review label Jul 19, 2025
@pablogsal pablogsal added skip news 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Jul 29, 2025
@bedevere-bot

bedevere-bot commented Jul 29, 2025

Copy link
Copy Markdown

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

Results will be shown at:

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 29, 2025
@pablogsal
pablogsal requested a review from Copilot July 29, 2025 16:20
Copilot AI reviewed Jul 29, 2025
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the sampling profiler to support profiling modules and scripts by launching them in subprocesses, in addition to the existing PID-based profiling. The change improves the usability of the profiler by allowing users to profile Python programs from startup rather than only attaching to existing processes.

  • Adds -m and script filename arguments as mutually exclusive alternatives to the existing -p PID option
  • Updates argument parsing to handle the new target modes with proper validation
  • Implements subprocess management with graceful termination handling

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
Lib/profile/sample.py Adds argument parsing for module and script modes, implements subprocess launching and management
Lib/test/test_sample_profiler.py Updates existing CLI tests to use -p flag and adds comprehensive test coverage for new module/script functionality
Comments suppressed due to low confidence (1)

Lib/test/test_sample_profiler.py:1637

  • The test uses contextlib.chdir() to change directories for module discovery, but doesn't test the case where the module is not found or the directory change fails. Consider adding a test case for module resolution errors.
            contextlib.chdir(tempdir.name),

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/profile/sample.py Outdated
Comment on lines +736 to +739
if not(args.pid or args.module or args.script):
parser.error(
"You must specify either a process ID (-p), a module (-m), or a script to run."
)

Copilot AI Jul 29, 2025

Copy link

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The condition not(args.pid or args.module or args.script) is redundant since the mutually exclusive group is already marked as required=True. The argparse library will automatically enforce that one of these options is provided.

Suggested change
if not(args.pid or args.module or args.script):
parser.error(
"You must specify either a process ID (-p), a module (-m), or a script to run."
)
# The mutually exclusive group already enforces that one of these arguments is required.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Copilot uses AI. Check for mistakes.
Comment thread Lib/profile/sample.py Outdated
process = subprocess.Popen(cmd)

try:
exit_code = process.wait(timeout=0.1)

Copilot AI Jul 29, 2025

Copy link

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The hardcoded timeout of 0.1 seconds is a magic number. Consider defining this as a named constant or making it configurable, as some programs may have longer startup times.

Suggested change
exit_code = process.wait(timeout=0.1)
exit_code = process.wait(timeout=DEFAULT_PROCESS_WAIT_TIMEOUT)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Copilot uses AI. Check for mistakes.
Comment thread Lib/profile/sample.py
if process.poll() is None:
process.terminate()
try:
process.wait(timeout=2)

This comment was marked as outdated.

Sign in to view

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pablogsal
pablogsal reviewed Jul 29, 2025
View reviewed changes
Comment thread Lib/profile/sample.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
lkollar
lkollar commented Aug 4, 2025
View reviewed changes
Comment thread Lib/profile/sample.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@AA-Turner AA-Turner changed the title [gh-135953] Profile a module or script with sampling profiler gh-135953: Profile a module or script with sampling profiler Aug 4, 2025
AA-Turner
AA-Turner reviewed Aug 4, 2025
View reviewed changes
Comment thread Lib/profile/sample.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/profile/sample.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/profile/sample.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/profile/sample.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/profile/sample.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/profile/sample.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@lkollar

lkollar commented Aug 5, 2025

Copy link
Copy Markdown
Contributor Author

Thanks @AA-Turner, I think I addressed all your comments.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lkollar
lkollar force-pushed the sample-module branch from 19cde40 to 3c53904 Compare August 5, 2025 23:50
pablogsal
pablogsal reviewed Aug 5, 2025
View reviewed changes
Comment thread Python/remote_debug.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
lkollar added 11 commits August 6, 2025 14:26
@lkollar
Profile a module or script with sampling profiler
eada5f0 
Add `-m` and `filename` arguments to the sampling profiler to launch the
specified Python program in a subprocess and start profiling it.
Previously only a PID was accepted, this can now be done by passing
`-p PID`.
@lkollar
Wait for interpreter to initialize in subprocess
d8d035e
@lkollar
Remove unnecessary argument check
d99a39e 
These args are already mutually exclusive, but we need to check if at
least on module argument has been passed.
@lkollar
Handle the process runs shorter than expected
f97cc55 
In this case the subprocess will go into zombie state until we can poll
it. We can simply assume this is the case if it's still detected as
running when we get a ValueError.
@lkollar
Improve ReadRemoteMemory error handling on macOS
18c4002 
Improve the return value check to be able to raise a ProcessLookupError
when the remote process is not available.

Mach uses composite error values where higher error values indicate
specific subsystems. We can use the err_get_code function to mask the
higher bits to make our error checking more robust in case the subsystem
bits are set. For example, in some situations if the process is in zombie
state, we can get KERN_NO_SPACE (0x3) but the actual return value is
0x10000003 which indicates a specific subsystem, thus we need to
use err_get_code to extract the error value.

This also improves how KERN_INVALID_ARGUMENT is handled to check whether
we got a generic invalid argument error, or if the process is no longer
accessible.
@lkollar
fixup! Handle the process runs shorter than expected
980c0f2
@lkollar
fixup! Wait for interpreter to initialize in subprocess
5043171
@lkollar
fixup! fixup! Wait for interpreter to initialize in subprocess
7832a7d
@lkollar
fixup! Improve ReadRemoteMemory error handling on macOS
c13a74d
@lkollar
fixup! fixup! Wait for interpreter to initialize in subprocess
2ce8a96
@lkollar
fixup! fixup! Improve ReadRemoteMemory error handling on macOS
1622ee3
@lkollar
lkollar force-pushed the sample-module branch from 3c53904 to 1622ee3 Compare August 6, 2025 17:26
@pablogsal
pablogsal requested review from 1st1 and ambv as code owners August 8, 2025 12:34
@pablogsal
pablogsal force-pushed the sample-module branch 3 times, most recently from 758af04 to 289d868 Compare August 8, 2025 13:31
@pablogsal
Do not return error without an exception set
0338ee1
@pablogsal
pablogsal force-pushed the sample-module branch from 289d868 to 0338ee1 Compare August 8, 2025 17:22
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 8, 2025
@bedevere-bot

bedevere-bot commented Aug 8, 2025

Copy link
Copy Markdown

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

Results will be shown at:

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 8, 2025
@pablogsal

pablogsal commented Aug 9, 2025

Copy link
Copy Markdown
Member

@lkollar Seems test_sample_target_module has a race since is failin on some buildbots:

AssertionError: 'slow_fibonacci' not found in 'Captured 10001 samples in 1.00 seconds
Sample rate: 10000.94 samples/sec
Error rate: 99.10%
Profile Stats:
       nsamples   sample%  tottime (ms)    cumul%  cumtime (ms)  filename:lineno(function)
          90/90     100.0         9.000     100.0         9.000  sample.py:96(SampleProfiler.sample)
           0/90       0.0         0.000     100.0         9.000  sample.py:549(sample)
           0/90       0.0         0.000     100.0         9.000  sample.py:590(wait_for_process_and_sample)
           0/90       0.0         0.000     100.0         9.000  sample.py:790(main)
           0/90       0.0         0.000     100.0         9.000  test_sample_profiler.py:1640(TestSampleProfilerIntegration.test_sample_target_module)
           0/90       0.0         0.000     100.0         9.000  case.py:613(TestCase._callTestMethod)
           0/90       0.0         0.000     100.0         9.000  case.py:667(TestCase.run)

Legend:
  nsamples: Direct/Cumulative samples (direct executing / on call stack)
  sample%: Percentage of total samples this function was directly executing
  tottime: Estimated total time spent directly in this function
  cumul%: Percentage of total samples when this function was on the call stack
  cumtime: Estimated cumulative time (including time in called functions)
  filename:lineno(function): Function location and name

Summary of Interesting Functions:

Functions with Highest Direct/Cumulative Ratio (Hot Spots):
  1.000 direct/cumulative ratio, 100.0% direct samples: sample.py:(SampleProfiler.sample)

Functions with Highest Call Frequency (Indirect Calls):
  90 indirect calls, 100.0% total stack presence: sample.py:(sample)
  90 indirect calls, 100.0% total stack presence: sample.py:(wait_for_process_and_sample)
  90 indirect calls, 100.0% total stack presence: sample.py:(main)

Functions with Highest Call Magnification (Cumulative/Direct):
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal

pablogsal commented Aug 9, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

why is the sample.py code in the results? Also somehow we have Error rate: 99.10%????
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal

pablogsal commented Aug 9, 2025

Copy link
Copy Markdown
Member

Oh, I think this is because we are sampling before the other process calls execv so we see the forked one! We may need some sync mechanism between the process and the profilee.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pablogsal
Protect against more errors with no exceptions
2576f8b
@pablogsal
pablogsal force-pushed the sample-module branch from a7fa762 to 2576f8b Compare August 9, 2025 17:40
@pablogsal
Attempt to sync processes
3847fff
@pablogsal
pablogsal force-pushed the sample-module branch from e40aa09 to 3847fff Compare August 9, 2025 17:53
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 9, 2025
@bedevere-bot

bedevere-bot commented Aug 9, 2025

Copy link
Copy Markdown

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

Results will be shown at:

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Aug 9, 2025
pablogsal
pablogsal approved these changes Aug 9, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Aug 9, 2025
@pablogsal
pablogsal merged commit 4497ad4 into python:main Aug 11, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Aug 11, 2025
maurycy added a commit to maurycy/cpython that referenced this pull request Aug 12, 2025
@maurycy
Merge branch 'main' into csv-sniffer-counter-set
11c891d 
* main:
  pythongh-137288: Update 3.14 magic numbers (pythonGH-137665)
  pythongh-135228: When @DataClass(slots=True) replaces a dataclass, make the original class collectible (take 2) (pythonGH-137047)
  pythongh-126008: Improve docstrings for Tkinter cget and configure methods (pythonGH-133303)
  pythongh-131885: Use positional-only markers for ``max()`` and ``min()`` (python#131868)
  pythonGH-137426: Remove code deprecation of `importlib.abc.ResourceLoader` (pythonGH-137567)
  pythongh-125897: Mark range function parameters as positional only (python#125945)
  pythongh-137400: Fix a crash when disabling profiling across all threads (pythongh-137471)
  pythongh-115766: Fix IPv4Interface.is_unspecified (pythonGH-137326)
  pythongh-128813: cleanup C-API docs for PyComplexObject (pythonGH-137579)
  pythongh-135953: Profile a module or script with sampling profiler (python#136777)
  Fix documentation of hash in PyHash_FuncDef (python#137595)
@lkollar
lkollar deleted the sample-module branch August 13, 2025 18:21
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@lkollar @Agent-Hellboy
pythongh-135953: Profile a module or script with sampling profiler (p…
5e3eba3 
…ython#136777)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AA-Turner AA-Turner AA-Turner left review comments
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@pablogsal pablogsal pablogsal approved these changes
@ambv ambv Awaiting requested review from ambv
@1st1 1st1 Awaiting requested review from 1st1

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@lkollar @bedevere-bot @pablogsal @AA-Turner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.