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

Add a section on memory management for extension #2864

Merged
merged 2 commits into from
Jan 10, 2023
Merged

Conversation

haixuanTao
Copy link
Contributor

Adding a special case of memory management when writing an extension.

This is a documentation of: #1056 and #2853


## On writing extension

Using `#[pyfunction]` and `#[pymethods]` makes `Python::with_gil` a no-op and
Copy link
Member

@adamreichold adamreichold Jan 7, 2023

Choose a reason for hiding this comment

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

This is not really correct. Using Python::allow_threads implies that Python::with_gil is not a no-op.

I think, the main observation is that within #[pyfunction] and #[pymethod] is already held and hence Python objects owned references tied to the GIL lifetime will usually be released only when the #[pyfunction] or #[pymethod] returns.

(The above also suggests another correction: It isn't any Python objects, but only those owned by GIL-bound Rust references.)

I would also suggest that the text stresses that this is usually not an issue except when the #[pyfunction] or #[pymethod] are long-running and/or perform a lot of transient allocations. This does happen, but it is not the common case.

Finally, the section is highly redundant to the existing one on "GIL-bound memory" and I would prefer if the information was integrated into that section. Since that section already contains everything related to the behaviour within loops and how to apply a GILPool, I think the main additions this PR should make are a) how this interactions with #[pyfunction] and #[pymethod] and b) a link to #1056 for further information on future design directions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.

I have made the section a small comment on the "GIL-bound memory" section, with a small explanation of the mechanism.

@davidhewitt davidhewitt added the CI-skip-changelog Skip checking changelog entry label Jan 10, 2023
haixuanTao and others added 2 commits January 10, 2023 07:37
Adding a special case of memory management when writing an extension.

This is a documentation of: PyO3#1056 and PyO3#2853
Copy link
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! I've force-pushed to adjust wording and remove the unneeded newsfragment (no need to log doc changes).

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 10, 2023

Build succeeded:

@bors bors bot merged commit 02fa6b8 into PyO3:main Jan 10, 2023
haixuanTao added a commit to dora-rs/dora that referenced this pull request Jan 10, 2023
To alievate the unbounded memory growth, we're replacing variable dereferencing
with scoped `GILPool` as described in the pyo3 documentation recently updated. See:
PyO3/pyo3#2864
haixuanTao added a commit to dora-rs/dora that referenced this pull request Jan 14, 2023
To alievate the unbounded memory growth, we're replacing variable dereferencing
with scoped `GILPool` as described in the pyo3 documentation recently updated. See:
PyO3/pyo3#2864
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-skip-changelog Skip checking changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants