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 compute combinators #4190

Open
wants to merge 9 commits into
base: series/3.x
Choose a base branch
from

Conversation

BalmungSan
Copy link
Contributor

Related to #3353

Adds a couple of useful combinators related to compute-bound operations.
How best to suspend compute-bound operations is a FAQ in Discord, thus I believe having some combinators for the simple use cases is a good idea.

@BalmungSan BalmungSan force-pushed the add-compute-combinators branch from 3714fd7 to d0950f7 Compare December 3, 2024 20:43
@BalmungSan BalmungSan force-pushed the add-compute-combinators branch from d0950f7 to e84d14b Compare December 3, 2024 21:27
@satorg
Copy link
Contributor

satorg commented Dec 4, 2024

I would like to weigh in a little.
I'm not sure that adding these two methods: computeAttempt and computeMapAttempt is really worth it:

def expensiveComputation: Either[Throwable, TheResult] = ???

computeAttempt { expensiveComputation } <=?=> compute { expensiveComputation }.rethrow

Thereby, computeAttempt doesn't look like a big win.

Is it really important to have rethrow executed before the finalizing cede rather than after it?

@satorg
Copy link
Contributor

satorg commented Dec 4, 2024

That said, compute and computeMap look like a big deal to me...

@BalmungSan
Copy link
Contributor Author

I'm not sure that adding these two methods: computeAttempt and computeMapAttempt is really worth it:

As with any combinator is hard to argue both ways IMHO.
They are there for convenience, but I do agree it is impractical to add each and every combinator, as Fabio points regularly.
Like, compute is also just convenience, but I do agree the name would help and it seems folks are reluctant of using cede.

Personally, I like avoiding the rethrow but I also understand that it may not be seen as much, so I am happy to remove them if others agree.

@djspiewak djspiewak added this to the v3.7.0 milestone Dec 20, 2024
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.

3 participants