Skip to content

Add Py_mod_gil slot to C extension modules #59135

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 1, 2024
Merged

Add Py_mod_gil slot to C extension modules #59135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
09e0f1b
Add Py_mod_gil slot to C extension modules
lysnikolaou Jun 28, 2024
5971c68
Add version guard
lysnikolaou Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pandas/_libs/src/datetime/pd_datetime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,12 @@ static int pandas_datetime_exec(PyObject *Py_UNUSED(module)) {
}

static PyModuleDef_Slot pandas_datetime_slots[] = {
{Py_mod_exec, pandas_datetime_exec}, {0, NULL}};
{Py_mod_exec, pandas_datetime_exec},
#if PY_VERSION_HEX >= 0x030D0000
{Py_mod_gil, Py_MOD_GIL_NOT_USED},
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @WillAyd mentioned, this is new in 3.13:

#if PY_VERSION_HEX >= 0x030D0000
    {Py_mod_gil, Py_MOD_GIL_NOT_USED},
#endif

https://docs.python.org/3.14/howto/free-threading-extensions.html#multi-phase-initialization

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. Friday was not a good day, apparently. 🤕

One question: In the free-threaded repo's README we recomment guarding with #ifdef Py_GIL_DISABLED. Should we change that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't kept tabs on the CPython implementation but seems like what the free-threaded repo suggests might be better than checking the version hex - there will be both GIL and GIL-less Python compilations for a few versions right?

Copy link
Contributor Author

@lysnikolaou lysnikolaou Jul 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WillAyd That's correct, however Py_mod_gil is just ignored in 3.13+ non-free-threaded builds. Effectively, both guards lead to correct behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a matter of style I think checking for Py_GIL_DISABLED would make more sense if you were evaluating whether that was true/false, not just checking if the symbol was defined.

So happy with what we have here, but probably worth asking for input from other projects for the free-threaded repo standard.

We can merge this as is for now and follow up with a different approach if a more canonical one becomes clearer

#endif
{0, NULL},
};

static struct PyModuleDef pandas_datetimemodule = {
PyModuleDef_HEAD_INIT,
Expand Down
7 changes: 6 additions & 1 deletion pandas/_libs/src/parser/pd_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,12 @@ static int pandas_parser_exec(PyObject *Py_UNUSED(module)) {
}

static PyModuleDef_Slot pandas_parser_slots[] = {
{Py_mod_exec, pandas_parser_exec}, {0, NULL}};
{Py_mod_exec, pandas_parser_exec},
#if PY_VERSION_HEX >= 0x030D0000
{Py_mod_gil, Py_MOD_GIL_NOT_USED},
#endif
{0, NULL},
};

static struct PyModuleDef pandas_parsermodule = {
PyModuleDef_HEAD_INIT,
Expand Down
Loading