Skip to content

gh-138122: Add blocking mode for accurate stack traces in Tachyon - #142998

Merged
pablogsal merged 6 commits into
python:mainfrom
pablogsal:fixes
Dec 23, 2025
Merged

gh-138122: Add blocking mode for accurate stack traces in Tachyon#142998
pablogsal merged 6 commits into
python:mainfrom
pablogsal:fixes

Conversation

@pablogsal

@pablogsalpablogsal commented Dec 20, 2025

Copy link
Copy Markdown
Member

Non-blocking sampling reads process memory while the target continues
running, which can produce torn stacks when generators or coroutines
rapidly switch between yield points. Blocking mode uses atomic process
suspension (task_suspend on macOS, NtSuspendProcess on Windows,
PTRACE_SEIZE on Linux) to stop the target during each sample, ensuring
consistent snapshots.

Use blocking mode with longer intervals (1ms+) to avoid impacting the
target too much. The default non-blocking mode remains best for most
cases since it has zero overhead.

📚 Documentation preview 📚: https://cpython-previews--142998.org.readthedocs.build/

@pablogsalpablogsal changed the title gh-142654: Add blocking mode for accurate stack traces in Tachyongh-138122: Add blocking mode for accurate stack traces in TachyonDec 20, 2025
@pablogsal
pablogsalforce-pushed the fixes branch 4 times, most recently from bb510c0 to 8c21dc9CompareDecember 20, 2025 02:57
@pablogsal

Copy link
Copy Markdown
MemberAuthor

@ivonastojanovic can you take a look?

Non-blocking sampling reads process memory while the target continues
running, which can produce torn stacks when generators or coroutines
rapidly switch between yield points. Blocking mode uses atomic process
suspension (task_suspend on macOS, NtSuspendProcess on Windows,
PTRACE_SEIZE on Linux) to stop the target during each sample, ensuring
consistent snapshots.
Use blocking mode with longer intervals (1ms+) to avoid impacting the
target too much. The default non-blocking mode remains best for most
cases since it has zero overhead.
Also fix a frame cache bug: the cache was including the last_profiled_frame
itself when extending with cached data, but this frame was executing in
the previous sample and its line number may have changed. For example,
if function A was sampled at line 6, then execution continued to line 10
and called B→C, the next sample would incorrectly report A at line 6
(from cache) instead of line 10. The fix uses start_idx + 1 to only trust
frames ABOVE last_profiled_frame — these caller frames are frozen at their
call sites and cannot change until their callees return.
Signed-off-by: Pablo Galindo <pablogsal@gmail.com>

// Extend frame_info with frames from start_idx onwards
PyObject *slice = PyList_GetSlice(entry->frame_list, start_idx, num_frames);
// Extend frame_info with frames ABOVE start_idx (not including it).

@pablogsalpablogsalDec 20, 2025

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This fixes a frame cache bug: the cache was including the last_profiled_frame
itself when extending with cached data, but this frame was executing in
the previous sample and its line number may have changed. For example,
if function A was sampled at line 6, then execution continued to line 10
and called B→C, the next sample would incorrectly report A at line 6
(from cache) instead of line 10. The fix uses start_idx + 1 to only trust
frames ABOVE last_profiled_frame: these caller frames are frozen at their
call sites and cannot change until their callees return.

@ivonastojanovicivonastojanovic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM! Just a couple of nits

Comment threadLib/profiling/sampling/sample.py Outdated
Comment threadLib/profiling/sampling/sample.py Outdated
Comment threadLib/test/test_profiling/test_sampling_profiler/test_blocking.py Outdated
Comment threadDoc/library/profiling.sampling.rst Outdated
pablogsaland others added 3 commits December 21, 2025 16:06
Co-authored-by: ivonastojanovic <80911834+ivonastojanovic@users.noreply.github.com>
@pablogsalpablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 23, 2025
@bedevere-bot

Copy link
Copy Markdown

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

Results will be shown at:

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

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

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 23, 2025
@pablogsal
pablogsal merged commit 81c8eb8 into python:mainDec 23, 2025
46 checks passed
@KowalskiThomas

Copy link
Copy Markdown
Contributor

👋 Hello!

Seems like test_profiling which was added as part of this PR has been failing quite a lot / quite often on Windows on main and feature branches on top of from main lately (one example from a branch of mine, but on main here, here, here and here from recent executions).

Is that something you're aware of / looking into already?

@hugovk

Copy link
Copy Markdown
Member

Hello! Yes, there's an issue (#150429) and PR (#150433).

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@pablogsal@bedevere-bot@KowalskiThomas@hugovk@ivonastojanovic