gh-138122: Add blocking mode for accurate stack traces in Tachyon #142998
+852
−78
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pablogsal
changed the title
pablogsal
force-pushed
the
fixes
branch
4 times, most recently
from
bb510c0 to
8c21dc9
Compare
Member
Author
|
@ivonastojanovic can you take a look? |
pablogsal
force-pushed
the
fixes
branch
4 times, most recently
from
de4448b to
c270764
Compare
pablogsal
requested review from
AA-Turner,
ezio-melotti and
hugovk
as code owners
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 <[email protected]>
pablogsal
commented
Dec 20, 2025
|
|
||
| // 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). |
Member
Author
There was a problem hiding this comment.
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.
Contributor
ivonastojanovic
left a comment
ivonastojanovic
left a comment
There was a problem hiding this comment.
LGTM! Just a couple of nits
| collector.collect_failed_sample() | ||
| errors += 1 | ||
| except Exception as e: | ||
| print(e) |
Contributor
There was a problem hiding this comment.
Suggested change
| print(e) |
| duration_sec = current_time - start_time | ||
| break | ||
| except (RuntimeError, UnicodeDecodeError, MemoryError, OSError): | ||
| except (RuntimeError, UnicodeDecodeError, MemoryError, OSError) as e: |
Contributor
There was a problem hiding this comment.
Suggested change
| except (RuntimeError, UnicodeDecodeError, MemoryError, OSError) as e: | |
| except (RuntimeError, UnicodeDecodeError, MemoryError, OSError): |
| # When consume_generator is on the arithmetic lines (temp1, temp2, etc.), | ||
| # fibonacci_generator should NOT be in the stack at all. | ||
| # Line numbers are important here - see ARITHMETIC_LINES below. | ||
| cls.generator_script = ''' |
Contributor
There was a problem hiding this comment.
nit: Maybe we could use textwrap.dedent here to keep the string nicely formatted. What do you think?
|
|
||
| .. option:: --blocking | ||
|
|
||
| Stop the target process during each sample. This ensures consistent |
Contributor
There was a problem hiding this comment.
Not sure if pause might sound better than stop here
Suggested change
| Stop the target process during each sample. This ensures consistent | |
| Pause the target process during each sample. This ensures consistent |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
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/