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

Use per-thread freelists in --disable-gil builds #111968

Closed
Tracked by #108219
colesbury opened this issue Nov 10, 2023 · 16 comments
Closed
Tracked by #108219

Use per-thread freelists in --disable-gil builds #111968

colesbury opened this issue Nov 10, 2023 · 16 comments
Assignees
Labels
3.13 bugs and security fixes topic-free-threading type-feature A feature request or enhancement

Comments

@colesbury
Copy link
Contributor

colesbury commented Nov 10, 2023

Feature or enhancement

CPython uses freelists for frequently allocated Python objects, like dict, list, and slice. There freelists are generally stored in the per-interpreter state, which is not thread-safe without the GIL. In --disable-gil builds, the freelists should be stored in the per-thread state (i.e., PyThreadState). I think we probably want to keep the freelists in the interpreter state for the default build. This will probably require some refactoring.

Freelists:

  • float
  • slice (slice_cache)
  • tuple
  • list
  • dict (PyDictKeysObject and PyDictObject)
  • generator (value_freelist and asend_freelist)
  • PyContext

For context, here are similar changes in the nogil-3.12 fork, but I expect the changes in CPython 3.13 to be a bit different:

Linked PRs

@Yaro1
Copy link

Yaro1 commented Dec 10, 2023

Hi!
I'm new here, but if you don't mind I would like to take that issue

@colesbury
Copy link
Contributor Author

Hi @Yaro1, thanks for volunteering. You are welcome to take this on, but I don't think it's a great first issue. There are some subtleties and it also depends on #112776.

@Yaro1
Copy link

Yaro1 commented Dec 10, 2023

Got it, thanks.
Can I take please in that case some of the issues from #108219 ?

@colesbury
Copy link
Contributor Author

I don't think there are any good issues right now that are scoped out and not blocked on other work.

@Yaro1
Copy link

Yaro1 commented Dec 10, 2023

Okay, got it. Thank you

@corona10
Copy link
Member

@colesbury I would like to take a look at this issue, Is there any pending issues that are related to this issue?

@colesbury
Copy link
Contributor Author

@corona10, thanks! I think the issues were fixed (problems with PyThreadState_Clear() being called by the wrong interpreter)

@corona10
Copy link
Member

corona10 commented Jan 2, 2024

#113584 will introduce seamless management of freelists that runtimes do not need to care about the implementation detail :)

graph TD;
    PerThread --> FreeList;
    PerInterpreter --> FreeList;
    GC -- if free-threaded --> gc_freethreading;
    GC -- if default build --> gc_gil;
    gc_freethreading --> PerThread;
    gc_gil --> PerInterpreter;
    _PyFreeListState -- if free-threaded --> PerThread;
    _PyFreeListState -- if default build --> PerInterpreter;
    Runtime -- _PyFreeListState_GET --> _PyFreeListState;
    list --> Runtime;
    tuple --> Runtime;
    slice --> Runtime;
    float --> Runtime;
    dict --> Runtime;
    generator --> Runtime;
    PyContext --> Runtime;
Loading

corona10 added a commit to corona10/cpython that referenced this issue Jan 5, 2024
corona10 added a commit to corona10/cpython that referenced this issue Jan 10, 2024
corona10 added a commit to corona10/cpython that referenced this issue Jan 10, 2024
corona10 added a commit to corona10/cpython that referenced this issue Jan 11, 2024
@corona10
Copy link
Member

Final diagram:

graph TD;
    PerThread --> FreeList;
    PerInterpreter --> FreeList;
    GC -- if free-threaded --> gc_freethreading;
    GC -- if default build --> gc_gil;
    gc_freethreading --> PerThread;
    gc_gil --> PerInterpreter;
    _Py_object_freelists -- if free-threaded --> PerThread;
    _Py_object_freelists -- if default build --> PerInterpreter;
    Runtime -- _Py_object_freelists_GET --> _Py_object_freelists;
    list --> Runtime;
    tuple --> Runtime;
    slice --> Runtime;
    float --> Runtime;
    dict --> Runtime;
    dictkey --> Runtime;
    async_gen --> Runtime;
    _PyContext --> Runtime;
   object_stack --> Runtime;
Loading

@ericsnowcurrently
Copy link
Member

(from #111968 (comment))

As noted in review comments in gh-114323, I think the following should be adjusted:

  • change the various type-specific _PyXXX_Fini() signatures back to taking a single PyInterpreterState * argument
  • change the type-specific struct names, e.g. struct _Py_float_state, to something more specific, e.g. struct _Py_float_freelist
  • change the struct name _Py_freelist_state to _Py_freelists or, better, _PyObject_freelists
  • change PyInterpreterState.freelist_state to PyInterpreterState.freelists or, even better, to PyInterpreterState.object_state.freelists
  • split struct _Py_dictkeys_freelist out of struct _Py_dict_freelist

Thanks for taking care of these, @corona10!

FWIW, there are definitely a bunch of things we could do to simplify our use of freelists and make our implementation more maintainable, but all that mostly pre-dates this issue and is certainly something to be considered separately from this issue. 😄

@corona10
Copy link
Member

@ericsnowcurrently @colesbury
Thanks for all the feedback on this issue!

woodruffw pushed a commit to woodruffw-forks/cpython that referenced this issue Mar 4, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 17, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

4 participants