Skip to content

feat: save unused buffers in buffer pool #536

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.

Sign up for GitHub

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

Merged
lewis6991 merged 4 commits into from
Dec 6, 2024
Merged

feat: save unused buffers in buffer pool #536

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e3fbc2e
feat: save unused buffers in buffer pool
apollo1321 Dec 5, 2024
32f2e2d
fix: codestyle
apollo1321 Dec 5, 2024
940b7ff
fix: address review comments
apollo1321 Dec 5, 2024
fb985fc
fix: don't check all buffers in the pool every time the context is cl…
apollo1321 Dec 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 33 additions & 23 deletions lua/treesitter-context/render.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ local config = require('treesitter-context.config')

local ns = api.nvim_create_namespace('nvim-treesitter-context')

--- List of buffers that are to be deleted.
--- List of free buffers that can be reused.
---@type integer[]
local retired_buffers = {}
local buffer_pool = {}

local MAX_BUFFER_POOL_SIZE = 20

--- @class WindowContext
--- @field context_winid integer? The context window ID.
Expand All @@ -20,15 +22,35 @@ local retired_buffers = {}
local window_contexts = {}

--- @return integer buf
local function create_buf()
local function create_or_get_buf()
for index = #buffer_pool, 1, -1 do
local buf = table.remove(buffer_pool, index)
if api.nvim_buf_is_valid(buf) then
return buf
end
end

local buf = api.nvim_create_buf(false, true)

vim.bo[buf].undolevels = -1
vim.bo[buf].bufhidden = 'wipe'
Copy link
Contributor Author

@apollo1321 apollo1321 Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option deletes the buffer after the window is closed. It appears that the problem existed before multi-window support was introduced.


return buf
end

local function delete_excess_buffers()
if fn.getcmdwintype() ~= '' then
-- Can't delete buffers when the command-line window is open.
return
end

while #buffer_pool > MAX_BUFFER_POOL_SIZE do
local buf = table.remove(buffer_pool, #buffer_pool)
if api.nvim_buf_is_valid(buf) then
api.nvim_buf_delete(buf, { force = true })
end
end
end

--- @param winid integer
--- @param context_winid integer?
--- @param width integer
Expand All @@ -40,7 +62,7 @@ end
local function display_window(winid, context_winid, width, height, col, ty, hl)
if not context_winid then
local sep = config.separator and { config.separator, 'TreesitterContextSeparator' } or nil
context_winid = api.nvim_open_win(create_buf(), false, {
context_winid = api.nvim_open_win(create_or_get_buf(), false, {
win = winid,
relative = 'win',
width = width,
Expand Down Expand Up @@ -305,25 +327,13 @@ local function close(context_winid)
end

local bufnr = api.nvim_win_get_buf(context_winid)
if bufnr ~= nil then
table.insert(retired_buffers, bufnr)
end
if api.nvim_win_is_valid(context_winid) then
api.nvim_win_close(context_winid, true)
end

if fn.getcmdwintype() ~= '' then
-- Can't delete buffers when the command-line window is open.
return
end

-- Delete retired buffers.
for _, retired_bufnr in ipairs(retired_buffers) do
if api.nvim_buf_is_valid(retired_bufnr) then
api.nvim_buf_delete(retired_bufnr, { force = true })
end
api.nvim_win_close(context_winid, true)
if bufnr ~= nil and api.nvim_buf_is_valid(bufnr) then
-- We can't delete the buffer in-place if the pool is full and the command-line window is open.
-- Instead, add the buffer to the pool and let delete_excess_buffers() address this situation.
table.insert(buffer_pool, bufnr)
Copy link
Contributor

@mnzaki mnzaki Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the only entry point to actually inserting a buffer into the pool is here, so can't we avoid need for the delete_excess_buffers() call by simply not inserting a buffer if the pool is at capacity?

Copy link
Contributor Author

@apollo1321 apollo1321 Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't delete buffers in-place, when the command-line window is open, see #507

Copy link
Member

@lewis6991 lewis6991 Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not what is being suggested?

Copy link
Contributor Author

@apollo1321 apollo1321 Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, no. If the pool is at capacity and we don't add a buffer, we'll need to delete one. However, if the command-line window is open, we cannot proceed with the deletion. Regardless, sometimes we have to add the buffer to the pool even if the pool is full. While I could introduce another 'if' branch to handle the case when the pool if full and we are not in command-line mode, I doubt it would enhance performance.

Copy link
Contributor Author

@apollo1321 apollo1321 Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, I'm not certain it's a good idea to check all buffers in the pool within the delete_excess_buffers function. We might just delete the last buffers in the list instead. I don't believe the buffers will become invalid without a reason.

Copy link
Contributor

@mnzaki mnzaki Dec 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah tricky, well the improved implementation should do though!

end
retired_buffers = {}
delete_excess_buffers()
end)
end

Expand Down
Loading