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

Move away from APIs that use borrowed references under the free-threaded build #8216

Merged
merged 10 commits into from
Jul 13, 2024

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Jul 8, 2024

For #8199.

Make a start on making Pillow's code ready for free-threading (PEP 703).

This is some of the low-hanging fruit from https://docs.python.org/3.13/howto/free-threading-extensions.html.

@hugovk hugovk added the Free-threading PEP 703 support label Jul 8, 2024
src/_imagingft.c Show resolved Hide resolved
src/encode.c Outdated Show resolved Hide resolved
Copy link
Contributor

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

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

LGTM! Let's merge this.

@hugovk hugovk merged commit 6944e9e into python-pillow:main Jul 13, 2024
49 of 52 checks passed
@hugovk hugovk deleted the free-threading branch July 13, 2024 14:09
@radarhere
Copy link
Member

radarhere commented Jul 15, 2024

pythoncapi_compat.h has been added in it's entirety, but if I'm reading this right, we're only using PyList_GetItemRef() and PyDict_GetItemRef().

Was the entire file added just for simplicity, or do you expect that other functions will be used in the future as well?

@hugovk
Copy link
Member Author

hugovk commented Jul 15, 2024

I thought about only including what we use, but added the whole thing for simplicity. It also makes updating it easier, and we might use more in the future.

I'm not against using a minimal file, but then again, does it affect compiled executable size (at all or significantly)? (I'm on a plane waiting to take off so can't test right now :)

@radarhere
Copy link
Member

The file is currently 1103 bytes, so I don't consider size to be a problem. I just wanted to clarify the rationale.

@radarhere radarhere mentioned this pull request Jul 15, 2024
@nulano
Copy link
Contributor

nulano commented Jul 15, 2024

does it affect compiled executable size (at all or significantly)

They appear to all be static inline functions, so as far as I understand (I also checked GCC on godbolt.org), only those functions that we actually use should be included in the compiled executable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Free-threading PEP 703 support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants