Skip to content

GH-146527: Add get_gc_stats function to _remote_debugging#148071

Open
sergey-miryanov wants to merge 21 commits intopython:mainfrom
sergey-miryanov:feat/gc-monitor-get
Open

GH-146527: Add get_gc_stats function to _remote_debugging#148071
sergey-miryanov wants to merge 21 commits intopython:mainfrom
sergey-miryanov:feat/gc-monitor-get

Conversation

@sergey-miryanov
Copy link
Copy Markdown
Contributor

@sergey-miryanov sergey-miryanov commented Apr 4, 2026

This PR implements second part of the issue. Instead of adding a new module or extending the gc module we add new function to the _remote_debugging module as it is a best place for this.

proc_handle_t handle;
uintptr_t runtime_start_address;
struct _Py_DebugOffsets debug_offsets;
} RuntimeOffsets;
Copy link
Copy Markdown
Contributor Author

@sergey-miryanov sergey-miryanov Apr 4, 2026

Choose a reason for hiding this comment

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

I'm not sure it is a best name, but I'm out of ideas a bit.

@sergey-miryanov
Copy link
Copy Markdown
Contributor Author

I'm not sure about news entry too.

@sergey-miryanov sergey-miryanov marked this pull request as ready for review April 12, 2026 12:19
@sergey-miryanov
Copy link
Copy Markdown
Contributor Author

@pablogsal Could you please take a look?

Comment thread Modules/_remote_debugging/gc_stats.h
Comment thread Modules/_remote_debugging/interpreters.c Outdated
@sergey-miryanov sergey-miryanov marked this pull request as draft April 16, 2026 13:56
sergey-miryanov and others added 3 commits April 16, 2026 18:57
Co-authored-by: Shamil <ashm.tech@proton.me>
@sergey-miryanov sergey-miryanov marked this pull request as ready for review April 16, 2026 14:18
[clinic start generated code]*/

static PyObject *
_remote_debugging_get_gc_stats_impl(PyObject *module, int pid,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For something that wants to keep getting the stats from a remote process this constanly initializes the proc handle and the runtime address. I think we can make it a method of the remote unwinder object and that can allow us to provide something that allows continuous sampling without wasting re-init work. What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I initially thought get_gc_stats would be misplaced on the unwinder object, since it doesn't directly relate to unwinding or stack trace collection.
But you're right that reinitializing it on every call is a waste of time.

On the other hand, keeping it separate greatly simplifies the monitoring loop (I have a prototype). We'd just wait for the process to fully initialize before starting monitoring (and ignore initialization errors), and stop monitoring on first "initialization" error after monitoring has already started.

I don't have a strong preference either way. If you think it makes more sense to move it to the unwinder object, I'm happy to make that change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

On the other hand, within the unwinder object we could use a "short-form" get_gc_stats that skips initialization and validation, simply builds RuntimeOffsets, and uses iterate_interpreters+get_gc_stats_from_interpreter_state.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants