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

Feature flag for crossbeam / multithreading usage? #18

Closed
repi opened this issue Aug 12, 2019 · 3 comments
Closed

Feature flag for crossbeam / multithreading usage? #18

repi opened this issue Aug 12, 2019 · 3 comments

Comments

@repi
Copy link

repi commented Aug 12, 2019

crossbeam is a pretty big dependency for a small crate so would be nice to be able to use this crate without bringing in all of crossbeam. Ran esp. into this with #16 where the old crossbeam version had a flagged security vulnerability in the old memoffset crate.

As ThreadMode::Sequential is default anyway, having an optional feature flag that adds the crossbeam dependency and exposes the ThreadMode::Parallel would be great and then. And not having this feature flag in the default feature listing, so crates have to opt in to the parallel crossbeam code instead if they want to use it.

Would such a change be ok? Would be a breaking change was ThreadMode::Parallel would only be exposed if the parallel feature flag is used. But would likely be ok as this crate is not at 1.0 stable yet

@mrijkeboer
Copy link
Member

Hi @repi

Thanks for creating this issue. Pull request #19 changes the dependency from crossbeam to crossbeam-utils which pulls in less dependencies. Would this suffice for your use case?

@repi
Copy link
Author

repi commented Aug 16, 2019

This definitely does reduce the amount of dependencies, which is great.

I'm a bit hesitant to smaller libraries having their own thread spawning or parallelism in general, as that is pretty heavy duty operations, so do think a feature flag would be the nicest. But this is not a major issue for us after the PR has been merged in so np to close this

@mrijkeboer
Copy link
Member

Hi @repi

Thanks for your feedback. I'll close this for now and will keep it in mind for the future.

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