Throttle ShellCheck requests to limit CPU usage

server/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Bash Language Server
 
+## 4.2.2
+
+- Reduce CPU usage by introduce a short execution delay and throttling for ShellCheck tasks https://github.com/bash-lsp/bash-language-server/pull/655
+
 ## 4.2.1
 
 - Add support for resolving loop variables https://github.com/bash-lsp/bash-language-server/pull/653

server/package.json

@@ -3,7 +3,7 @@
   "description": "A language server for Bash",
   "author": "Mads Hartmann",
   "license": "MIT",
-  "version": "4.2.1",
+  "version": "4.2.2",
   "publisher": "mads-hartmann",
   "main": "./out/server.js",
   "typings": "./out/server.d.ts",

server/src/__tests__/server.test.ts

@@ -15,6 +15,12 @@ import { getMockConnection } from '../../../testing/mocks'
 import LspServer from '../server'
 import { CompletionItemDataType } from '../types'
 
+// Skip ShellCheck throttle delay in test cases
+jest.spyOn(global, 'setTimeout').mockImplementation((fn) => {
+  fn()
+  return 0 as any
+})
+
 async function initializeServer(
   { rootPath }: { rootPath?: string } = { rootPath: pathToFileURL(FIXTURE_FOLDER).href },
 ) {

server/src/shellcheck/__tests__/index.test.ts

@@ -7,26 +7,55 @@ import { Linter } from '../index'
 
 const mockConsole = getMockConnection().console
 
+jest.useFakeTimers()
+
 const FIXTURE_DOCUMENT_URI = `file://${FIXTURE_FOLDER}/foo.sh`
 function textToDoc(txt: string) {
   return TextDocument.create(FIXTURE_DOCUMENT_URI, 'bar', 0, txt)
 }
 
+async function getLintingResult({
+  additionalShellCheckArguments = [],
+  cwd,
+  document,
+  executablePath = 'shellcheck',
+  sourcePaths = [],
+}: {
+  additionalShellCheckArguments?: string[]
+  cwd?: string
+  document: TextDocument
+  executablePath?: string
+  sourcePaths?: string[]
+}): Promise<[Awaited<ReturnType<Linter['lint']>>, Linter]> {
+  const linter = new Linter({
+    console: mockConsole,
+    cwd,
+    executablePath,
+  })
+  const promise = linter.lint(document, sourcePaths, additionalShellCheckArguments)
+  jest.runAllTimers()
+  const result = await promise
+  return [result, linter]
+}
+
 describe('linter', () => {
   it('default to canLint to true', () => {
     expect(new Linter({ console: mockConsole, executablePath: 'foo' }).canLint).toBe(true)
   })
 
   it('should set canLint to false when linting fails', async () => {
     const executablePath = '77b4d3f6-c87a-11ec-9b62-a3c90f66d29f'
-    const linter = new Linter({
-      console: mockConsole,
+
+    const [result, linter] = await getLintingResult({
+      document: textToDoc(''),
       executablePath,
     })
-    expect(await linter.lint(textToDoc(''), [])).toEqual({
+
+    expect(result).toEqual({
       codeActions: [],
       diagnostics: [],
     })
+
     expect(linter.canLint).toBe(false)
     expect(mockConsole.warn).toBeCalledWith(
       expect.stringContaining(
@@ -35,15 +64,14 @@ describe('linter', () => {
     )
   })
 
-  it('should lint when shellcheck present', async () => {
+  it('should lint when shellcheck is present', async () => {
     // prettier-ignore
     const shell = [
       '#!/bin/bash',
       'echo $foo',
     ].join('\n')
 
-    const linter = new Linter({ console: mockConsole, executablePath: 'shellcheck' })
-    const result = await linter.lint(textToDoc(shell), [])
+    const [result] = await getLintingResult({ document: textToDoc(shell) })
     expect(result).toMatchInlineSnapshot(`
       Object {
         "codeActions": Array [
@@ -153,25 +181,23 @@ describe('linter', () => {
   })
 
   it('should correctly follow sources with correct cwd', async () => {
-    const linter = new Linter({
-      console: mockConsole,
+    const [result] = await getLintingResult({
       cwd: FIXTURE_FOLDER,
-      executablePath: 'shellcheck',
+      document: FIXTURE_DOCUMENT.SHELLCHECK_SOURCE,
     })
-    const result = await linter.lint(FIXTURE_DOCUMENT.SHELLCHECK_SOURCE, [])
+
     expect(result).toEqual({
       codeActions: [],
       diagnostics: [],
     })
   })
 
   it('should fail to follow sources with incorrect cwd', async () => {
-    const linter = new Linter({
-      console: mockConsole,
+    const [result] = await getLintingResult({
       cwd: path.resolve(path.join(FIXTURE_FOLDER, '../')),
-      executablePath: 'shellcheck',
+      document: FIXTURE_DOCUMENT.SHELLCHECK_SOURCE,
     })
-    const result = await linter.lint(FIXTURE_DOCUMENT.SHELLCHECK_SOURCE, [])
+
     expect(result).toMatchInlineSnapshot(`
       Object {
         "codeActions": Array [],
@@ -222,14 +248,11 @@ describe('linter', () => {
   })
 
   it('should follow sources with incorrect cwd if the execution path is passed', async () => {
-    const linter = new Linter({
-      console: mockConsole,
+    const [result] = await getLintingResult({
       cwd: path.resolve(path.join(FIXTURE_FOLDER, '../')),
-      executablePath: 'shellcheck',
+      document: FIXTURE_DOCUMENT.SHELLCHECK_SOURCE,
+      sourcePaths: [path.resolve(FIXTURE_FOLDER)],
     })
-    const result = await linter.lint(FIXTURE_DOCUMENT.SHELLCHECK_SOURCE, [
-      path.resolve(FIXTURE_FOLDER),
-    ])
     expect(result).toEqual({
       codeActions: [],
       diagnostics: [],

server/src/shellcheck/index.ts

@@ -5,6 +5,7 @@ import { spawn } from 'child_process'
 import * as LSP from 'vscode-languageserver/node'
 import { TextDocument } from 'vscode-languageserver-textdocument'
 
+import { ThrottledDelayer } from '../util/async'
 import { analyzeShebang } from '../util/shebang'
 import { CODE_TO_TAGS, LEVEL_TO_SEVERITY } from './config'
 import {
@@ -15,24 +16,33 @@ import {
 } from './types'
 
 const SUPPORTED_BASH_DIALECTS = ['sh', 'bash', 'dash', 'ksh']
-
+const DEBOUNCE_MS = 300
 type LinterOptions = {
   executablePath: string
   console: LSP.RemoteConsole
   cwd?: string
 }
 
+type LintingResult = {
+  diagnostics: LSP.Diagnostic[]
+  codeActions: LSP.CodeAction[]
+}
+
 export class Linter {
-  public executablePath: string
-  private cwd: string
   private console: LSP.RemoteConsole
+  private cwd: string
+  public executablePath: string
+  private uriToDelayer: {
+    [uri: string]: ThrottledDelayer<LintingResult>
+  }
   private _canLint: boolean
 
   constructor({ console, cwd, executablePath }: LinterOptions) {
-    this.executablePath = executablePath
-    this.cwd = cwd || process.cwd()
     this._canLint = true
     this.console = console
+    this.cwd = cwd || process.cwd()
+    this.executablePath = executablePath
+    this.uriToDelayer = Object.create(null)
   }
 
   public get canLint(): boolean {
@@ -43,11 +53,28 @@ export class Linter {
     document: TextDocument,
     sourcePaths: string[],
     additionalShellCheckArguments: string[] = [],
-  ): Promise<{ diagnostics: LSP.Diagnostic[]; codeActions: LSP.CodeAction[] }> {
+  ): Promise<LintingResult> {
     if (!this._canLint) {
       return { diagnostics: [], codeActions: [] }
     }
 
+    const { uri } = document
+    let delayer = this.uriToDelayer[uri]
+    if (!delayer) {
+      delayer = new ThrottledDelayer<LintingResult>(DEBOUNCE_MS)
+      this.uriToDelayer[uri] = delayer
+    }
+
+    return delayer.trigger(() =>
+      this.executeLint(document, sourcePaths, additionalShellCheckArguments),
+    )
+  }
+
+  private async executeLint(
+    document: TextDocument,
+    sourcePaths: string[],
+    additionalShellCheckArguments: string[] = [],
+  ): Promise<LintingResult> {
     const result = await this.runShellCheck(
       document,
       [...sourcePaths, dirname(fileURLToPath(document.uri))],
@@ -57,6 +84,9 @@ export class Linter {
       return { diagnostics: [], codeActions: [] }
     }
 
+    // Clean up the debounced function
+    delete this.uriToDelayer[document.uri]
+
     return mapShellCheckResult({ document, result })
   }
 
@@ -100,15 +130,15 @@ export class Linter {
       proc.stdout.on('data', (data) => (out += data))
       proc.stderr.on('data', (data) => (err += data))
       proc.stdin.on('error', () => {
-        // XXX: Ignore STDIN errors in case the process ends too quickly, before we try to
+        // NOTE: Ignore STDIN errors in case the process ends too quickly, before we try to
         // write. If we write after the process ends without this, we get an uncatchable EPIPE.
         // This is solved in Node >= 15.1 by the "on('spawn', ...)" event, but we need to
         // support earlier versions.
       })
       proc.stdin.end(documentText)
     })
 
-    // XXX: do we care about exit code? 0 means "ok", 1 possibly means "errors",
+    // NOTE: do we care about exit code? 0 means "ok", 1 possibly means "errors",
     // but the presence of parseable errors in the output is also sufficient to
     // distinguish.
     let exit
@@ -141,6 +171,7 @@ export class Linter {
     return ShellCheckResultSchema.parse(raw)
   }
 }
+
 function mapShellCheckResult({
   document,
   result,