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

Heterogeneous concurrent apply #803

Closed
wants to merge 1 commit into from

Conversation

pepeiborra
Copy link
Contributor

We want to use rule parallelism in ghcide, but our user rules have heterogeneous key and result types.

The Apply applicative is to Shake as the Concurrently applicative is to async.

For an example of the intended usage, see:

haskell/haskell-language-server@dfeb649

We want to use rule parallelism in ghcide, but our user rules have heterogeneous
key and result types.

The Apply applicative is to Shake as the Concurrently applicative is to async

For an example of the intended usage, see:

haskell/haskell-language-server@dfeb649
Copy link
Owner

@ndmitchell ndmitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you aware that just doing normal applicative operations in the Action monad already does them together? So if you do apply F <*> apply X it will evaluate F and X in parallel. With that, do we need this mechanism?

@pepeiborra
Copy link
Contributor Author

Were you aware that just doing normal applicative operations in the Action monad already does them together? So if you do apply F <*> apply X it will evaluate F and X in parallel. With that, do we need this mechanism?

I was vaguely aware, I requested that feature but it wasn't clear when/how it works. Can you clarify whether F and G are evaluated in parallel in fmap f (apply F) <*> fmap g (apply G)?

Do we still need this? Maybe. Apply guarantees that the parallelism happens, so it can still be useful.

@ndmitchell
Copy link
Owner

Any combination of operations that doesn't use >>= or join is guaranteed to be run in parallel - so your example above is guaranteed. I think there is an advantage of having Apply which makes it explicit, but a disadvantage of having to use apply' when most things are conveniently wrapped in a well-typed Action - e.g. you can't use need with apply' without exposing the internal types behind files (not a good abstraction) or adding need' (two functions for everything, which isn't great).

Looking at the docs for Action they don't mention this behaviour, which is a bug in the docs.

@pepeiborra
Copy link
Contributor Author

That seems too good to be true. Why do we need parallel if that's the case?

@ndmitchell
Copy link
Owner

parallel came first - I should probably add to the docs that it isn't necessary in many cases.

There are still useful cases for parallel though - if your Shake threads are doing meaningful work, or there are monadic dependencies in either branch, it can help. But it's use case is significantly diminished from when you didn't have applicative merging.

@pepeiborra
Copy link
Contributor Author

In that case, all we need to do in ghcide is enable ApplicativeDo and sit back. Nice! I'll give it a go and revert to this PR

@pepeiborra
Copy link
Contributor Author

haskell/haskell-language-server#1667 enables ApplicativeDo in ghcide and seems to do the trick (although I have not collected any evidence of parallelism).

Closing this PR as unneeded, since the applicative instance already does what we need and as you said apply' is not always convenient to use.

@ndmitchell
Copy link
Owner

Thanks, I've improved the docs.

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