Run command in another process to avoid freezing neovim

[BrunoMazzo]

Firstly, thanks for the plugin!

I'm working in a massive monorepo and Neovim freezes for around 2 seconds when I try to run one test file. I did a little of digging and found out that go list was producing around 270k lines of output and taking around 2 seconds to run. I was able to test here and verify that it was the reason for most of my freezing time.

Changing from vim.system to async.process.run fixes most of the problem. But I just had to change vim.fn.json_decode to vim.json.decode because the string now was not utf8 anymore.

I still have a small freeze, even with this solution, my investigation is saying that the processing of this long string is the cause, but I didn't find a good solution for it yet. But now the experience is way better.

[BrunoMazzo]

Should this be

if error ~= nil and error ~= "" then

I kept the old logic, but I'm not sure why it was that way

[fredrikaverpil]

Hey @BrunoMazzo and thanks for this!

I understand Neovim froze up on you completely but with this change it is still responsive while waiting for the command to finish?

After having a quick look, I think we can potentially make other optimizations too:

• Set the cwd (in runspec/dir.lua, runspec/file.lua) to the parent dir of the test dir/file, so to avoid traversing too many files. You can see how this is actually done today for runspec/test.lua: perf: more efficient 'go list' $cwd #246
• Limit the JSON output size to only contain the desired data using -f: perf: lower 'go list' JSON payload size #248

   go list -f '{ \
   "Dir": "{{.Dir}}", \
   "ImportPath": "{{.ImportPath}}", \
   "Name": "{{.Name}}", \
   "TestGoFiles": [{{range $i, $f := .TestGoFiles}}{{if ne $i 0}},{{end}}"{{$f}}"{{end}}], \
   "XTestGoFiles": [{{range $i, $f := .XTestGoFiles}}{{if ne $i 0}},{{end}}"{{$f}}"{{end}}], \
   "Module": { "GoMod": "{{.Module.GoMod}}" } \
   }' \
   ./...

Good call on making use of vim.json.decode by the way. It should be faster than vim.fn.json_decode!
I actually added this in as a separate commit: 9c18f08 (#249)

I'm busy during the holidays and somewhat AFK until Jan 8th, but will have a look at this when I get the chance.

[fredrikaverpil]

By the way, let me know if this big monorepo is open source. It's always good to have something real to profile against.

[BrunoMazzo]

Thanks! The branch really helps with the freezing.

Just one thing I noticed. Running the whole file is way slower than just running the top test that wraps all the tests. The difference between the two RunSpec were the cwd used. When you run the whole file we use the go.mod directory path, but when we run only the top test we use the file directory path as cwd. Can we change to use the file directory as cwd when running the whole file?

Unfortunately, the project is a private one.

[fredrikaverpil]

I actually updated the cwd in this PR already: #246

So maybe you are just not yet on that change on your end?
I was planning on releasing that change along with making 'go list' into an asynchronous call.

[fredrikaverpil]

@BrunoMazzo I just released v1.7.2. Can you please confirm that you are still seeing freezing in Neovim due to long-running go list execution?

[BrunoMazzo]

I will test it right now. But just to confirm, the line I was talking about was line

neotest-golang/lua/neotest-golang/runspec/file.lua

Line 97 in c84bac2

cwd = go_mod_folderpath,

and it is still using the go.mod path.

[BrunoMazzo]

I can confirm here that the performance is way better! There is still a small slow down, but nowhere near the long freeze it was. Amazing work!! Thanks a lot!

[fredrikaverpil]

I will test it right now. But just to confirm, the line I was talking about was line

neotest-golang/lua/neotest-golang/runspec/file.lua

Line 97 in c84bac2

cwd = go_mod_folderpath,

and it is still using the go.mod path.

Ah, that cwd is used only for the go test, and it doesn't matter whether we set this to the test's parent dir or the go.mod folder path. You can try changing it in your local installation temporarily if you wish, but I don't think there will be any difference in execution time? 🤔

  --- @type neotest.RunSpec
  local run_spec = {
    command = test_cmd,
-    cwd = go_mod_folderpath,
+    cwd = pos_path_folderpath,
    context = context,
  }

I can confirm here that the performance is way better! There is still a small slow down, but nowhere near the long freeze it was. Amazing work!! Thanks a lot!

Nice to hear! 🎉

In regards to the async calling of go list, I'll look into this a bit later as I'm busy this weekend. Did you see the README (here) about configuring neotest for big projects and avoiding slow-downs?
You can place a local .lazy.lua file in the project root and perform a per-project override of the
Neotest config there, if you wish to avoid a global setting.

[fredrikaverpil]

@BrunoMazzo would you mind testing the perf/async-golist branch again?
Does it make any difference what so ever on your end?

[BrunoMazzo]

@BrunoMazzo would you mind testing the perf/async-golist branch again? Does it make any difference what so ever on your end?

I tested here, there is a small improvement, but not much to be honest. I think the other commits fixed most of the problems. The only noticeable improvement is when I ran all the tests, but not a massive one. There is a still freezing, but way less than before. I usually only run the current file so it is good enough for me. In File tests there is basically a small lag now sometimes, but no freezing, before it was always freezing.

Ah, that cwd is used only for the go test, and it doesn't matter whether we set this to the test's parent dir or the go.mod folder path. You can try changing it in your local installation temporarily if you wish, but I don't think there will be any difference in execution time? 🤔

I don't know why, but there is a massive difference in performance on the project. Selecting the top test and running as the nearest test takes around 4 seconds from the keypress to the icon change to green tick, but running the file as test takes more than one minute. I did changed it locally and it fixes it. But I agree that it shouldn't matter, but my tests are saying that it matters. I will try to dig here a little more to see if I can find any other reason for it.

[BrunoMazzo]

Did you see the README (here) about configuring neotest for big projects and avoiding slow-downs?

Yes, I'm using it.

[fredrikaverpil]

I tested here, there is a small improvement, but not much to be honest. I think the other commits fixed most of the problems.

That's great to hear, that the already merged changes made a difference. What I improved was to limit the work done by go list, limit its JSON output size and also make the JSON decoding a little faster.

I actually don't think I will try to make the go list call asynchronous in that case.

The only noticeable improvement is when I ran all the tests, but not a massive one. There is a still freezing, but way less than before. I usually only run the current file so it is good enough for me. In File tests there is basically a small lag now sometimes, but no freezing, before it was always freezing.

I wonder if this freezing is due to the go list execution or whether it might be because of something else. 🤔

I don't know why, but there is a massive difference in performance on the project.
Selecting the top test and running as the nearest test takes around 4 seconds from the keypress to the icon change to green tick, but running the file as test takes more than one minute. I did changed it locally and it fixes it.

Aha, so you actually experience a significant improvement when you change the cwd path in runspec/file.lua, making the file execution on par with the single test execution?
That's interesting. I wonder if the go test build step is actually faster as a result. 🤔
Maybe the entire Go project is built, taking up the majority of that minute you mentioned.

You can inspect the logs and extract the go test commands that are executed by Neotest and run them manually. Maybe it's possible to add verbosity or inspect how much is being built and how long it takes.

But since this cwd change seem like it matters in big projects, we can definitively change this path permanently - as it technically is fine to it this way too.

[fredrikaverpil]

Please let me know what differences you can see with v1.7.3, which I just released.

[BrunoMazzo]

Please let me know what differences you can see with v1.7.3, which I just released.

Amazing! Now running the file has the same performance than running just the top file! Thank you very much for the work!