lsp: improve performance by converting vim.NIL to nil during JSON deserialization

[hrsh7th]

• nvim --version: v0.6.0-dev+32-ge43dbfdd0
• language server name/version: typescript-language-server@0.5.4
• Operating system/version: Mac OS X BigSur

nvim -c ":checkhealth nvim lspconfig"

health#nvim#check ======================================================================== ## Configuration - WARNING: $NVIM_TUI_ENABLE_CURSOR_SHAPE is ignored in Nvim 0.2+ - ADVICE: - Use the 'guicursor' option to configure cursor shape. :help |'guicursor'| - https://github.com/neovim/neovim/wiki/Following-HEAD#20170402

Performance

• OK: Build type: Release

Remote Plugins

• OK: Up to date

terminal

• INFO: key_backspace (kbs) terminfo entry: key_backspace=^H
• INFO: key_dc (kdch1) terminfo entry: key_dc=\E[3~
• INFO: $TERM_PROGRAM='iTerm.app'
• INFO: $COLORTERM='truecolor'

health#lspconfig#check

Checking language server protocol configuration

• INFO: intelephense: configuration checked.
• INFO: clangd: configuration checked.
• INFO: purescriptls: configuration checked.
• INFO: cssls: configuration checked.
• ERROR: jdtls: The given command "vim.NIL/bin/java" is not executable.
• INFO: html: configuration checked.
• INFO: jsonls: configuration checked.
• INFO: sumneko_lua: configuration checked.
• INFO: gopls: configuration checked.
• INFO: efm: configuration checked.
• INFO: vimls: configuration checked.
• INFO: tsserver: configuration checked.
• INFO: rust_analyzer: configuration checked.

Steps to reproduce using nvim -u minimal_init.lua

1. Apply the following patch to runtime/lua/vim/lsp/rpc.lua

patch

diff --git a/runtime/lua/vim/lsp/rpc.lua b/runtime/lua/vim/lsp/rpc.lua
index 4c5f02af9..bc4bda1bb 100644
--- a/runtime/lua/vim/lsp/rpc.lua
+++ b/runtime/lua/vim/lsp/rpc.lua
@@ -11,7 +11,9 @@ local validate, schedule, schedule_wrap = vim.validate, vim.schedule, vim.schedu
 --@param data (table) Data to encode
 --@returns (string) Encoded object
 local function json_encode(data)
+  local t = os.clock()
   local status, result = pcall(vim.fn.json_encode, data)
+  print(string.format("json_decode: %sms", 1000 * (os.clock()-t)))
   if status then
     return result
   else
@@ -68,7 +70,10 @@ end
 --@param v (any) Argument
 --@returns (any)
 local function convert_NIL(v)
-  return recursive_convert_NIL(v, {})
+  local t = os.clock()
+  local r = recursive_convert_NIL(v, {})
+  print(string.format("convert_NIL: %sms", 1000 * (os.clock()-t)))
+  return r
 end
 
 --@private

2. Create sample project with npm

package.json

{
  "name": "sandbox-typescript",
  "main": "index.js",
  "devDependencies": {
    "@babel/cli": "^7.14.5",
    "@babel/core": "^7.14.6",
    "@babel/preset-env": "^7.14.5",
    "@types/date-fns": "^2.6.0",
    "@types/jest": "^26.0.24",
    "@types/lodash": "^4.14.171",
    "@types/node": "^16.3.2",
    "@types/react": "^17.0.14",
    "@types/react-dom": "^17.0.9",
    "babel-loader": "^8.2.2",
    "date-fns": "^2.22.1",
    "jest": "^27.0.6",
    "typescript": "^4.3.5"
  },
  "dependencies": {
    "@emotion/react": "^11.4.0",
    "@fortawesome/fontawesome-svg-core": "^1.2.35",
    "@fortawesome/free-solid-svg-icons": "^5.15.3",
    "@fortawesome/react-fontawesome": "^0.1.14",
    "@material-ui/core": "^4.12.1",
    "@material-ui/icons": "^4.11.2",
    "formik": "^2.2.9",
    "lodash": "^4.17.21",
    "react": "^17.0.2",
    "react-dom": "^17.0.2"
  }
}

3. Create App.tsx under the above sample project's (src/App.tsx)

App.tsx

import React from 'react';

export const App = () => {
  return (
    <>
    </div>
  );
};

4. Invoke v:lua.vim.lsp.omnifunc on the <|> | is cursor

• This step needs a setup typescript-language-server

Actual behaviour

json_decode and convert_NIL will take the time near the 100ms.

screenshot

Expected behaviour

It will be good if these move to another thread or do more quickly.

[hrsh7th]

@mjlbach I've created the issue. If this is not a problem with neovim itself, feel free to close it.

[hrsh7th]

I've added a screenshot for explanation.

[vigoux]

This may be fixed by using cjson as suggested in #14871 ?
We'll follow this and see...

[clason]

It also helps to add a debounce via

  flags = { debounce_text_changes = 100 },

in the server setup call. (I've come across the same issue with nvim-lsp-compl and texlab, and that fixed it immediately.)

[clason]

(We've talked about making that a default -- any opinions?)

[mjlbach]

@clason that doesn't solve the case here, compe will clear our debounced cache on each completion call. The main issue is the size of the json packet from the server when the number of completion candidates is large, #14871 will address @hrsh7th's first point, but it will need a follow-up PR to make it drop nil's during the deserialization process in order to address the second. I think the first is enough that 0.5.1 will have acceptable completion performance for theia (in my testing)

[clason]

Gotcha; sorry for the drive-by comment!

[hrsh7th]

This issue does not relate to compe's implementation because the reproduce step uses v:lua.vim.lsp.omnifunc. (BTW, compe will cache completion response as much as possible).

I think this issue will be a real-world problem only the user using any auto completion engine.

[mjlbach]

@hrsh7th, sorry I didn't mean to imply that compe was unnecessarily flushing the cached text changes, just that any completion plugin will have to do this to provide up to date completion candidates which mean @clason's suggestion will not affect performance in this case (apart from reducing pressure on the language server generally).