Our internal headers are far too interlinked

Adding a small feature like #131198 (comment) becomes a complicated puzzle of moving functions and structs between header files to avoid cycles.
I was ultimately defeated in this case.

This is a bit silly. We should break up these dependencies, by breaking headers into structs and code.
Code depends on data, and headers that depend on some structs, but not the code, can import only the struct definitions. Breaking headers into smaller, more self contained chunks would also help.

As an example of the problem, to use _PyThreadState_GET() one must import pycore_pystate.h but that imports pycore_runtime.h which imports nineteen other "core" header files!

Linked PRs

GH-127705: Handle trace refs in specialized decref (GH-131198)

One solution to reduce dependencies would be to convert some static inline and macros to regular functions to move the implementation details from header files to the C code (.c files).

As an example of the problem, to use _PyThreadState_GET() one must import pycore_pystate.h but that imports pycore_runtime.h which imports nineteen other "core" header files!

In Python 3.11, _PyThreadState_GET() uses the runtime API:

static inline PyThreadState*
_PyThreadState_GET(void)
{
    return _PyRuntimeState_GetThreadState(&_PyRuntime);
}

In Python 3.12, it's no longer the case:

static inline PyThreadState*
_PyThreadState_GET(void)
{
#if defined(HAVE_THREAD_LOCAL) && !defined(Py_BUILD_CORE_MODULE)
    return _Py_tss_tstate;
#else
    return _PyThreadState_GetCurrent();
#endif
}

But there is still code which access the PyRuntimeState API:

static inline int
_Py_IsMainInterpreterFinalizing(PyInterpreterState *interp)
{
    return (_PyRuntimeState_GetFinalizing(&_PyRuntime) != NULL &&
            interp == &_PyRuntime._main_interpreter);
}

#define HEAD_LOCK(runtime) \
    PyThread_acquire_lock((runtime)->interpreters.mutex, WAIT_LOCK)
#define HEAD_UNLOCK(runtime) \
    PyThread_release_lock((runtime)->interpreters.mutex)

The problem is that the structs and functions are mixed together, which means that any function that refers to a struct in another file introduces cycles. Breaking the headers in struct and function pairs breaks most of the dependencies

Another problem is that the _PyRuntimeState structure is a giant beast. I never understood the rationale for putting ALL global variables in a single place. For example, _Py_ID() requires to get access to the whole _PyRuntimeState structure since singletons are stored there. It means pulling all includes to build this structure. But I don't know if it would make sense to move back globals were they were before to split again _PyRuntimeState.

pythongh-131238: Add explicit include to pycore headers

GH-131238: Core header refactor (GH-131250)

pythongh-131238: Add explicit include to pycore headers

pythongh-131238: Convert pycore_pystate.h static inline to functions

gh-131238: Convert pycore_pystate.h static inline to functions (#131352)

gh-131238: Add explicit includes to pycore headers (#131257)

pythongh-131238: Remove pycore_runtime.h from pycore_pystate.h

GH-131238: More refactoring of core header files (GH-131351)

pythonGH-127705: Handle trace refs in specialized decref (pythonGH-13…

pythongh-131238: Remove pycore_object_deferred.h from pycore_object.h

gh-131238: Remove pycore_object_deferred.h from pycore_object.h (#131549

pythongh-131238: Add pycore_interpframe_structs.h header

gh-131238: Add pycore_interpframe_structs.h header (#131553)

pythongh-131238: Move _Py_VISIT_STACKREF() to pycore_stackref.h

These changes keep breaking JIT builds on main: https://github.com/python/cpython/actions/workflows/jit.yml

To save resources, we skip JIT CI on changes that don't obviously touch JIT code. But these header refactorings are breaking things anyways, since they don't "touch" any JIT files.

Would you be able to check locally that JIT builds still work correctly, or maybe force JIT CI to run by touching a JIT file and reverting before merging more of these changes? It would also be great if you could figure out how to fix the JIT builds too, since so much stuff has moved. Thanks.

gh-131238: Move _Py_VISIT_STACKREF() to pycore_stackref.h (#131560)

pythongh-131238: Add missing pycore_function.h includes for JIT compiler

These changes keep breaking JIT builds on main: https://github.com/python/cpython/actions/workflows/jit.yml

Oh, sorry about that. I forgot that JIT CIs are skipped by default. I wrote #131571 to fix it.

Would you be able to check locally that JIT builds still work correctly, or maybe force JIT CI to run by touching a JIT file and reverting before merging more of these changes? It would also be great if you could figure out how to fix the JIT builds too, since so much stuff has moved. Thanks.

I was able to reproduce the issue locally and I wrote a fix.

Thanks!

gh-131238: Add missing pycore_function.h includes for JIT compiler (#…

I close the issue. While it's not perfect, the situation should now be better than it was before these changes.

Previously, including a single header like pycore_object.h pulled almost all pycore headers. I modified pycore header files to remove many pycore includes. Now C files must include more pycore headers which makes the dependencies more explicit.

pycore_runtime.h remains the largest header pulling most pycore headers. I tried to minimize pycore headers including pycore_runtime.h. Going further would require to convert static inline functions to regular functions: it can affect performance in a negative way (even if LTO should reduce the issue).

For example, it would be nice to avoid #include "pycore_runtime.h" from pycore_object.h, but the following code uses _PyRuntime (which comes pycore_runtime.h):

#define _PyReftracerTrack(obj, operation) \
    do { \
        struct _reftracer_runtime_state *tracer = &_PyRuntime.ref_tracer; \
        if (tracer->tracer_func != NULL) { \
            void *data = tracer->tracer_data; \
            tracer->tracer_func((obj), (operation), data); \
        } \
    } while(0)

Navigation Menu

Repository navigation

Our internal headers are far too interlinked #131238

Linked PRs

33 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Our internal headers are far too interlinked #131238

Description

Linked PRs

Activity

vstinner commented on Mar 14, 2025

markshannon commented on Mar 14, 2025

vstinner commented on Mar 14, 2025

33 remaining items

brandtbucher commented on Mar 21, 2025

vstinner commented on Mar 21, 2025

brandtbucher commented on Mar 21, 2025

vstinner commented on Mar 21, 2025

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions