Skip to content

Commit

Permalink
Fix a large chunk of exc_info changes on 0.29.x (GH-4610)
Browse files Browse the repository at this point in the history 
By disabling "CYTHON_FAST_THREAD_STATE" and "CYTHON_USE_EXC_INFO_STACK"

I think this still leaves some breakage in Coroutines.c but it's enough of a fix that Cython succeeds in building itself. Therefore I think it's worth doing now even if it doesn't fix everything.

Related to #4500
  • Loading branch information
@da-woods
da-woods committed Feb 2, 2022
1 parent 229a453 commit bbac8b5
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions Cython/Utility/ModuleSetupCode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,10 @@
#ifndef CYTHON_UNPACK_METHODS
#define CYTHON_UNPACK_METHODS 1
#endif
#ifndef CYTHON_FAST_THREAD_STATE
#if PY_VERSION_HEX >= 0x030B00A4

This comment has been minimized.

Sign in to view

Copy link
@rainwoodman

rainwoodman Aug 24, 2023

Contributor

Disabling CYTHON_FAST_THREAD_STATE breaks FastGIL under Python 3.11. Any suggestions / thoughts on what we can do to keep FastGIL working with Python 3.11 and Cython 0.29?

FYI + @robertwb

This comment has been minimized.

Sign in to view

Copy link
@scoder

scoder via email Aug 24, 2023

Contributor

This comment has been minimized.

Sign in to view

Copy link
@rainwoodman

rainwoodman Aug 24, 2023

Contributor

Thank you for the information. We do rely on FastGIL.

On the update approach: we have a distribution of packages. Last time I tried to update to Cython 3.0 (likely to 3.0a6), I stopped after updating all dependencies of h5py towards a recent h5py version that works with Cython 3.0, and added some workarounds for pandas. There was no end in sight on how many packages still may need to be updated.

I can certainly give it another try since a lot of packages may have been updated incidentally to Cython 3 compatible versions. Meanwhile, in your evaluation, how difficult would it to backport 3.11 support to 0.29? Would be like one afternoon of work? If I work out a patch, would you accept it?

This comment has been minimized.

Sign in to view

Copy link
@scoder

scoder Aug 25, 2023 •
edited
Loading

Contributor

likely to 3.0a6 [...] a lot of packages may have been updated incidentally to Cython 3 compatible versions

Certainly. A lot has happened since Cython left alpha status (and a6 was released in 2020, we had 11 alphas).

There was no end in sight on how many packages still may need to be updated.

As always, you have two options: stick with what you have (e.g. Python 3.10 and the packages that support it), or start upgrading and then upgrade everything that you need in order to make newer versions (like Python 3.11/12) work well for you. If you upgrade, you have to go all the way.

If I work out a patch, would you accept it?

No. We were very strict about avoiding any non-trivial changes since at least 0.29.30 (May 2022), and less strictly so for years. We had a couple of cases before where we allowed tiny (which this isn't) and apparently safe and well motivated "feature" changes into the legacy release series and ended up rolling them back due to problems with existing code. We will not do that again. We put a lot of work into making it easy to migrate existing code to Cython 3.0. The 0.29.x releases are really only there to help with the migration and to keep existing code working that cannot be adapted to Cython 3.0 for organisational reasons. We give no guarantees to how well we will support new CPython version with it, but we will at least try to keep it working at all for another while, in the safest (and easiest) possible way.

This comment has been minimized.

Sign in to view

Copy link
@rainwoodman

rainwoodman Aug 25, 2023

Contributor

Thanks for the information.

I tried briefly to upgrade to Cython 3.0, and immediately got stuck with h5py being unable to build with Cython 3, e.g. h5py/h5py#2268 (which points to callable type cast may be missing an "except" clause parser). More worrying was this suggestion that certain Cython versions won't be able to build certain versions of numpy and h5py: h5py/h5py#2268

I think we may be able to get away by keeping 2 versions of Cython, where certain packages are built with 0.29 cython without FastGIL support, and we only move packages to Cython 3 when they can be built. Although it is unclear how much of FastGIL we can actually still get if we do that.

This comment has been minimized.

Sign in to view

Copy link
@rainwoodman

rainwoodman Aug 25, 2023

Contributor

Also, do you know to what extent FastGIL is still relevant in Python 3.11? Was there a study on this, and if none, any suggestions what benchmarks to look at?

This comment has been minimized.

Sign in to view

Copy link
@scoder

scoder via email Aug 26, 2023

Contributor

This comment has been minimized.

Sign in to view

Copy link
@rainwoodman

rainwoodman Sep 14, 2023

Contributor

Update: we run a small FastGIL benchmark and for Python 3.10 and the 0.29 branch, with FastGIL is not faster than without FastGIL. I'll move the discussion to an issue.
#undef CYTHON_FAST_THREAD_STATE
#define CYTHON_FAST_THREAD_STATE 0
#elif !defined(CYTHON_FAST_THREAD_STATE)
#define CYTHON_FAST_THREAD_STATE 1
#endif
#ifndef CYTHON_FAST_PYCALL
Expand All @@ -194,7 +197,10 @@
#ifndef CYTHON_USE_DICT_VERSIONS
#define CYTHON_USE_DICT_VERSIONS (PY_VERSION_HEX >= 0x030600B1)
#endif
#ifndef CYTHON_USE_EXC_INFO_STACK
#if PY_VERSION_HEX >= 0x030B00A4
#undef CYTHON_USE_EXC_INFO_STACK
#define CYTHON_USE_EXC_INFO_STACK 0
#elif !defined(CYTHON_USE_EXC_INFO_STACK)
#define CYTHON_USE_EXC_INFO_STACK (PY_VERSION_HEX >= 0x030700A3)
#endif
#endif
Expand Down

0 comments on commit bbac8b5

Please sign in to comment.