Skip to content

Enable support for free-threading #1295

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

Merged
merged 1 commit into from
May 25, 2025
Merged

Enable support for free-threading #1295

merged 1 commit into from
May 25, 2025

Conversation

zsol
Copy link
Member

@zsol zsol commented Feb 22, 2025

Enable support for free-threading

This PR:

  1. marks the libcst.native module as free-threading-compatible
  2. replaces the use of ProcessPoolExecutor with ThreadPoolExecutor if free-threaded CPython is detected at runtime

This depends on #1294 and #1289.


Stack created with Sapling. Best reviewed with ReviewStack.

@ngoldbaum
Copy link
Contributor

I'll look at this next week - thanks for merging my other PR!

Some other things came up but I'm going to come back to this next week.

@ngoldbaum
Copy link
Contributor

I ran the test suite with unittest-ft and there are a lot of errors and failures. Unfortunately the default settings for unittest-ft run all the tests simultaneously, which makes it a little difficult for me to understand exactly where the issues are. amyreese/unittest-ft#21 might help.

I see some tracebacks inside unittest.mock, so there's probably thread safety issues in the way mock is used in the tests.

All that to say, I think I agree with #1295 (comment) that we should focus on multithreaded stress tests rather than trying to use the existing test suite as a stress test - we'll likely end up spending most of our effort on fixing races in the test suite itself.

For now I'll focus on writing a stress test that uses a thread pool to process many files simultaneously.

@ngoldbaum
Copy link
Contributor

I doubt it'll be very elucidating (it's 1.5 MB...) but here's the full output from running unittest-ft on this branch, after rebasing on main: https://gist.githubusercontent.com/ngoldbaum/0390a6afadd0fa8ebd5d5dbefe0cfd88/raw/c6d1a205ffb19240fa7de8807ca92ef3c3082ad2/gistfile1.txt

@zsol zsol force-pushed the pr1295 branch 6 times, most recently from d6c78bd to d838c82 Compare May 22, 2025 18:59
@zsol zsol marked this pull request as ready for review May 22, 2025 19:24
@zsol zsol force-pushed the pr1295 branch 2 times, most recently from dbd34c2 to 404bef2 Compare May 25, 2025 10:32
This PR:
1. marks the `libcst.native` module as free-threading-compatible
2. replaces the use of ProcessPoolExecutor with ThreadPoolExecutor if free-threaded CPython is detected at runtime

This depends on #1294 and #1289.
@zsol zsol merged commit 16ed48d into main May 25, 2025
61 checks passed
@ngoldbaum
Copy link
Contributor

Nice! I updated the tracking table too: Quansight-Labs/free-threaded-compatibility#215

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants