gh-131253: free-threaded build support for pystats #137189
Draft
+985
−481
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.
Allow the
--enable-pystats
build option to be used with free-threading. For the free-threaded builds, the stats structure is allocated per-thread and then periodically merged into a per-interpter stats structure (on thread exit or when the reporting function is called).Summary of changes:
introduce
_Py_tss_stats
thread-local variable. This is set when stats are on, replacing the_Py_stats
global.replace
_Py_stats
references with_PyStats_GET()
move pystats logic from Python/specialize.c into Python/pystats.c
move the pystats global state into the interpreter structure
add some free-threaded specific stat counters
Notes and potential issues:
Adding a new thread-local variable would strictly not be necessary. I did so for two reasons. First, I think it should be
slightly faster than using
tstate
. Second, if we did add it to the tstate structure, I think it should properly go into_PyThreadStateImpl
. That seems tricky to do without some major header file re-organization. E.g. thePyStats
structure is needed byPy_INCREF()
andPy_DECREF()
and those don't have access to that structure. At least, I couldn't figure out a simple way to do it.The FTStats counts will need review. I wasn't sure about the naming or even how useful these might be. For
mutex_sleeps
, we need to determine what is the most useful thing to measure. I increment that count whenever we have to "spin" on a mutex.The logic related to
_PyStats_Attach()
,_PyStats_Detach
,_Py_StatsOn()
and_Py_StatsOff()
is intricate and I fear there could be bugs there. Trying to match the behavior of the default build, calling_Py_StatsOn()
will enable pystats recording for all threads immediately. I think calling_PyStats_Off()
should keep the current counts (not clear them).The verbose code for
print_*
andmerge_*
is not ideal. I considered making this data driven instead. However, I think that actually would add complexity, not reduce it.I plan on adding two levels of pystats recording: default and extended. The "default" level would be enabled by default (or at least could be) and would only include stats that are relatively cheap to record. The "extended" level would be like what the current
--enable-pystats
does (counting things like INCREF/DECREF, which is expensive).