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

Add concurrentMap #479

Closed
wants to merge 4 commits into from
Closed

Conversation

chrismwendt
Copy link
Contributor

@chrismwendt chrismwendt commented Feb 21, 2022

Adds concurrentMap, which spawns worker threads to process values concurrently.

This adds dependencies on containers and async. If that's unacceptable, perhaps it could be moved to conduit-extra.

conduit/test/Spec.hs Outdated Show resolved Hide resolved
@chrismwendt chrismwendt marked this pull request as ready for review February 21, 2022 10:14
@snoyberg
Copy link
Owner

So far we haven’t included this kind of behavior in conduit. There’s a large area here for proper API design, and I’d be afraid of introducing potentially unstable APIs instead the core, stable library. I think it would be great to have a separate library focused on concurrency helpers for conduit.

@chrismwendt
Copy link
Contributor Author

Out of curiosity what might a proper API design look like? It seems like concurrentMap's type signature is pretty good on its own, but maybe if there were a few more concurrency functions then a pattern of type signatures and naming conventions would emerge, and concurrentMap would be likely to change. Is that what you were thinking of too?

conduit-extra is "Experimental helper functions for conduit", so maybe that would be a good place for it. WDYT?

@snoyberg
Copy link
Owner

It's not that there's any particular issue with the API design. It's that concurrent design usually ends up needing quite a bit of special tuning. For example: do we want to ensure that items stay in the same order? That right there can lead to an extra parameter or a parallel set of functions.

conduit-extra may say "experimental," but it isn't really that anymore, the API has been stable for years. Something that may include API changes should go in a new package so it can rapidly iterate.

@chrismwendt
Copy link
Contributor Author

Would a Data.Conduit.Experimental module be a possibility? It could be documented that this function is still undergoing fine tuning and might graduate to a non-experimental namespace once it's stable. As long as users aren't expecting it to remain stable, I don't see any downside.

It could even be documented in the Haddock docstring that the type signatures of anything in *.Experimental is subject to arbitrary change and doesn't follow semver.

I suppose I could make a separate package, but I'd worry a little about fragmenting the ecosystem further.

@snoyberg
Copy link
Owner

No, I would rather it go into a separate package.

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

Successfully merging this pull request may close these issues.

None yet

2 participants