A severe performance issue: textDocument/didOpen called multiple times

[wookayin]

FAQ

• I have checked the FAQ and it didn't resolve my problem.

Issues

• I have checked existing issues and there are no issues with the same problem.

Neovim Version

0.9.0 stable, and HEAD (NVIM v0.10.0-dev-4334+g4e5061dba)

Dev Version?

• I am using a stable Neovim release version, or if I am using a dev version of Neovim I have confirmed that my issue is reproducible on a stable version.

Operating System

macOS

Minimal Config

local root = vim.fn.fnamemodify("./.repro", ":p")

-- set stdpaths to use .repro
for _, name in ipairs({ "config", "data", "state", "cache" }) do
  vim.env[("XDG_%s_HOME"):format(name:upper())] = root .. "/" .. name
end

-- bootstrap lazy
local lazypath = root .. "/plugins/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({ "git", "clone", "--filter=blob:none", "https://github.com/folke/lazy.nvim.git", lazypath, })
end
vim.opt.runtimepath:prepend(lazypath)

local null_ls_config = function()
    local null_ls = require("null-ls")
    -- add only what you need to reproduce your issue
    null_ls.setup({
        sources = {
            ----------------------------------------------------------------------------------
            require('null-ls.builtins.formatting.yapf').with {
                condition = function() return true end,
            },
            require('null-ls.builtins.diagnostics.pylint').with {
                method = null_ls.methods.DIAGNOSTICS_ON_SAVE,
                -- condition = function(utils) return true end,
            },
            require('null-ls.builtins.diagnostics.flake8').with {
                method = null_ls.methods.DIAGNOSTICS_ON_SAVE,
                condition = function(utils) return true end,
            },
            require('null-ls.builtins.formatting.black').with {
                method = null_ls.methods.DIAGNOSTICS_ON_SAVE,
                condition = function(utils) return true end,
            },
            ----------------------------------------------------------------------------------
        },
        debug = true,
    })
end

-- install plugins
local plugins = {
    "folke/tokyonight.nvim",
    {
        "jose-elias-alvarez/null-ls.nvim",
        dependencies = { "nvim-lua/plenary.nvim" },
        config = null_ls_config,
        event = 'UIEnter',
    },
    {
        -- this plugin shows # of textDocument/didOpen handlers running
        "j-hui/fidget.nvim",
        config = function()
            require("fidget").setup {
                text = { spinner = "zip", },
                window = { relative = "win" },
            }
        end,
    }
}
require("lazy").setup(plugins, {
  root = root .. "/plugins",
})
vim.cmd.colorscheme("tokyonight")

Steps to Reproduce

nvim -u minimal_init.lua ANY_PYTHON_FILE.py

Reproducibility Check

• I confirm that my minimal config is based on the minimal_init.lua template and that my issue is reproducible by running nvim --clean -u minimal_init.lua and following the steps above.

Expected Behavior

No null-ls linters should run on the first BufEnter of the python file.

Actual Behavior

One of the linter (black in this example, non-deterministic depending on the source condition and order) is spawned for N times. Here N is as many as the number of null-ls sources registered.

In my in-the-wild config, eight instances of a linter that runs for seconds are spawned -- all the CPU cores hit 100% usage.

Debug Log

Not relevant

Help

Yes

Implementation Help

No response

Requirements

• I have read and followed the instructions above and understand that my issue will be closed if I did not provide the required information.

[wookayin]

Here is some trace information:

• For each of the conditionally registered source, null-ls.client.on_source_change is called. https://github.com/jose-elias-alvarez/null-ls.nvim/blob/main/lua/null-ls/client.lua#L173-L196
• This calls M.retry_add as a BufEnter callback, which makes a synthetic didOpen notification:

  M.notify_client(methods.lsp.DID_OPEN, {
            textDocument = { uri = vim.uri_from_bufnr(bufnr) },
        })

• The above happens N times, where N is the number of sources registered. So DID_OPEN LSP requests were made redundantly for the same buffer (which is unnecessary).
• This invokes null-ls.diagnostics.handler https://github.com/jose-elias-alvarez/null-ls.nvim/blob/main/lua/null-ls/diagnostics.lua#L139-L192, and then triggers N times of the linter call.

There are two problems: (i) DID_OPEN called too many times, and (ii) it's also strange that the linters were marked as method = ON_SAVE so should have not been executed.

I'd like to note that this bug has something to do with conditional registration. When no conditions are used, only one request will be made. The call hierarchy is:

• null-ls.source: on conditional registration, M.register(source) is called
• null-ls.source.register(...) will call require("null-ls.client").on_source_change()

[wookayin]

This seems to be a bit tricky to fix. @jose-elias-alvarez your inputs would be appreciated.

For now, users can use a workaround by monkey-patching the problematic retry_add function before null-ls call:

+ require('null-ls.client').retry_add = require('null-ls.client').try_add

 null_ls.setup {
    ...
 }

[jose-elias-alvarez]

There are two problems: (i) DID_OPEN called too many times, and (ii) it's also strange that the linters were marked as method = ON_SAVE so should have not been executed.

From a user's perspective, DID_OPEN is indeed called too many times, but from the perspective of null-ls' architecture, each newly registered source could affect the diagnostics generated for each buffer. Since null-ls sources only run in response to LSP requests / notifications, we send a synthetic notification, which causes all sources to run again.

As you noted, this is pretty tricky to solve. The easiest solution might be to debounce the DID_OPEN so it's only sent once, even if multiple sources are registered in quick succession.

As for the ON_SAVE issue: DID_OPEN notifications trigger both ON_SAVE and ON_CHANGE linters.

[wookayin]

Thanks or your comments.

As for the ON_SAVE issue: DID_OPEN notifications trigger both ON_SAVE and ON_CHANGE linters.

This makes sense, at least the LSP should lint the file at least when it is opened for the first time.

The easiest solution might be to debounce the DID_OPEN so it's only sent once, even if multiple sources are registered in quick succession.

I agree with this, probably this can be a general solution. For instance, if you save the file frequently while the slow linter is still running, we would still have multiple linter processes running. We'd probably need to make sure each of the registered LSP source runs at most one at a time for a file (buffer).