From f901149de4bc69f7aa4abe00dea13bca99b9c765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Linse?= Date: Mon, 4 Jan 2021 19:04:21 +0100 Subject: [PATCH] state: throttle batched event processing when input is available before, calling vim.schedule() from inside an event would execute the scheduled callback immediately after this event without checking for user input in between. Break event processing whenever user input or an interrupt is available. --- src/nvim/edit.c | 2 +- src/nvim/eval/funcs.c | 3 ++- src/nvim/ex_getln.c | 2 +- src/nvim/normal.c | 2 +- src/nvim/os/input.c | 20 ++++++++++++--- src/nvim/state.c | 28 +++++++++++++++++++++ src/nvim/terminal.c | 2 +- test/functional/plugin/lsp_spec.lua | 1 + test/functional/ui/input_spec.lua | 38 +++++++++++++++++++++++------ 9 files changed, 81 insertions(+), 17 deletions(-) diff --git a/src/nvim/edit.c b/src/nvim/edit.c index 53717229f..68c7438ea 100644 --- a/src/nvim/edit.c +++ b/src/nvim/edit.c @@ -1024,7 +1024,7 @@ static int insert_handle_key(InsertState *s) break; case K_EVENT: // some event - multiqueue_process_events(main_loop.events); + state_handle_k_event(); goto check_pum; case K_COMMAND: // some command diff --git a/src/nvim/eval/funcs.c b/src/nvim/eval/funcs.c index 8c8e0d568..60229e1eb 100644 --- a/src/nvim/eval/funcs.c +++ b/src/nvim/eval/funcs.c @@ -3029,10 +3029,11 @@ static void f_getchar(typval_T *argvars, typval_T *rettv, FunPtr fptr) if (argvars[0].v_type == VAR_UNKNOWN) { // getchar(): blocking wait. + // TODO(bfredl): deduplicate shared logic with state_enter ? if (!(char_avail() || using_script() || input_available())) { (void)os_inchar(NULL, 0, -1, 0, main_loop.events); if (!multiqueue_empty(main_loop.events)) { - multiqueue_process_events(main_loop.events); + state_handle_k_event(); continue; } } diff --git a/src/nvim/ex_getln.c b/src/nvim/ex_getln.c index d470bfb41..5979f4d3a 100644 --- a/src/nvim/ex_getln.c +++ b/src/nvim/ex_getln.c @@ -935,7 +935,7 @@ static int command_line_execute(VimState *state, int key) if (s->c == K_EVENT || s->c == K_COMMAND) { if (s->c == K_EVENT) { - multiqueue_process_events(main_loop.events); + state_handle_k_event(); } else { do_cmdline(NULL, getcmdkeycmd, NULL, DOCMD_NOWAIT); } diff --git a/src/nvim/normal.c b/src/nvim/normal.c index 4d8b11f83..0b4e2e1f2 100644 --- a/src/nvim/normal.c +++ b/src/nvim/normal.c @@ -8103,7 +8103,7 @@ static void nv_event(cmdarg_T *cap) // lists or dicts being used. may_garbage_collect = false; bool may_restart = (restart_edit != 0); - multiqueue_process_events(main_loop.events); + state_handle_k_event(); finish_op = false; if (may_restart) { // Tricky: if restart_edit was set before the handler we are in ctrl-o mode, diff --git a/src/nvim/os/input.c b/src/nvim/os/input.c index 9d6518841..eca245650 100644 --- a/src/nvim/os/input.c +++ b/src/nvim/os/input.c @@ -159,16 +159,28 @@ bool os_char_avail(void) return inbuf_poll(0, NULL) == kInputAvail; } -// Check for CTRL-C typed by reading all available characters. +/// Poll for fast events. `got_int` will be set to `true` if CTRL-C was typed. +/// +/// This invokes a full libuv loop iteration which can be quite costly. +/// Prefer `line_breakcheck()` if called in a busy inner loop. +/// +/// Caller must at least check `got_int` before calling this function again. +/// checking for other low-level input state like `input_available()` might +/// also be relevant (i e to throttle idle processing when user input is +/// available) void os_breakcheck(void) { + if (got_int) { + return; + } + int save_us = updating_screen; // We do not want screen_resize() to redraw here. + // TODO(bfredl): we are already special casing redraw events, is this + // hack still needed? updating_screen++; - if (!got_int) { - loop_poll_events(&main_loop, 0); - } + loop_poll_events(&main_loop, 0); updating_screen = save_us; } diff --git a/src/nvim/state.c b/src/nvim/state.c index b195c1d96..a3c74789d 100644 --- a/src/nvim/state.c +++ b/src/nvim/state.c @@ -75,6 +75,34 @@ getkey: } } +/// process events on main_loop, but interrupt if input is available +/// +/// This should be used to handle K_EVENT in states accepting input +/// otherwise bursts of events can block break checking indefinitely. +void state_handle_k_event(void) +{ + while (true) { + Event event = multiqueue_get(main_loop.events); + if (event.handler) { + event.handler(event.argv); + } + + if (multiqueue_empty(main_loop.events)) { + // don't breakcheck before return, caller should return to main-loop + // and handle input already. + return; + } + + // TODO(bfredl): as an further micro-optimization, we could check whether + // event.handler already checked input. + os_breakcheck(); + if (input_available() || got_int) { + return; + } + } +} + + /// Return true if in the current mode we need to use virtual. bool virtual_active(void) { diff --git a/src/nvim/terminal.c b/src/nvim/terminal.c index 642c44331..f6995cddb 100644 --- a/src/nvim/terminal.c +++ b/src/nvim/terminal.c @@ -457,7 +457,7 @@ static int terminal_execute(VimState *state, int key) case K_EVENT: // We cannot let an event free the terminal yet. It is still needed. s->term->refcount++; - multiqueue_process_events(main_loop.events); + state_handle_k_event(); s->term->refcount--; if (s->term->buf_handle == 0) { s->close = true; diff --git a/test/functional/plugin/lsp_spec.lua b/test/functional/plugin/lsp_spec.lua index 8ac81daee..d5791fbbf 100644 --- a/test/functional/plugin/lsp_spec.lua +++ b/test/functional/plugin/lsp_spec.lua @@ -257,6 +257,7 @@ describe('LSP', function() eq(0, client.resolved_capabilities().text_document_did_change) client.request('shutdown') client.notify('exit') + client.stop() end; on_exit = function(code, signal) eq(0, code, "exit code", fake_lsp_logfile) diff --git a/test/functional/ui/input_spec.lua b/test/functional/ui/input_spec.lua index 9313a3570..ea8968a65 100644 --- a/test/functional/ui/input_spec.lua +++ b/test/functional/ui/input_spec.lua @@ -1,16 +1,18 @@ local helpers = require('test.functional.helpers')(after_each) -local clear, feed_command, nvim = helpers.clear, helpers.feed_command, helpers.nvim +local clear, feed_command = helpers.clear, helpers.feed_command local feed, next_msg, eq = helpers.feed, helpers.next_msg, helpers.eq local command = helpers.command local expect = helpers.expect +local meths = helpers.meths +local exec_lua = helpers.exec_lua local write_file = helpers.write_file local Screen = require('test.functional.ui.screen') -describe('mappings', function() - local cid +before_each(clear) +describe('mappings', function() local add_mapping = function(mapping, send) - local cmd = "nnoremap "..mapping.." :call rpcnotify("..cid..", 'mapped', '" + local cmd = "nnoremap "..mapping.." :call rpcnotify(1, 'mapped', '" ..send:gsub('<', '').."')" feed_command(cmd) end @@ -21,8 +23,6 @@ describe('mappings', function() end before_each(function() - clear() - cid = nvim('get_api_info')[1] add_mapping('', '') add_mapping('', '') add_mapping('', '') @@ -115,7 +115,6 @@ describe('mappings', function() end) describe('input utf sequences that contain CSI/K_SPECIAL', function() - before_each(clear) it('ok', function() feed('i…') expect('…') @@ -129,7 +128,6 @@ describe('input non-printable chars', function() it("doesn't crash when echoing them back", function() write_file("Xtest-overwrite", [[foobar]]) - clear() local screen = Screen.new(60,8) screen:set_default_attr_ids({ [1] = {bold = true, foreground = Screen.colors.Blue1}, @@ -215,3 +213,27 @@ describe('input non-printable chars', function() ]]) end) end) + +describe("event processing and input", function() + it('not blocked by event bursts', function() + meths.set_keymap('', '', "lua vim.rpcnotify(1, 'stop') winning = true ", {noremap=true}) + + exec_lua [[ + winning = false + burst = vim.schedule_wrap(function(tell) + if tell then + vim.rpcnotify(1, 'start') + end + -- Are we winning, son? + if not winning then + burst(false) + end + end) + burst(true) + ]] + + eq({'notification', 'start', {}}, next_msg()) + feed '' + eq({'notification', 'stop', {}}, next_msg()) + end) +end)