Skip to content

docs: Expand docs on when and why allow_threads is necessary#4767

Merged
davidhewitt merged 9 commits intoPyO3:mainfrom
ngoldbaum:expand-allow-threads-docs
Jan 1, 2025
Merged

docs: Expand docs on when and why allow_threads is necessary#4767
davidhewitt merged 9 commits intoPyO3:mainfrom
ngoldbaum:expand-allow-threads-docs

Conversation

@ngoldbaum
Copy link
Copy Markdown
Contributor

This is a response to the confusion encountered in #4738

I verified that the example I added in parallelism.md runs sucessfully despite the fact that it's decorated with rust,no_run.

@ngoldbaum ngoldbaum force-pushed the expand-allow-threads-docs branch from 8b47a5d to c2bb87d Compare December 5, 2024 18:11
Copy link
Copy Markdown
Contributor Author

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

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

noticed a grammar issue on my phone

Comment thread guide/src/parallelism.md Outdated
Comment thread guide/src/parallelism.md
@ngoldbaum ngoldbaum force-pushed the expand-allow-threads-docs branch from 615d8f2 to b71acef Compare December 11, 2024 18:47
Copy link
Copy Markdown
Member

@mejrs mejrs left a comment

Choose a reason for hiding this comment

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

When I read this next to https://docs.python.org/3.13/c-api/init.html I notice there's a mismatch in the terminology used. For example this documentation uses (and used) "attach" and "detach" a lot, but the C api docs don't - they talk about setting, resetting and associating threadstate. I think it would be clearer if we used the same terminology.

Otherwise it looks great, thank you!

Comment thread guide/src/free-threading.md Outdated
Comment thread guide/src/free-threading.md Outdated
Comment on lines +160 to +166
objects or call into the CPython C API. If you are not yet attached to the
Python runtime, you can register a thread using the [`Python::with_gil`]
function. Threads created via the Python [`threading`] module do not not need to
do this, but all other OS threads that interact with the Python runtime must
explicitly attach using `with_gil` and obtain a `'py` liftime.

Since there is no GIL in the free-threaded build, releasing the GIL for
long-running tasks is no longer necessary to ensure other threads run, but you
should still detach from the interpreter runtime using [`Python::allow_threads`]
when doing long-running tasks that do not require the CPython runtime. The
garbage collector can only run if all threads are detached from the runtime (in
a stop-the-world state), so detaching from the runtime allows freeing unused
memory.
do this, and pyo3 will handle setting up the [`Python<'py>`] token when CPython
calls into your extension, but all other OS threads that interact with the
Python runtime must explicitly attach using `with_gil` and obtain a `'py`
liftime.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a lot of words to say "you still need to call with_gil and use Python, to ensure that the current thread has a Python threadstate associated with it". Can we pare it down?

Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com>
@ngoldbaum
Copy link
Copy Markdown
Contributor Author

We intentionally went with the attach/detach terminology in #4577 (see e.g. #4577 (review)). Let me see if I can make it make sense using "save/restore a thread state"...

@ngoldbaum
Copy link
Copy Markdown
Contributor Author

I just asked about this issue on the CPython discussion forum: https://discuss.python.org/t/c-api-docs-for-free-threading/74183. IMO the CPython docs need free-threading specific copy here, and the terminology we're using about attaching and detaching from the runtime (IMO) makes more sense when thinking about the free-threaded build.

@ngoldbaum
Copy link
Copy Markdown
Contributor Author

There's at least some interest to change the CPython docs to emphasize having an "attached thread state" over just "holding the GIL".

@mejrs
Copy link
Copy Markdown
Member

mejrs commented Dec 16, 2024

There's at least some interest to change the CPython docs to emphasize having an "attached thread state" over just "holding the GIL".

That's some good news :) Please disregard my terminology comment then.

Copy link
Copy Markdown
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me! Sorry for the long delay with review, and Happy New Year!

@davidhewitt davidhewitt added this pull request to the merge queue Jan 1, 2025
Merged via the queue into PyO3:main with commit e33dcdb Jan 1, 2025
davidhewitt pushed a commit that referenced this pull request Jan 3, 2025
* Expand docs on when and why allow_threads is necessary

* spelling

* simplify example a little

* use less indirection in the example

* Update guide/src/parallelism.md

* Add note about the GIL preventing parallelism

* Update guide/src/free-threading.md

Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com>

* pared down text about need to use with_gil

* rearrange slightly

---------

Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com>
davidhewitt pushed a commit that referenced this pull request Jan 11, 2025
* Expand docs on when and why allow_threads is necessary

* spelling

* simplify example a little

* use less indirection in the example

* Update guide/src/parallelism.md

* Add note about the GIL preventing parallelism

* Update guide/src/free-threading.md

Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com>

* pared down text about need to use with_gil

* rearrange slightly

---------

Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com>
JeanArhancet pushed a commit to JeanArhancet/pyo3 that referenced this pull request Jan 17, 2025
* Expand docs on when and why allow_threads is necessary

* spelling

* simplify example a little

* use less indirection in the example

* Update guide/src/parallelism.md

* Add note about the GIL preventing parallelism

* Update guide/src/free-threading.md

Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com>

* pared down text about need to use with_gil

* rearrange slightly

---------

Co-authored-by: Bruno Kolenbrander <59372212+mejrs@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants