Conversation
There was a problem hiding this comment.
since we're changing how the subinterpreter support is flagged, i wanted some tests to show it's still working
d65661e to
16fe68d
Compare
| return 0; | ||
| } | ||
|
|
||
| static struct PyModuleDef msgspecmodule = { |
There was a problem hiding this comment.
This was simply moved down to make the flow easier, as it references _core_exec
74741fb to
06be1a7
Compare
Siyet
left a comment
There was a problem hiding this comment.
Leaving a note for the history / backlink: this obsoletes the PyState_AddModule workaround jcrist added in #561, where the body said "in the long run we should move to using multiphase module init which should avoid this problem entirely." Good to have the thread connected.
| with multi-phase init PyState_FindModule is not usable. | ||
| since we declare MULTIPLE_INTERPRETERS_NOT_SUPPORTED, we are guaranteed to have | ||
| at most one live module instance per process, so we can cache its state here. | ||
| state is populated populate in _core_exec and cleared by m_clear / m_free. |
There was a problem hiding this comment.
nit: populated populate - duplicated word.
please do ping if/when that happens 🙏🏼 |
Closes #563.
Implement multi-phase init.
Relatively straightforward, the only slight hiccough is
msgspec_get_global_state, which relied onPyState_FindModule/PyState_AddModule, which in turn don't work (well) with multi-phase init. I've replaced them with a globalstatic MsgspecState, set during module init; This is not safe for sub-interpreters, but handling it properly would be a large refactor that I think is best kept for separate PRs.Road to subinterpreter support
This is the first step for msgspec to support subinterpreters, but it's still a long way to go. I'll open a separate tracking issue for that once this gets merged, but it'll likely involve some larger refactors, especially around moving things to heap types where absolutely necessary, and ensuring this does not impact performance negatively.