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 53717229f6..68c7438ea3 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 8c8e0d568b..60229e1ebc 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 d470bfb418..5979f4d3a0 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 4d8b11f832..0b4e2e1f23 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 9d6518841a..eca245650a 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 b195c1d96b..a3c74789d1 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 642c443318..f6995cddb6 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 8ac81daeef..d5791fbbfa 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 9313a35708..ea8968a653 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)