feat: implement multi-phase initialization (PEP-489)#5525
feat: implement multi-phase initialization (PEP-489)#5525davidhewitt merged 11 commits intoPyO3:mainfrom
Conversation
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks very much for picking this up, it's been a constant want to move forward with this but only so many things I can proceed on at once!
Have placed a full build label on the PR, so if you push any changes it'll trigger a full run. Last I recall, GraalPy testing was not working properly so that'll be what we need to understand before we can merge.
src/impl_/pymodule.rs
Outdated
| // TODO: remove this once we default to `gil_used = false` (i.e. assume free-threading | ||
| // support in extensions). This is purely a convenience so that users only need to | ||
| // annotate the top-level module. |
There was a problem hiding this comment.
I would be keen to switch the default to gil_used = false in PyO3 0.28, which would allow us to simplify here.
There was a problem hiding this comment.
I'll leave as is until I understand the graalpy failure, but it should be easy to drop the gil_used stuff out before final review
c84e6e5 to
f056e32
Compare
f056e32 to
edb4193
Compare
|
Interesting errors, I think this is because of
&
And I doubt there are prebuilts for the esoteric triples happening here. I'll look into this locally |
|
I think those warnings are red herrings, we should be skipping tests for any packages which don't support graalpy. |
edb4193 to
d4e90fc
Compare
74b35bf to
b4e88e9
Compare
|
Found the issue, eventually (thanks strace): Not sure how we want to handle this @davidhewitt? |
|
Looks like graalpy supports these from v25.0.0 onwards: oracle/graalpython@9faded9 |
|
Aha great find! In that case I think we should drop support for GraalPy 3.11, it seems justified if it allows us to get a big fix in. That can be done as a separate PR and then we can rebase here. |
|
Okay, can you give me a quick rundown of what that entails? crate feature/cfgs, workflow bump? |
|
#5116 is probably a good example to look at |
|
I get a clean test suite locally with |
b7bb908 to
fc9dcee
Compare
|
Not sure how to deal with the |
84a512c to
cf095d8
Compare
src/impl_/pymodule.rs
Outdated
| // Because the array is c-style, the empty elements should be zeroed. | ||
| // `std::mem::zeroed()` requires msrv 1.75 for const | ||
| values: [ZEROED_SLOT; N], |
There was a problem hiding this comment.
Switch to mem::zeroed in 01b44ed, though I think the explicitness of ZEROED_SLOT is better, personally
This code was for user convenience when gil_used defaulted to true, but since we're changing that to false, it is no longer needed.
cf095d8 to
01b44ed
Compare
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks very much for helping push this over the line! I am happy with this code as-is, and I'll follow up by writing migration notes for users both related to gil_used and for the change to multi-phase init.
PyO3 0.28 will probably be shipping around December, so perhaps with some more iteration we can figure out what's needed for users to (unsafely) opt-in to subinterpreter support in time.
(We have a lot of globals which are not subinterpreter safe, so it might be that we need to iterate gradually and accept it'll be sometime next year when the support lands.)
|
Thanks David! I'll push it for as long as I have time. I think next up is looking into how to support a public interface to module state, creating an inventory of the aforementioned globals, and then a plan for how we'll handle them. I expect module state will play a part in that plan. |
Description
This is a rebased version of #5142.
Within, we switch to using PEP-489 multi-phase initialization for pyo3 modules, over the previous single-phase initialization method.
This work lays the foundation for several major upsides for pyo3 (and consumers therein):
pymoduleinitialization via rusty wrappers around thePy_mod_execcallbackstatics when managingpymodulestateTo be clear, this PR does not implement these, but it is a necessary base step towards them.
Of note, we also switch to
gil_used = falseby default in this PR, which may require consumers to explicitly opt in, if theirpymoduleis not thread safe. I expect this to be a relatively rare occurrence given rust's stance on thread safety.