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

[Python] Support for the free-threaded build of CPython 3.13 #43536

Closed
10 of 11 tasks
lysnikolaou opened this issue Aug 2, 2024 · 18 comments
Closed
10 of 11 tasks

[Python] Support for the free-threaded build of CPython 3.13 #43536

lysnikolaou opened this issue Aug 2, 2024 · 18 comments

Comments

@lysnikolaou
Copy link
Contributor

lysnikolaou commented Aug 2, 2024

Describe the enhancement requested

Hey all! 👋

I've started looking into adding support for the 3.13 free-threaded build of CPython. This aims to be a tracking issue for all work necessary to support it. A very high-level list of steps is (more details to follow as I investigate more issues):

Hope this all sounds good! Let me know in case there's something that doesn't seem right.

Component(s)

Python

lysnikolaou added a commit to lysnikolaou/arrow that referenced this issue Aug 7, 2024
This is done by passing an extra flag when building the Cython extension
modules. It is needed so that the GIL is not dynamically reenabled
when importing `pyarrow.lib`.
@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Aug 13, 2024

@lysnikolaou for adding a free-threaded test build, I suppose we will want to add a wheel build as well (so after #43539 is finally ready), to ensure we have nightly wheels people can test with.
But regardless of wheels, it might be good to have a plain test build as well, which could mimic the cpython-debug build that Antoine added? (#43565) That is something that doesn't have to wait on the wheels / general python 3.13 support in CI, I think

@lysnikolaou
Copy link
Contributor Author

But regardless of wheels, it might be good to have a plain test build as well, which could mimic the cpython-debug build that Antoine added? (#43565) That is something that doesn't have to wait on the wheels / general python 3.13 support in CI, I think

On it.

lysnikolaou added a commit to lysnikolaou/arrow that referenced this issue Aug 13, 2024
kou added a commit that referenced this issue Aug 13, 2024
### Rationale for this change

This is done by passing an extra flag when building the Cython extension modules. It is needed so that the GIL is not dynamically reenabled when importing `pyarrow.lib`.

### What changes are included in this PR?

Changes to CMake so that the extra flag is passed when building Cython extension modules.

* GitHub Issue: #43536

Lead-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 18.0.0 milestone Aug 13, 2024
@kou
Copy link
Member

kou commented Aug 13, 2024

Issue resolved by pull request 43606
#43606

@kou
Copy link
Member

kou commented Aug 13, 2024

@lysnikolaou Could you create a sub issue per sub item? Our release management scripts don't support multiple PRs per issue for now.

@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Aug 14, 2024

This is fine to keep as the general tracking issue (the release script will just only list this issue in the changelog, which IMO is perfectly fine). It is easier project management to keep track of this in a single issue.

@lysnikolaou
Copy link
Contributor Author

I agree that this issue should be kept open, since it provides a nice overview of what level of support can be expected.

@kou Sure! Should I open subissues for the already open/merged PRs as well or is it okay to just do it in the future?

@kou
Copy link
Member

kou commented Aug 15, 2024

If @jorisvandenbossche doesn't like create subissues, I don't ask you to create subissues strongly. (As a committer, I don't want to reopen this issue manually after I merge a PR by our merge script.)
@raulcd What do you think about this as a release manager? We can't assign multiple milestones to one issue. If we slip some subtasks to 19.0.0, it may cause a problem.

FYI: We used an umbrella issue and subissues when we added support for Azure filesystem: #18014

@raulcd
Copy link
Member

raulcd commented Aug 15, 2024

At the moment all our merge (closes issues automatically) and release scripts (generation of release notes, cherry picking) expect one issue per PR.

This was a requirement for JIRA but now that we are tracking issues on GH I think we can revisit that pattern, we could even avoid some issues and just use PRs for some things but this would require some time to adapt our current scripts.

As per this particular issue, the biggest potential problems for the release:

  • As you have already seen the issue will keep getting closed every time we merge a subtask.
  • PRs merged pointing to this PR will probably not appear on the release notes (as far as I know release notes are generated from issues tagged with the correct milestone not PRs)
  • If we merge something after the feature freeze it won't get cherry-picked by our scripts

To be honest not really breaking issues for the release but if this is a workflow we want to use (which I think we should) we should adapt our scripts. I am currently on vacation so might take some time responding.

@raulcd
Copy link
Member

raulcd commented Aug 15, 2024

@kou Sure! Should I open subissues for the already open/merged PRs as well or is it okay to just do it in the future?

Already merged PRs point to the "parent" issue on the commit message, this is what is used to link the commits to issues on the release scripts, unless we want to change the commit history (which we are not going to do) it doesn't matter if we open issues for the ones already merged :)

BTW, thanks for the great work you are doing @lysnikolaou

@lysnikolaou
Copy link
Contributor Author

Ok then, I'll be creating subissues for PRs I open from now on. Thanks everyone!

pitrou added a commit that referenced this issue Aug 15, 2024
### Rationale for this change

For better reference safety under Python free-threaded builds (i.e. with the GIL removed), we should be using `Py(List|Dict)_GetItemRef` that return strong references and are implemented in a thread-safe manner.

### What changes are included in this PR?

- Vendor a copy of https://github.com/python/pythoncapi-compat
- Port to strong reference APIs for lists and dicts

### Are these changes tested?

I ran the tests with the free-threaded build before and after, and there's the same expected failures.

* GitHub Issue: #43536

Lead-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Co-authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou
Copy link
Member

pitrou commented Aug 15, 2024

Issue resolved by pull request 43540
#43540

@pitrou pitrou closed this as completed Aug 15, 2024
@lysnikolaou
Copy link
Contributor Author

Can we please reopen this? These are PRs that I had created before we discussed this. Sorry!

@pitrou
Copy link
Member

pitrou commented Aug 15, 2024

Woops, sorry, I've been a bit too optimistic here :)

@pitrou pitrou reopened this Aug 15, 2024
pitrou pushed a commit to lysnikolaou/arrow that referenced this issue Sep 9, 2024
pitrou added a commit that referenced this issue Sep 9, 2024
#43671)

### Rationale for this change

Testing with the free-threaded build is required for adding support for it. (see #43536)

### What changes are included in this PR?

- Add a Docker build with the CPython free-threaded build from deadsnakes.
- Add a Crossbow job to run said Docker build with Python 3.13t

### Are there any user-facing changes?

No.
* GitHub Issue: #43536

Lead-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Co-authored-by: Antoine Pitrou <antoine@python.org>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
khwilson pushed a commit to khwilson/arrow that referenced this issue Sep 14, 2024
…d build (apache#43671)

### Rationale for this change

Testing with the free-threaded build is required for adding support for it. (see apache#43536)

### What changes are included in this PR?

- Add a Docker build with the CPython free-threaded build from deadsnakes.
- Add a Crossbow job to run said Docker build with Python 3.13t

### Are there any user-facing changes?

No.
* GitHub Issue: apache#43536

Lead-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Co-authored-by: Antoine Pitrou <antoine@python.org>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
pitrou added a commit that referenced this issue Sep 18, 2024
#43965)

### Rationale for this change

Building free-threaded wheels is necessary to support the free-threaded build. We probably want to upload these wheels as nightlies somewhere as well, so that downstream users can test the free-threading-related changes.

### What changes are included in this PR?

- Add necessary configuration to build 3.13 free-threading wheels on `manylinux`.
- Do necessary changes to build free-threaded wheels on macOS as well.

### Are these changes tested?

I tested the `manylinux` wheel builds. macOS is still untested, since it's not dockerized.

### Are there any user-facing changes?

No.

Related to #43536.
* GitHub Issue: #43964

Lead-authored-by: Lysandros Nikolaou <lisandrosnik@gmail.com>
Co-authored-by: Antoine Pitrou <antoine@python.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
@pitrou
Copy link
Member

pitrou commented Oct 8, 2024

Thanks to astral-sh/python-build-standalone#326 , there should be very soon free-threaded builds available on https://github.com/indygreg/python-build-standalone/ . That might be easier than using a PPA and being constrained to Ubuntu.

@raulcd
Copy link
Member

raulcd commented Oct 9, 2024

@jorisvandenbossche @pitrou is there anything to be done here for 18.0.0?

@pitrou
Copy link
Member

pitrou commented Oct 9, 2024

@raulcd I don't think so. The wheels are produced fine AFAICT (hopefully they will correctly be uploaded on PyPI as well? :-)).

@jorisvandenbossche
Copy link
Member

Indeed, I think the basics are ready (I also had the question whether the wheels would automatically be uploaded, but @raulcd thought that would be the case).

At the moment the linux wheels builds for free-threaded are failing though, but created a separate issue for that: #44355

I will open a follow-up issue for the remaining tasks, so we can close this was as done for 18.0

@raulcd
Copy link
Member

raulcd commented Oct 13, 2024

I am closing this as done for 18.0.0. The only sub-task missing is around testing Cython. Please feel free to open a new issue for that if necessary.

@raulcd raulcd closed this as completed Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants