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

Comparison to the impedance crate, and also consider propagating panic's #2

Closed
guswynn opened this issue Jun 28, 2021 · 3 comments
Closed

Comments

@guswynn
Copy link

guswynn commented Jun 28, 2021

I have been working on a library that is supposed to make it easier to work with blocking code while writing async programs: https://docs.rs/impedance/0.3.0/impedance/

One such module in this library is the rayon one. Currently, this provides a function that lets you correctly use a 'static rayon ParallelIterator in an asynchronous fashion: https://docs.rs/impedance/0.3.0/impedance/rayon/fn.par_iter.html

This code is very similar to the way tokio-rayon works (as they both use the suggestion from the tokio readme itself: use a oneshot), but with one difference: impedance catches panic's and allows users to unpack and resume unwinds.

Today, I was looking into adding a more complete api to impedance that lets users easily use rayon ParallelIterators that contain references (https://github.com/guswynn/impedance/blob/setup/src/rayon.rs#L50-L57), but I hit a snag: rust-lang/rust#70263 (comment) that may not be easy to resolve. Therefore, I an interested in offering a more general api that would be the same as tokio_rayon::spawn, except catching panics. This would let users write things like:

spawn(
     vec,
    |v| v.par_iter(|reference| ...).collect(),
)

My understanding is that the MIT licenses on our projects are very permissive so this shouldn't be a licensing problem, but I wanted to let you know of the similarities between your crate and this (planned) section of my crate and see if you have any objections to me offering such an api. Also, if tokio-rayon in the future exports an API that exposes panics to the caller, then it might be possible to have impedance just re-export, and that may be something I am interested in!

@guswynn
Copy link
Author

guswynn commented Jun 28, 2021

Oh I see: https://github.com/andybarron/tokio-rayon/pull/1/files now, this is new from the last time I looked into this crate! Panic's are propagated, whereas the api I have chosen is to pass along the panic payload. The above information still stands though!

@andybarron
Copy link
Owner

i was actually quite torn on catching vs propagating panics, but some feedback from r/rust convinced me to do it this way (since you can always just catch_unwind if you care). but yes, feel free to steal as much or as little code as you want! looks like a great project and a lot more general-purpose than this little guy 👍

@guswynn
Copy link
Author

guswynn commented Aug 4, 2021

i was actually quite torn on catching vs propagating panics, but some feedback from r/rust convinced me to do it this way (since you can always just catch_unwind if you care). but yes, feel free to steal as much or as little code as you want! looks like a great project and a lot more general-purpose than this little guy 👍

thats awesome to hear! Thanks for the response!

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

No branches or pull requests

2 participants