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

perf: Use BitmapBuilder in a lot more places #20776

Merged
merged 4 commits into from
Jan 18, 2025

Conversation

orlp
Copy link
Collaborator

@orlp orlp commented Jan 17, 2025

This isn't quite every place yet, but gotta put a cut-off point somewhere for a PR.

@github-actions github-actions bot added performance Performance issues or improvements python Related to Python Polars rust Related to Rust Polars labels Jan 17, 2025
@orlp orlp changed the title perf: Use BitmapBuilder it a lot more places perf: Use BitmapBuilder in a lot more places Jan 17, 2025
@ritchie46
Copy link
Member

Hmm.. those coverage tests look related.

@orlp
Copy link
Collaborator Author

orlp commented Jan 17, 2025

@ritchie46 They were. The new BitmapBuilder has a method .into_opt_validity() which returns None if there were no unset bits. The old code could only return Some(bitmap), so it made an unwrap there incorrect.

Copy link

codecov bot commented Jan 17, 2025

Codecov Report

Attention: Patch coverage is 88.34951% with 24 lines in your changes missing coverage. Please review.

Project coverage is 79.77%. Comparing base (6753bb6) to head (e6218fe).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
crates/polars-arrow/src/bitmap/builder.rs 87.62% 12 Missing ⚠️
...rates/polars-compute/src/comparisons/dictionary.rs 0.00% 2 Missing ⚠️
crates/polars-compute/src/gather/generic_binary.rs 0.00% 2 Missing ⚠️
crates/polars-core/src/series/series_trait.rs 0.00% 2 Missing ⚠️
...ates/polars-ops/src/chunked_array/array/any_all.rs 0.00% 2 Missing ⚠️
...rates/polars-ops/src/chunked_array/list/any_all.rs 0.00% 2 Missing ⚠️
crates/polars-compute/src/unique/boolean.rs 50.00% 1 Missing ⚠️
crates/polars-compute/src/unique/primitive.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #20776      +/-   ##
==========================================
+ Coverage   79.74%   79.77%   +0.03%     
==========================================
  Files        1561     1561              
  Lines      222012   221979      -33     
  Branches     2530     2530              
==========================================
+ Hits       177043   177094      +51     
+ Misses      44387    44303      -84     
  Partials      582      582              

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

@ritchie46 ritchie46 merged commit 226449b into pola-rs:main Jan 18, 2025
21 of 22 checks passed
@c-peters c-peters added the accepted Ready for implementation label Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation performance Performance issues or improvements python Related to Python Polars rust Related to Rust Polars
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants