-
-
Notifications
You must be signed in to change notification settings - Fork 389
Consider using Reflex backend #796
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
Comments
I just tried on master again and hover was reporting in 4.5s for The branch is still experimental so it should only be used in anger I think. I am not claiming it will be ready to merge at any immediate point but if anyone is interested in also experimenting on it then they should feel free. |
4.5s is insane. A profile (either Shake based or GHC based) would be fantastic, ideally both. FWIW, we had massive slowdowns in DAML at one point, and it turned out we were deepseq'ing a huge list repeatedly. There are so many subtle ways to get it wrong :) |
Are you measuring the command line or the IDE through LSP. GHC has a lot of files and include directories so to correctly determine if anything has changed requires a huge number of doesFileExist calls. Those are unavoidable if you want correctness, either in a push or pull model. But I believe we can cheat and assume they haven't changed, and some LSP monitoring solves some of them. |
I am measuring through an IDE. The shake profile reports the right total-time but the sum of all the rules in the run doesn't add up to it. I will need to do some proper profiling but in order to do that will have to build with ghc-8.10. |
Here is the profile of performing a single hover request. https://gist.github.com/mpickering/c12db7d1b9c0724024259f8411189ca6 Load the JSON into (This is with ghcide restricted to a single thread, hovering with |
Are we still considering using it? |
I am still considering abstracting out the build system parts, and making it easier to write our own/use reflex/use Shake and change between them. But its nowhere near the top of my list. |
Now that we have reactive builds via reverse dependencies in Shake, is there any reason to consider Reflex? |
will close soon if nobody disagree |
closing, @ndmitchell feel free to reopen if you think the suggestion is still sensible after last changes in ghcide |
@mpickering wrote a port to a Reflex backend: https://mpickering.github.io/posts/2020-03-16-ghcide-reflex.html. See https://github.com/mpickering/ghcide/tree/reflex
I'm sceptical that the 1s slowdown on hover is due to Shake overheads, so it would be good to get profile reports showing that. I suspect there's something else we're doing that is taking the 1s, and that should probably be fixed - but the profile will tell us all.
In the longer run, we all knew a well-tested FRP would be a better fit. @mpickering, is this a path towards Ghcide 1.1, or an experiment that we should use to guide future FRP/Ghcide evolution so they collide at a future date?
The text was updated successfully, but these errors were encountered: