BetterThanTomorrow / calva

Clojure & ClojureScript Interactive Programming for VS Code
https://marketplace.visualstudio.com/items?itemName=betterthantomorrow.calva
Other
1.69k stars 217 forks source link

Add setting for disabling clojure-lsp #917

Closed bpringe closed 3 years ago

bpringe commented 3 years ago

At least currently, clojure-lsp has high memory usage. This compounds on itself when users have multiple instances of VS Code + Calva running. A setting to allow users to opt out of clojure-lsp would be nice.

I'm not sure about all the implications yet. We need to make sure that without clojure-lsp, Calva still works as it did before as far as REPL features and such.

I think this should be a user setting, but what about a project setting? We could have a user put something in their .calva/config.edn that says whether to start clojure-lsp or not, and this setting would take priority over the user setting. Though, this project setting may be a bit much, and maybe we should start with a user setting only.

What do you think, @PEZ?

bpringe commented 3 years ago

This is going to be put on hold for now, unless users mention it's causing real issues. I'm going to prioritize improving clojure-lsp's memory usage, and if this proves to take longer than desired we can consider adding a setting.

Ideally the language server should not be optional and should provide most of the language's features (as much as it can), while, for Clojure, the REPL provides what the language server cannot.

practicalli-johnny commented 3 years ago

Another use case for disabling clojure-lsp:

When opening Calva with a Clojure project open. If there is a new upgrade, pressing install and restart required in the extension will restart VS Code. If clojure-lsp has not completed indexing the project before the VS code restart, a second clojure-lsp process runs.

Is seems a restart of VS Code may also cause a clojure-lsp process to not die, consuming resources. Whist carrying out some development on a VS Code extension with a Clojure project open, I had 3 processes running (or at least consuming resources) at one point.

Having a user level configuration to prevent clojure-lsp from automatically running each time would make using Calva less disruptive.

With anacondo (an alternative approach to using lsp that also used clj-kondo), it would not run automatically, however, I could call a function for a project that would run the indexing for lsp. This gives control back to the user, rather than let the computer start consuming resources when ever it wants

Its very reminisent of the frustrating times when IntelliJ or Windows would just go and index stuff and you had to wait or have your computer slow down because its taking all the resources. On my older laptop with only 8Gb memory, Calva with lsp does have a big impact as I get a lot more swapping. So lsp could affect a lot of new people learning Clojure who tend to have less resources available.

PEZ commented 3 years ago

Calva is headed in a direction where clojure-lsp is an integral part of it. It would get quite hard to keep it optional. Already that is quite a bit tricky, and this will increase as the integration deepens.

The startup time and memory consumtion is problematic, I agree. To me, and to @bpringe, this speaks in the direction to deal with that problem, rather than spending time on some temporary disablement fix. Hopefully we can bring it where it is fast enough, and memory efficient enough to not be a problem. If it turns out that path is very hard and long, we will have to reconsider our options. But for now, people will either have to endure the costs of the clojure-lsp features, or stay on v2.0.136 until we have brought down the costs to whatever level is acceptable for a given user.

bpringe commented 3 years ago

a restart of VS Code may also cause a clojure-lsp process to not die, consuming resources

This is something I think we may be able to fix, and I've noticed multiple lsp processes running when I have no VS Code instance running. So I'll be looking into how/if we can ensure the process is stopped when it should be.

Its very reminisent of the frustrating times when IntelliJ or Windows would just go and index stuff and you had to wait or have your computer slow down because its taking all the resources

I very much agree it is a problem.

As Peter said above, Calva is headed in a direction where we'd like to rely on it more and even get rid of code in Calva where/if possible in favor of using clojure-lsp. This means if it were disabled, Calva would lack features it used to have. However, I currently do not want to push any more down that path until we know for sure what can be done to improve the memory usage of clojure-lsp, or until that's actually done.

sirmspencer commented 3 years ago

The issue I am having is that it runs automatically on projects where I have not added .clj-kondo or .lsp to gitignore. Im getting obnoxious amounts of changes for git. Some of these project I dont have permissions to modify gitignore and it just makes the PR process a huge pain.

** Edit, a global gitignore file seems to work for this.

I second whats been said above though. Many of my projects are large. VS code remembers the last project you were working on, and if it happens to not be the one I need, clojure-lsp is running long processes on both repos.

bpringe commented 3 years ago

it runs automatically on projects where I have not added .clj-kondo or .lsp to gitignore

Yeah, this can be annoying at first, but it settles after you've added them to the .gitignore. I haven't figured out yet if there's something that should/could be done about this. I already don't really notice it any more after modifying the .gitignore files on the projects I'm most often working in.

VS code remembers the last project you were working on, and if it happens to not be the one I need, clojure-lsp is running long processes on both repos.

If you're referring to issue #906, it was fixed today, maybe soon after your comment. If a project opens you didn't want open, you can switch and it won't leave processes running in the background (shouldn't any more, at least).

ebdekock commented 3 years ago

Im currently finding Calva difficult to use, it's very slow when evaluating a broken expression, takes more than 30 seconds. Im assuming its related to the new LSP stuff, spoiler, skip from 0:10 -> 0:50

https://user-images.githubusercontent.com/5781355/103608183-66147b00-4f23-11eb-8ed2-0499a5f9a259.mov

Edit:

sdfREPL-y 0.4.4, nREPL 0.8.3
Clojure 1.10.1
OpenJDK 64-Bit Server VM 11.0.7+10-LTS
PEZ commented 3 years ago

@ebdekock, that took crazy long time! We fixed a similar issue some days ago. Which version of Calva are you using?

ebdekock commented 3 years ago

Calva - 2.0.146 clj-kondo - 2020.12.120

Mac, Catalina - 10.15.7 VSCode - November 2020 (version 1.52)

Reinstalled the Calva extension just to make sure its the right version, still persists, please let me know if you have any ideas :)

PEZ commented 3 years ago

The issue I was referring to is this one: https://github.com/BetterThanTomorrow/calva/issues/896

TL;DR; An older version of cider-nrepl was locking in some strange circumstances (while trying to query clojuredocs).

But this should not be going on now, since we are using latest cider-nrepl... Also, this shouldn't have anything to do with clojure-lsp. I'd appreciate a new issue about it.

ebdekock commented 3 years ago

Sorry for the issue hijack, I've opened a new one, please see #929

PEZ commented 3 years ago

@sirmspencer

Some of these project I dont have permissions to modify gitignore and it just makes the PR process a huge pain. ** Edit, a global gitignore file seems to work for this.

What also works is to put a .gitignore with the contents of * in the .lsp directory and in the .clj-kondo directory .

Not sure if these directories are supposed to sometimes hold files that shouldn't be ignored by git, though. What's @ericdallo's and @borkdude's input on that? Maybe Calva (or even clojure-lsp, and/or clj-kondo) could add .gitignore files to these directories automatically?

borkdude commented 3 years ago

@PEZ @sirmspencer Only .clj-kondo/.cacheshould be ignored, .clj-kondo/config.ednand hook code is useful to put in source control.

PEZ commented 3 years ago

Thanks, @borkdude. That's what I use in my projects. Although I have to ignore .clj-kondo/cache (w/o the dot before cache) as well.

borkdude commented 3 years ago

@PEZ That surprises me since clj-kondo does not write to .clj-kondo/cache, only .clj-kondo/.cache.

PEZ commented 3 years ago

It's quite odd, for sure. Looks like so in a project I just created with lein new and then started Calva in:

image
borkdude commented 3 years ago

@PEZ You can override the cache using a setting. Maybe some other tools like Calva itself or clojure-lsp is using a different cache key? I've never seen this happen to my own projects. Also not reproducible for me from the command line:

borkdude@MBP2019 /tmp $ lein new foo
Generating a project called foo based on the 'default' template.
The default template is intended for library projects, not applications.
To see other templates (app, plugin, etc), try `lein help new`.
borkdude@MBP2019 /tmp $ cd foo
borkdude@MBP2019 /tmp/foo $ ls
CHANGELOG.md LICENSE      README.md    doc          project.clj  resources    src          test
borkdude@MBP2019 /tmp/foo $ mkdir .clj-kondo
borkdude@MBP2019 /tmp/foo $ clj-kondo --lint src test
test/foo/core_test.clj:2:34: warning: use alias or :refer [deftest is testing]
test/foo/core_test.clj:3:30: warning: use alias or :refer
linting took 41ms, errors: 0, warnings: 2
borkdude@MBP2019 /tmp/foo $ ls -la .clj-kondo
total 0
drwxr-xr-x   3 borkdude  wheel   96 Jan  5 10:11 .
drwxr-xr-x  13 borkdude  wheel  416 Jan  5 10:11 ..
drwxr-xr-x   3 borkdude  wheel   96 Jan  5 10:11 .cache
bpringe commented 3 years ago

Closing this, since we don't plan to allow disabling clojure-lsp. We want to rely more on clojure-lsp as a provider of language features, and in doing so, disabling it would cause Calva to lose certain features. We intend to work closely with clojure-lsp maintainers to continue to improve the experience of it with Calva. For anyone reading this, if you have problems related to clojure-lsp, please file a new issue about the specific problem so we can address it. Thanks!