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

nu-ls.nvim appears to be quite slow 🤔 #7

Closed
amtoine opened this issue May 3, 2023 · 15 comments
Closed

nu-ls.nvim appears to be quite slow 🤔 #7

amtoine opened this issue May 3, 2023 · 15 comments

Comments

@amtoine
Copy link
Contributor

amtoine commented May 3, 2023

hello there 👋 😋

i've been trying nu-ls.nvim on and off for a few days now and it looks a bit slow to me...
neovim hangs a tiny little on each character change 🤔

did you notice the same behaviour @zioroboco? 😢

here are the changes i use in my kickstart.nvim config to enable it 👍

diff --git a/lua/custom/plugins/null-ls.lua b/lua/custom/plugins/null-ls.lua
new file mode 100644
index 0000000..fcaaa45
--- /dev/null
+++ b/lua/custom/plugins/null-ls.lua
@@ -0,0 +1,10 @@
+return {
+  "jose-elias-alvarez/null-ls.nvim",
+  config = function ()
+    require("null-ls").setup {
+      sources = {
+        require("nu-ls"),
+      },
+    }
+  end
+}
@zioroboco
Copy link
Owner

I hadn't noticed, but I believe you 🤔

I suspect it'd be the completions, since they're presumably running per keystroke and shelling out to nu every time (which I'm sure is very inefficient 😅). It's doing that async, so I wouldn't have expected it to block input, but I might be misunderstanding something about null-ls or neovim...

Assuming that it is some misunderstanding about the async methods, I'll see if I can add a debounce to the completions so that it's not looking stuff up so aggressively while you're actively typing.

@zioroboco
Copy link
Owner

@amtoine I'd be interested to know if the debounce branch helps — I've added a global 250ms timer between the last keystroke and completions being generated by nu. I might be way off about what's causing this, so I don't want to assume that this is the right fix.

Using lazy.nvim you should be able to install it like this:

require("lazy").setup({
  { "zioroboco/nu-ls.nvim", branch = "debounce" },
})

@amtoine
Copy link
Contributor Author

amtoine commented May 5, 2023

amazing, let me try this 😌

@amtoine
Copy link
Contributor Author

amtoine commented May 5, 2023

mm interesting, it's a lot better but when i tap super fast i see the strokes freeze around every 250ms 😮

@amtoine
Copy link
Contributor Author

amtoine commented May 5, 2023

there might a deeper issue, right?
but for now, that's a good enough fix i think 😋

an idea, would it be possible to write reproducible benchmarks for nu-ls?
that would be great to keep track of this 😌

@fdncred
Copy link

fdncred commented May 5, 2023

In the vscode extension, we use this type of debounce for the document validation which is the --ide-check command.

documents.onDidChangeContent(
  (() => {
    const throttledValidateTextDocument = debounce(
      validateTextDocument,
      500,
      false
    );

    return (change) => {
      throttledValidateTextDocument(change.document);
    };
  })()
);

The 500 is 500ms. I totally made up 500ms based on my playing around. I tested between 100ms and 3000ms and for my coding/typing it seemed "right" but I'm not sure if's what should be used.

I'd also add that, the size of the file you're editing may come into play. e.g. I'm pretty sure a 10_000 line script would be slower than a 100 line script, just because of the amount of data traversing back and forth.

@zioroboco
Copy link
Owner

I'd been using this with local scripts all under 200-ish lines, so that might explain why I hadn't run up against this!

I've merged the debounce timer in 23ebad1. Seems to work well enough.

I've left the default at 250ms for no reason other than this is what null-ls uses specifically for diagnostics in insert mode (according to their docs in CONFIG.md). In case anyone is running this against massive files, I've also exposed this value through an optional setup function in c6c1b59.

I'm still not clear on how null-ls is handling functions marked as async. The docs don't make this obvious, so I think finding out might require adding some logging here and manually poking around.

@zioroboco
Copy link
Owner

an idea, would it be possible to write reproducible benchmarks for nu-ls?

A good first step would be a CI pipeline!

I've been flying by the seat of my pants a bit — the unit tests are running in a headless instance of my local neovim (including config 🤪), and the the type checks depend on finding things at specific paths on my filesystem. I'd love it if this wasn't the case, but the issues are obscure (and my neovim dev comparatively weak), so I haven't been able to figure these out with enough efficiency to justify the time.

I'm also torn between spending more time than is strictly necessary on this null-ls implementation specifically, v.s. putting that same effort into reimplementing the language server methods from the nushell binary as part of a standalone language server (see #5). I haven't made any progress on this, but it doesn't seem impossibly difficult either, and wouldn't require going so deep on obscure neovim-specific stuff (which I'm not against in principle, if I could just find the right docs!)

@zioroboco
Copy link
Owner

I'm going to close this — hopefully we've got a good-enough fix for the specific problem raised.

@amtoine
Copy link
Contributor Author

amtoine commented May 7, 2023

@zioroboco
thanks for the work and the quick fix ✨

@amtoine
Copy link
Contributor Author

amtoine commented May 9, 2023

hey @zioroboco it's me again 😋

i really would like to try your tool more, to see what is missing, catch bugs, etc, etc... 💪

but it's just too slow for me...
it's really not practical, even with the debouncing, even though better!
it also freezes quite a lot when working in a large file 😢

i'm thinking of something simple, at least for now 😋
could it be possible to add an option to only trigger nu-ls when saving a file?
that would help a lot using it more!

cheers 👋

@zioroboco
Copy link
Owner

Hey @amtoine, it's a shame that it's that bad. 😞 I wonder if there's anything else going on here besides file size, but I'm aware I'm only using nu for local scripts, so not digging around in any big standard library modules or anything... maybe I should give that a try and see.

I don't have any great ideas for a fix (short of implementing a stand-alone language server, which might allow better optimisation), so in the meantime I've added the ability to enable only specific language server methods (from #troubleshooting in the README):

require("null-ls").setup({
  sources = {
    require("nu-ls").setup({
      methods = {
        "diagnostics_on_open",
        "diagnostics_on_save",
        "hover",
      },
    }),
    -- ...
  },
})

@amtoine
Copy link
Contributor Author

amtoine commented May 19, 2023

@zioroboco
sorry for the late reply, a lot of things on the stack 😱

i've just tried this new config and IT IS SO MUCH BETTER 🤩
i need to try it a bit for some time, but that looks even usable ❤️

i'll keep you updated in the following days 😉
thanks again 😊

@amtoine
Copy link
Contributor Author

amtoine commented May 19, 2023

any idea, btw, to split the following file into two separate? 🤔

-- lua/custom/plugins/null-ls.lua
return {
  "jose-elias-alvarez/null-ls.nvim",
  config = function ()
    require("null-ls").setup {
      sources = {
        require("nu-ls").setup({
          methods = {
            "diagnostics_on_open",
            "diagnostics_on_save",
            "hover",
          },
        })
      },
    }
  end
}

@zioroboco
Copy link
Owner

Updating here for anyone wandering through... I started to notice some completion lag after upgrading to nu v0.80, so I've bumped the default debounce time up to 500ms in 6a7205d which seemed to improve things in my case (there are instructions in the README for modifying this further if needed).

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

No branches or pull requests

3 participants