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

feat(python): Expose allocator to capsule #17817

Merged
merged 1 commit into from
Jul 27, 2024

Conversation

ruihe774
Copy link
Contributor

Currently, pyo3 extensions and plugins are statically linked to their own copy of allocator (jemalloc/mimalloc). This does not only bloat the code size, but also increase memory usage and fragmentation: arenas of allocators are not shared, and dirty pages may not be purged after use. (purging only happens on new allocation; when an extension/plugin finishes its work, it does not allocate anymore, so the dirty pages allocated by its allocator cannot be purged.)

This PR adds the allocator capsule (polars.polars._allocator), which exposes the allocator used by python polars. pyo3 extensions and plugins can import this capsule and relay allocations to it. All allocations can now be managed by one single jemalloc/mimalloc.

@github-actions github-actions bot added enhancement New feature or an improvement of an existing feature python Related to Python Polars labels Jul 23, 2024
@ruihe774
Copy link
Contributor Author

The corresponding PR to pyo3-polars: pola-rs/pyo3-polars#88

Copy link

codecov bot commented Jul 23, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 16 lines in your changes missing coverage. Please review.

Project coverage is 80.49%. Comparing base (9eec688) to head (ae0c33c).

Files Patch % Lines
py-polars/src/allocator.rs 46.66% 16 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #17817   +/-   ##
=======================================
  Coverage   80.49%   80.49%           
=======================================
  Files        1503     1504    +1     
  Lines      197027   197059   +32     
  Branches     2795     2795           
=======================================
+ Hits       158589   158616   +27     
- Misses      37918    37923    +5     
  Partials      520      520           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orlp
Copy link
Collaborator

orlp commented Jul 25, 2024

I've thought about pros and cons and I think overall this is worth doing. Neat idea! The main downside I can think of is that each allocation call in plugins will involve an extra (properly predicted) branch and call, but a big pro to me is that in the future we can add (with a feature flag) extra memory usage tracking in our global allocator and also track the memory usage of plugins.

@ruihe774
Copy link
Contributor Author

ruihe774 commented Jul 25, 2024

The main downside I can think of is that each allocation call in plugins will involve an extra (properly predicted) branch and call

Yes, it involves a branch and an indirect call. However, imo the overhead is trivial and is not measurable in benchmarks.

we can add (with a feature flag) extra memory usage tracking in our global allocator and also track the memory usage of plugins.

Yes, it is a selling point.

@s-banach
Copy link
Contributor

I'm learning to write plugins from @MarcoGorelli's cookie cutter repo. Other than deleting the jemalloc stuff from Cargo.toml, will I have to make any other changes?

@ruihe774
Copy link
Contributor Author

I'm learning to write plugins from @MarcoGorelli's cookie cutter repo. Other than deleting the jemalloc stuff from Cargo.toml, will I have to make any other changes?

I've updated the examples in pyo3-polars: pola-rs/pyo3-polars@b5fce50. Please have a look.

@ritchie46 ritchie46 merged commit 1bb1535 into pola-rs:main Jul 27, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or an improvement of an existing feature python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants