haskell / haskell-language-server

Official haskell ide support via language server (LSP). Successor of ghcide & haskell-ide-engine.
Apache License 2.0
2.71k stars 368 forks source link

Migrate to OsPath #3046

Open kokobd opened 2 years ago

kokobd commented 2 years ago

We have a better alternative to FilePath now. See: https://hasufell.github.io/posts/2022-06-29-fixing-haskell-filepaths.html

What we should do:

  1. Change NormalizedFilePath to NormalizedOsPath after the issue in lsp to support OsPath https://github.com/haskell/lsp/issues/445 is solved. Profiling it to make sure the performance improves.
  2. Change other usages of FilePath to OsPath

However, this can not be done until ghc-9.6 is released (ships the new filepath library), or all ghc shipped libraries are available on Hackage.

Many GHC bundled libraries do not have new versions released to Hackage (for example ghci and hpc, their versions on hackage is very old). Thus, cabal solver fails if I add filepath ^>= 1.4.100.0 to a project that depends on ghc, which then depends on ghci. The installed ghci links against filepath-1.4.2.2 in this case, and cabal can not rebuild a ghci with filepath ^>= 1.4.100.0, as the source code is not found on hackage.

pepeiborra commented 2 years ago

Thus, cabal solver fails if I add filepath ^>= 1.4.100.0 to a project that depends on ghc, which then depends on ghci.

Oh bummer. Well, if we really want to, it should be ok to embed OsPath and combinators in a filepath-compat package.

hasufell commented 2 years ago

https://gitlab.haskell.org/ghc/ghc/-/issues/21887#note_443922

It may be possible to provide updated GHC releases that include new filepath. But this is still in discussion.

kokobd commented 2 years ago

I don't know why, but the overall memory usage decreased from 1.5G to 700M The data is collected by launching haskell-language-server from the command line in the HLS source repo with profiling RTS options enabled.

image image
hasufell commented 2 years ago

@kokobd you mean between String and OsPath?

kokobd commented 2 years ago

@kokobd you mean between String and OsPath?

@hasufell Yes. But when loading HLS executable into VSCode, I no longer witness such a big difference. I'm still investigating the reason.

pepeiborra commented 2 years ago

Can you run cabal bench ghcide and share the contents of the ghcide/bench-results folder minus the binaries ?

kokobd commented 2 years ago

cabal bench ghcide is failing with master & ghc 9.2.3. Verified by creating a fresh new Gitpod workspace from master.

image
pepeiborra commented 2 years ago

I'll take a look, but in the meantime CI shows that it works fine on 8.10.7. Would you be able to run it there?

EDIT: nevermind, I see that 8.10.7 won't do EDIT2: #3069 should help

pepeiborra commented 2 years ago

Ran the edit benchmark on Cabal 3.6.3.0, results seem quite promising: edit

kokobd commented 2 years ago

Next steps:

  1. Upgrade lsp and lsp-types in HLS to 1.5. It has some breaking changes.
  2. Either extend filepath-compat to support older versions of GHC (currently it only supports ghc 9.2.3), or release GHC minor versions with new filepath package. Support of some old GHC might need to be dropped. See: https://gitlab.haskell.org/ghc/ghc/-/issues/21887
kokobd commented 2 years ago

Real OsPath will only be available since GHC 9.6. Instead, I use ShortByteString within NormalizedFilePath. The result is still very promising.

Soon, we will benefit from the performance improvement with all versions of GHC supported by HLS.

image image

Next steps:

  1. Merge https://github.com/haskell/lsp/pull/446 and #3067. (almost done)
  2. Eliminate the usage of partial functions introduced by this PR gradually.
michaelpj commented 9 months ago

I think this is mostly done. Was there more to do?

hasufell commented 9 months ago

I haven't looked at it, but I wonder how you do file operations etc. I'm guessing you convert back to filepath somewhere? In that case the conversion is not complete

michaelpj commented 9 months ago

It looks like the situation is:

hasufell commented 9 months ago

So in ten years, I guess?

michaelpj commented 9 months ago

Two major GHC versions, so yeah, a while. Or I guess if someone is keen and wants to do it with conditional logic they could do!