Linting concurrency should be limited

[AlekSi]

vscode-go version: 0.6.89

This issue is similar to #1890, but I think it is separate. Please close it if it's not.

If I edit and save file faster then my linter (golangci-lint in my case) finishes, another process will be created. Even with custom flags (--concurrency=1 --fast to limit GOMAXPROCS and run only fast linters) several concurrent processes slow down everything.

I think it doesn't make sense to run several linters concurrently. So I propose to SIGTERM previous linter process before spawning a new one or schedule a new process to start after the previous one finishes.

[ramya-rao-a]

This is different than #1890
#1890 was about using a single process to run go build on multiple packages instead of spawning a process to run go build on each package.

Your case is about killing an older linting process before starting a new one.

If you look at https://github.com/Microsoft/vscode-go/blob/0.6.89/src/goLint.ts, you will see that we do indeed cancel the current running linting process before starting a new one. So I dont see why you would see multiple processes running for the linter, unless the specific linter itself is spawning new processes.

Can you try setting your lint tool as golint and see if you see the same?

[AlekSi]

Yes, it's the same with golint. How to reproduce:

1. Open a relatively big Go project.
2. Add to settings

    "go.lintTool": "golint",
    "go.lintOnSave": "workspace",

3. Open a file.
4. Add some spaces to any line, save. Spaces will be erased by gofmt, linter process will be created.
5. Quickly repeat step 4 several times. You will have several concurrent golint processes.

[ramya-rao-a]

Does it also happen when go.lintOnSave is set to package?

[vscodebot]

This issue has been closed automatically because it needs more information and has not had recent activity. Thank you for your contributions.

[AlekSi]

The only reason why it is hard to notice that with go.lintOnSave set to package is because golint is fast for a typical package. Give it a huge package – and the problem will become visible: https://github.com/AlekSi/vscode-go-issue-916 Edit any file in pack/ by adding spaces, save, etc.

aleksi           43993  16.8  0.9  4444352  78444   ??  R     8:35PM   0:01.02 /Users/aleksi/Soft/GOPATH/bin/golint
aleksi           44061  16.2  0.1  4375924  11156   ??  R     8:35PM   0:00.16 /Users/aleksi/Soft/GOPATH/bin/golint
aleksi           44014  13.9  0.4  4376564  34780   ??  R     8:35PM   0:00.75 /Users/aleksi/Soft/GOPATH/bin/golint
aleksi           43984  10.6  1.1  4444672  90156   ??  R     8:35PM   0:01.10 /Users/aleksi/Soft/GOPATH/bin/golint

Please reopen.

[ramya-rao-a]

Is the above after multiple saves or for the single save?

[ramya-rao-a]

I am currently a bit swamped. Do you mind debugging this by running the Go extension from the source? Its pretty straight forward

If you are up for it, then follow the below instructions:

• Build and Debug extension
• For the sake of debugging, disable the other save participants using the go.buildOnSave and go.vetOnSave settings
• https://github.com/Microsoft/vscode-go/blob/0.6.89/src/goLint.ts#L45 is the code that ensures previous processes are killed before creating a new one for linting
• https://github.com/Microsoft/vscode-go/blob/0.6.89/src/util.ts#L603 is the code that actually runs the linter

[AlekSi]

Is the above after multiple saves or for the single save?

After multiple saves.

[karthikraobr]

Is this up for grabs? I can have a look at it, if available.

[AlekSi]

Go ahead! And thank you.

[karthikraobr]

Hey @AlekSi! Even with your sample, I wasn't able to reproduce the issue. The lines following this code always ensure that the lint process is killed, before a new one is spawned.

[nscimerical]

I would like to add that the Replace in Files command ends up spawning multiple golints. For example, I have updated several imports in multiple files which is over 25 files using it. I took a look at the process monitor and it spawned over that many golint processes. Each golint process takes a significant amount of RAM (around 600-700 MB) and also a decent cpu percentage. It always ends up crashing my computer with only 8 GBs of RAM.

My operating system is Solus Linux if that makes a difference.

To reproduce the issue, clone any decent size Go project and execute the Replace in Files command.

[ramya-rao-a]

@mk0a1a Your particular scenario is being tracked in #2202 and is fixed in the latest beta version of this extension where the on save feature is invoked only the file that is in the active editor.

@AlekSi Was your issue also seen only when saving multiple files at once?

[AlekSi]

No, my issues about multiple saves of a single file.

[ramya-rao-a]

@AlekSi We last left things at
#1916 (comment)

Would you have some time to look deeper into why you would have multiple linters running?

[m13253]

Would you have some time to look deeper into why you would have multiple linters running?

Multiple linters are easily triggered when lintOnSave is turned on, and the user save multiple times in a row.
For big projects, the linter usually run for 20-30 seconds. It is very often that user may save twice or more during these seconds.

When multiple linters are running, my VScode barely gets any CPU power, causing the UI running at ~2fps.

[ramya-rao-a]

@m13253 Like I said before, we do have code in place that is meant to cancel processes that were spawned for previous linting attempts when a new linting request is in place.

Can you follow
#1916 (comment) and help us figure out why that doesn't work in the scenario that you mentioned?

[m13253]

@m13253 Like I said before, we do have code in place that is meant to cancel processes that were spawned for previous linting attempts when a new linting request is in place.

Can you follow
#1916 (comment) and help us figure out why that doesn't work in the scenario that you mentioned?

Thank you for your quick response.
Sorry I misunderstood some previous posts. I will try to help debug the code.

---

By the way, I want to propose another logic which doesn't involve canceling or killing, since as long as I know, canceling tokens are somewhat not 100% reliable:

1. Have a queue with length=1, and a flag indicating whether linter is currently running.
2. When linting is requested:
   If the linter is not running, run it;
   Else if the queue is full, clear the queue and push to it;
   Else, push into the queue.
3. When linting is finished:
   If the queue is full, pop from the queue and run it.

This logic is to ensure that if multiple linting requests are requested, the running one is not interrupted (and whose results may be useful), and the excessive ones are suppressed except the latest request.

[ramya-rao-a]

if multiple linting requests are requested ,the running one is not interrupted (and whose results may be useful)

Such results may also be stale if subsequent edits to the file have already fixed the linting warnings.

[stamblerre]

Since this issue has not had much activity for the last year, I will close it in favor of a new umbrella issue for improving VS Code Go's handling of subprocesses: #3044. It will be easier to track improvements with one issue, instead of many hard-to-find ones.

As an alternative, you can use the language server, which handles cancellation.

[stamblerre]

Duplicate of #3044