GH-146527: Add get_gc_stats function to _remote_debugging#148071
GH-146527: Add get_gc_stats function to _remote_debugging#148071sergey-miryanov wants to merge 21 commits intopython:mainfrom
Conversation
| proc_handle_t handle; | ||
| uintptr_t runtime_start_address; | ||
| struct _Py_DebugOffsets debug_offsets; | ||
| } RuntimeOffsets; |
There was a problem hiding this comment.
I'm not sure it is a best name, but I'm out of ideas a bit.
|
I'm not sure about news entry too. |
|
@pablogsal Could you please take a look? |
Co-authored-by: Shamil <ashm.tech@proton.me>
| [clinic start generated code]*/ | ||
|
|
||
| static PyObject * | ||
| _remote_debugging_get_gc_stats_impl(PyObject *module, int pid, |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
This PR implements second part of the issue. Instead of adding a new module or extending the
gcmodule we add new function to the_remote_debuggingmodule as it is a best place for this.