Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: Filetype is applied once for each line when redirecting command output #332

Closed
3 tasks done
jedrzejboczar opened this issue Jan 26, 2023 · 7 comments
Closed
3 tasks done
Labels
bug Something isn't working stale

Comments

@jedrzejboczar
Copy link
Contributor

Did you check docs and existing issues?

  • I have read all the noice.nvim docs
  • I have searched the existing issues of noice.nvim
  • I have searched the exsiting issues of plugins related to this issue

Neovim version (nvim -v)

NVIM v0.9.0-dev-784+g72f0c461a

Operating system/version

Arch Linux

Describe the bug

When using noice.redirect to display command output in a popup, it is very slow (e.g. over 1.2 seconds to display 50 lines on my system). After some debugging I found that it is due to constantly setting filetype for the buffer (View:render -> nui.nvim utils.set_buf_options). This option is being set on each displayed line, which in turn runs the following lines from ftplugin.vim, which is quite slow on my system and takes ~10ms for each call. Tested with:

local name = 'noice'
local start = vim.fn.reltime()
vim.cmd('runtime! ftplugin/' .. name .. '.vim ftplugin/' .. name .. '_*.vim ftplugin/' .. name .. '/*.vim')
vim.cmd('runtime! ftplugin/' .. name .. '.lua ftplugin/' .. name .. '_*.lua ftplugin/' .. name .. '/*.lua')
local elapsed = vim.fn.reltime(start)
print(string.format('Elapsed %.3f ms', vim.fn.reltimefloat(elapsed) * 1000)) -- "Elapsed 9.482 ms"

Steps To Reproduce

While in this repro it will not be slow because there are not many plugins in runtimepath, the following steps will show how many times the filetype option is being set:

  • Use the provided repro.lua: nvim -u repro.lua, then exit
  • Apply the following nui.patch: cd ./.repro/plugins/nui.nvim/ && git apply ../../../nui.patch
diff --git a/lua/nui/utils/init.lua b/lua/nui/utils/init.lua
index b1e4fbe..608039f 100644
--- a/lua/nui/utils/init.lua
+++ b/lua/nui/utils/init.lua
@@ -121,11 +121,17 @@ function _.clear_namespace(bufnr, ns_id, linenr_start, linenr_end)
   vim.api.nvim_buf_clear_namespace(bufnr, ns_id, linenr_start - 1, linenr_end - 1)
 end

+_G.dbg = {}
+
 ---@private
 ---@param bufnr number
 ---@param buf_options table<string, any>
 function _.set_buf_options(bufnr, buf_options)
   for name, value in pairs(buf_options) do
+    local opt_counts = _G.dbg[bufnr] or {}
+    opt_counts.__bufname = vim.api.nvim_buf_get_name(bufnr)
+    opt_counts[name] = (opt_counts[name] or 0) + 1
+    _G.dbg[bufnr] = opt_counts
     vim.api.nvim_buf_set_option(bufnr, name, value)
   end
 end
  • Open nvim nvim -u repro.lua, type :TestCmd and hit <m-cr>
  • It will show the output in a popup
  • Run :lua = _G.dbg and hit <m-cr>
  • It will show how many times filetype has been set for a buffer, which for one buffer should correspond to the number of message lines:
  [22] = {
    __bufname = "",
    buftype = 101,
    filetype = 101
  },

Expected Behavior

Maybe this is an issue with my setup with quite a lot of plugins, or maybe I have some plugin that makes the call to runtime! very slow - I am not sure, I'll try to investigate more. But still I think it should be expected that noice is not setting filetype on every output line.

Repro

-- DO NOT change the paths and don't remove the colorscheme
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)

-- install plugins
local plugins = {
  "folke/tokyonight.nvim",
  "folke/noice.nvim",
  'MunifTanjim/nui.nvim',
  -- add any other plugins here
}
require("lazy").setup(plugins, {
  root = root .. "/plugins",
})

vim.cmd.colorscheme("tokyonight")
-- add anything else here

require('noice').setup {}

local redirect = function()
  require('noice').redirect(vim.fn.getcmdline())
end
vim.keymap.set('c', '<m-cr>', redirect, { desc = 'Redirect Cmdline' })

vim.api.nvim_create_user_command('TestCmd', function()
  local msg = {}
  for i = 1, 100 do
    table.insert(msg, string.rep(' ', i % 10) .. 'Message ' .. i)
  end
  print(table.concat(msg, '\n'))
end, {})
@jedrzejboczar jedrzejboczar added the bug Something isn't working label Jan 26, 2023
Copy link
Contributor

github-actions bot commented Jul 6, 2024

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Jul 6, 2024
Copy link
Contributor

This issue was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 13, 2024
@jedrzejboczar
Copy link
Contributor Author

Just checked that this is still an issue.

folke pushed a commit that referenced this issue Jul 15, 2024
## Description

Using `vim.b` variables seems slow for big tables. This PR improves
performance of command output redirection (and possibly other places).
From my tests this is an order of 20 sec -> 4 sec.

Also, it looks like the current code sometimes assumes that
`vim.b[buf].messages` is a list of buffers, while it is a list of
message ids. I also changed it from a list to a set, because it's mostly
used for random access.

## Related Issue(s)

This helps in case described here:
#332 (filetype is still
applied many times, but from my recent benchmarks it seems that
`vim.b.messages` had bigger impact).
@folke folke reopened this Jul 15, 2024
@folke folke removed the stale label Jul 15, 2024
@folke
Copy link
Owner

folke commented Jul 15, 2024

Are you sure this is still a problem? The code that sets those options is wrapped in eventignore, so it should no longer trigger autocmds

@jedrzejboczar
Copy link
Contributor Author

Ah, you're right. I used the original test, which was just checking that the filetype was being set, but if I create a FileType * autocmd and increment count per buffer, then it's not called.

However, I found another issue. I'm using perfanno.nvim which uses LuaJIT profiler (:PerfLuaProfileStart, do the test, :PerfLuaProfileStop, :PerfHottestLines). It looks like most of the time is spent here:

So I added simple counter _G.dbg[bufnr] = (_G.dbg[bufnr] or 0) + 1 inside line:render and I got an unexpectedly high count. When running =require'noice.config' which produced 965 lines of output I got count of 466095 calls to line:render. This is exactly the sum 1+2+3+...+965 so the complexity is O(n^2), which is explains why printing not so big outputs takes a lot of time.

Is it possible to reimplement command redirection in such a way that it does not always replace the content of the whole buffer? I don't really understand the details of how this works, so I wonder if this is due to some limitation of how message redirection works or maybe there is some simple fix for that?

Copy link
Contributor

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Aug 15, 2024
Copy link
Contributor

This issue was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale
Projects
None yet
Development

No branches or pull requests

2 participants