From edab366d3aa0653515731d2696ee2814c3acf62f Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 28 Apr 2019 22:05:03 -0700 Subject: [PATCH] Eliminate the "input callback queue" This was a sort of side channel that was only used to propagate redraws after universal variable changes. We can eliminate it and handle these more directly. --- src/env.cpp | 7 ++++--- src/env.h | 6 ++++-- src/input_common.cpp | 31 ++++++------------------------- src/input_common.h | 10 ++-------- src/reader.cpp | 2 +- 5 files changed, 17 insertions(+), 39 deletions(-) diff --git a/src/env.cpp b/src/env.cpp index 1d38966e9..4a58201c8 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -61,7 +61,7 @@ static latch_t s_universal_variables; /// Getter for universal variables. static env_universal_t *uvars() { return s_universal_variables; } -void env_universal_barrier() { env_stack_t::principal().universal_barrier(); } +bool env_universal_barrier() { return env_stack_t::principal().universal_barrier(); } // A typedef for a set of constant strings. Note our sets are typically on the order of 6 elements, // so we don't bother to sort them. @@ -498,9 +498,9 @@ env_scoped_t::env_scoped_t(std::unique_ptr vars) : vars_(std::move( env_scoped_t::env_scoped_t(env_scoped_t &&) = default; env_scoped_t::~env_scoped_t() = default; -void env_stack_t::universal_barrier() { +bool env_stack_t::universal_barrier() { ASSERT_IS_MAIN_THREAD(); - if (!uvars()) return; + if (!uvars()) return false; callback_data_list_t callbacks; bool changed = uvars()->sync(callbacks); @@ -509,6 +509,7 @@ void env_stack_t::universal_barrier() { } env_universal_callbacks(this, callbacks); + return changed || !callbacks.empty(); } /// If they don't already exist initialize the `COLUMNS` and `LINES` env vars to reasonable diff --git a/src/env.h b/src/env.h index b61b03922..6a1a60c85 100644 --- a/src/env.h +++ b/src/env.h @@ -271,7 +271,8 @@ class env_stack_t final : public env_scoped_t { void pop(); /// Synchronizes all universal variable changes: writes everything out, reads stuff in. - void universal_barrier(); + /// \return true if something changed, false otherwise. + bool universal_barrier(); /// Returns an array containing all exported variables in a format suitable for execv const char *const *export_arr(); @@ -302,7 +303,8 @@ extern bool g_use_posix_spawn; extern bool term_has_xn; // does the terminal have the "eat_newline_glitch" /// Synchronizes all universal variable changes: writes everything out, reads stuff in. -void env_universal_barrier(); +/// \return true if any value changed. +bool env_universal_barrier(); /// Returns true if we think the terminal supports setting its title. bool term_supports_setting_title(); diff --git a/src/input_common.cpp b/src/input_common.cpp index df3bbfcd1..d92d8a233 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -36,11 +36,6 @@ static int wait_on_escape_ms = WAIT_ON_ESCAPE_DEFAULT; /// Events which have been read and returned by the sequence matching code. static std::deque lookahead_list; -// Queue of pairs of (function pointer, argument) to be invoked. Expected to be mostly empty. -typedef std::list> callback_queue_t; -static callback_queue_t callback_queue; -static void input_flush_callbacks(); - static bool has_lookahead() { return !lookahead_list.empty(); } static char_event_t lookahead_pop() { @@ -78,9 +73,6 @@ static char_event_t readb() { bool do_loop; do { - // Flush callbacks. - input_flush_callbacks(); - fd_set fdset; int fd_max = 0; int ioport = iothread_port(); @@ -139,7 +131,12 @@ static char_event_t readb() { barrier_from_readability = notifier.notification_fd_became_readable(notifier_fd); } if (barrier_from_poll || barrier_from_readability) { - env_universal_barrier(); + if (env_universal_barrier()) { + // A variable change may have triggered a repaint, etc. + if (auto mc = lookahead_pop_evt()) { + return *mc; + } + } } if (ioport > 0 && FD_ISSET(ioport, &fdset)) { @@ -245,19 +242,3 @@ char_event_t input_common_readch_timed(bool dequeue_timeouts) { void input_common_queue_ch(char_event_t ch) { lookahead_push_back(ch); } void input_common_next_ch(char_event_t ch) { lookahead_push_front(ch); } - -void input_common_add_callback(std::function callback) { - ASSERT_IS_MAIN_THREAD(); - callback_queue.push_back(std::move(callback)); -} - -static void input_flush_callbacks() { - // We move the queue into a local variable, so that events queued up during a callback don't get - // fired until next round. - ASSERT_IS_MAIN_THREAD(); - callback_queue_t local_queue; - std::swap(local_queue, callback_queue); - for (auto &f : local_queue) { - f(); - } -} diff --git a/src/input_common.h b/src/input_common.h index d7b061fa4..62a31d72a 100644 --- a/src/input_common.h +++ b/src/input_common.h @@ -2,13 +2,11 @@ #ifndef INPUT_COMMON_H #define INPUT_COMMON_H -#include - -#include - #include "common.h" #include "maybe.h" +#include + enum class readline_cmd_t { beginning_of_line, end_of_line, @@ -172,8 +170,4 @@ void input_common_queue_ch(char_event_t ch); /// once). void input_common_next_ch(char_event_t ch); -/// Adds a callback to be invoked at the next turn of the "event loop." The callback function will -/// be invoked and passed arg. -void input_common_add_callback(std::function); - #endif diff --git a/src/reader.cpp b/src/reader.cpp index bec8579e8..fbf784a4d 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -3436,7 +3436,7 @@ void reader_react_to_color_change() { if (!data->repaint_needed || !data->screen_reset_needed) { data->repaint_needed = true; data->screen_reset_needed = true; - input_common_add_callback(reader_repaint_if_needed); + input_common_queue_ch(readline_cmd_t::repaint); } }