Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] [WIP] Multi-phase module initialization #353

Closed
wants to merge 4 commits into from

Conversation

steve-s
Copy link
Contributor

@steve-s steve-s commented Sep 22, 2022

  • HPy uses multi-phase module initialization internally on CPython
  • HPy interface to module initialization follows the CPython multi-phase module initialization interface, the previous way of initializing HPy modules is removed
  • Please do not focus on the implementation too much, I'll polish it, but we need to agree on the interface

Examples:

// Previously:

HPy_MODINIT(my_module)
static HPy init_my_module_impl(HPyContext *ctx)
{
    HPy m = HPyModule_Create(ctx, &moduledef);
    if (HPy_IsNull(m))
        return HPy_NULL;
    // ... add types etc. to m
}

// Now:

HPy_MODINIT(my_module, moduledef, init_module)
static int init_module(HPyContext *ctx, HPy module) {
    // add types, etc. to module
}

// Optionally one can specify "create" slot for the module

HPy_MODINIT(my_module, moduledef, create_module, init_module)
static HPy create_module(HPyContext *ctx, HPy loader_spec, HPyModuleDef *def) {
    // Do something useful according to the loader_spec, use different HPyModuleDef, ...
    HPy m = HPyModule_Create(ctx, &moduledef);
    if (HPy_IsNull(m))
        return HPy_NULL;
    return m;      
}

static int init_module(HPyContext *ctx, HPy module) {
    // add types, etc. to module
}

Open issues:

  • we need to duplicate the module name in HPyModuleDef and as a parameter of the HPy_MODINIT macro :(

After we are happy with the API, I'll update other tests, examples and documentation. For now, only the newly added tests work (in all three modes: cpython, universal, debug).

@hodgestar hodgestar changed the title [RFC] [WIP] Multi-phase module inicialization [RFC] [WIP] Multi-phase module initialization Sep 24, 2022
@timfel
Copy link
Contributor

timfel commented Oct 2, 2022

Eric Snow quoted an email to Python Discourse (https://discuss.python.org/t/pep-684-a-per-interpreter-gil/19583/4) that "Multi-phase init modules are assumed to support multiple interpreters" - we should check if that is a hard assumption, because if it is, we need to rethink if this should really always be used, or if it needs to be opt in.

@mattip
Copy link
Contributor

mattip commented Oct 2, 2022

If I understand the comments from Cython, it allows multi-phase initialization but does not support multiple interpreters. I think the statement in PEP-489 is more a wish than a hard requirement, although it would be good if when refactoring for HPy, c-extension authors also prepared themselves for subinterpreters. NumPy has a long way to go to support subinterpreters.

@hodgestar
Copy link
Contributor

We should decouple these two issues completely.

Transitioning to HPy will, for many extensions, be a good step towards supporting sub-interpreters, but we shouldn't require extensions to support multiple interpreters. We should just have a good story for them when they do.

@steve-s
Copy link
Contributor Author

steve-s commented Oct 3, 2022

From the discussion it seems to me that:

  • "I implement multi-phase-init" == "I support subinterpreters" is already the case
  • "I implement multi-phase-init" == "I support per subinterpreter GIL" is not settled
  • note that the later is subset of the former, so in other words, do we want to make the implication of "I implement multi-phase-init" stronger

The idea was that we'd decouple module initialization, which from the point of HPy API would be always multi-phase, and something like an "extension initialization" -- some way to communicate required HPy ABI version and what execution mode the extension supports. Concretely:

The Python engine would first locate a symbol that would give it:

  • expected HPy ABI version
  • flags like "I support per-interpreter GIL", "I support no-gil", etc.

This symbol will be a global variable holding some struct with those flags (*). This contract will be set in stone, forever, and the struct will be part of the ABI (we can add new flags at the end). Once this communication between Python and the extension is done, then we can start module initialization (passing in the right HPyContext and making sure all the assumptions of the extension are met, like GIL is held when we call its init).

(*) in my current PR (#356) it is several symbols, one for HPy ABI minor version, one for HPy ABI major version, one for each other flag (there are no flags yet), but thinking about it now, a struct feels better somehow.

@timfel
Copy link
Contributor

timfel commented Oct 4, 2022

The idea was that we'd decouple module initialization, which from the point of HPy API would be always multi-phase, and something like an "extension initialization"

My point is that with this change, on CPython we would always use multi-phase init, which, according to PEP 489 means that "Extensions using the new initialization scheme are expected to support subinterpreters", so even the first step in our porting example would suddenly signal to CPython that the extension supports subinterpreters. That's why I think we have to stick with the old generated PyInit_ returning a fully initialized module unless the user explicitly opts in to the new scheme

@mattip
Copy link
Contributor

mattip commented Oct 4, 2022

What does the expectation in the PEP-489 statement actually imply? I understand the statement to be an inspirational warning to extension authors put there to encourage a move from static state to global module state. As far as I know there is nothing different inside CPython that makes that expectation a hard requirement.

@steve-s
Copy link
Contributor Author

steve-s commented Oct 4, 2022

My point is that with this change, on CPython we would always use multi-phase init

Right. I though we could keep the multi-phase init interface, but generate non-multi-phase init under the hood depending on some flag that would explicitly indicate subintepreters support. I.e., the fact that "I implement multi-phase-init" => "I support subinterpreters" would be CPython specific implementation detail. I think that decoupling the two things would be the best and I also commented on that here: https://discuss.python.org/t/pep-684-a-per-interpreter-gil/19583/13

Alternatively, we can keep the implementation simple and API similar to CPython by sticking to its conventions, once they are actually ironed out (not clear now if "multi-phase init" => "subinterpreters with or without shared GIL").

@hodgestar
Copy link
Contributor

hodgestar commented Oct 11, 2022 via email

@steve-s
Copy link
Contributor Author

steve-s commented Nov 3, 2022

FYI I plan to come back to this eventually once I finish other things in progress. My plan is this:

  • add possibility to specify some flags in HPy_MODINIT, not sure about the concrete API maybe: HPy_MODINIT(my_module, moduledef, init_module, some = 42, additional = "flags") -- this will generate a global symbol with a struct holding the flags
  • the flags will tell if the extension supports: sub-interpreters, per-interpreter-GIL, etc.
  • at runtime when loading the extension we will check those flags and we will do multi or single phase init depending on what flags you passed
  • I need to check if we can "emulate" multi-phase init while actually doing single phase init under the hood on CPython

@steve-s steve-s added this to the Version 0.9 milestone Nov 3, 2022
@steve-s
Copy link
Contributor Author

steve-s commented Jan 12, 2023

Closing in favor of #393

@steve-s steve-s closed this Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants