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

fix(rust, python): remove uses of rayon global thread pool #6682

Merged

Conversation

phaile2
Copy link
Contributor

@phaile2 phaile2 commented Feb 5, 2023

Found a few spots where the rayon global thread pool is being used, and swapped them out with the internal pool. I was able to track down the spots by using a local version of rayon and adding a panic statement in the set_global_registry method which is invoked whenever the global thread pool is constructed. I'm not sure what the best way to enforce this is. One idea is to use test fixtures, and we can check if the global pool was used at the end of each test. Another could be to patch rayon when testing similar to what I did to detect the errors.

Found a few spots where the rayon global thread pool is being used, and swapped them out with the internal pool. I was able to track down the spots by using a local version of rayon and adding a panic statement in the `set_global_registry` method which is invoked whenever the global thread pool is constructed. I'm not sure what the best way to enforce this is. One idea is to use test fixtures, and we can check if the global pool was used at the end of each test. Another could be to patch rayon when testing similar to what I did to detect the errors.
@github-actions github-actions bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Feb 5, 2023
@ritchie46
Copy link
Member

Wow! Great catch. Thanks a lot.

I think following up with a way to panic on tests is a great addition.

@ritchie46 ritchie46 merged commit 515514d into pola-rs:master Feb 5, 2023
Vincenthays pushed a commit to Vincenthays/polars that referenced this pull request Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants