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

gh-117142: ctypes: Migrate global closure freelist to thunk-type state #117874

Closed
wants to merge 11 commits into from
Closed

gh-117142: ctypes: Migrate global closure freelist to thunk-type state #117874

wants to merge 11 commits into from

Conversation

neonene
Copy link
Contributor

@neonene neonene commented Apr 14, 2024

This introduces PyCThunkType_Type, a metaclass of PyCThunk_Type, to bypass the module check during a finish.

@encukou
Copy link
Member

encukou commented May 6, 2024

What problem is this solving, and why can't it be solved in a simpler way?

I think the closure freelist can stay process-global. The free-threaded build will need some locking (not just for multiple interpreters), but I think closure alloc/dealloc happens rarely enough that a global mutex should be enough. (This is unlike most of Python's freelists, which exist because allocation/deallocation is too common.)

We don't free the allocated pages at interpreter shutdown. I don't think that's a big concern, though -- while we don't munmap the pages, we do recycle them.

Is there something I'm missing?

@neonene
Copy link
Contributor Author

neonene commented May 6, 2024

I'm trying to reduce global variables (globals-to-fix.tsv) as much as possible first, then asking for a decision. If you think that the closure (de)allocation is not frequent, malloc_closure.c (freelist) can be removed in the future? Leaving a freelist-less experiment. The CI appears to be OK.

@neonene neonene closed this May 6, 2024
@encukou
Copy link
Member

encukou commented May 8, 2024

It would be nice if libffi (ffi_closure_alloc/_free) always did all this for us (on all platforms) so we could drop this code. Not sure what's the status there; I don't know that much about libffi.

@neonene neonene deleted the freelist_type branch May 22, 2024 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants