From b56b23007664525e13f7f7ec50a9a6adff0bdfd9 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 25 Apr 2021 12:56:34 +0200 Subject: [PATCH] Avoid excessive polling of universal variable file Reproducible at least on Linux, where the "named pipe" universal variable notifier is used: rm -rf build/test/xdg_config XDG_CONFIG_HOME=build/test/xdg_config ./build/fish -c "xterm -e ./build/fish" The child fish reacts to keyboard input with a noticeable initial delay. This is because the universal variable file is polled over a million times, even when I immediately press Control-D. This polling prevents readb() from handling keyboard input. Before commit 939aba02d ("Refactor input_common.cpp:readb"), readb() reacted to keyboard input even when there were universal variable notifications. Restore this behavior, but make sure to call the universal variable notifier after the new "prepare_to_select" logic. Maybe the problem is in the notifier but the old behavior was sane. Fixes the problems described in https://github.com/fish-shell/fish-shell/commit/7a556ec6f21f16b528be3fb94f5160e2209c3cbf#commitcomment-49773677 Adding "-d uvars-file" to the reproducesr shows that we are checking the uvar file repeatedly: uvar-file: universal log sync uvar-file: universal log sync elided based on fast stat() uvar-file: universal log no modifications --- src/input_common.cpp | 9 +++++---- tests/checks/tmux-complete.fish | 3 --- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/input_common.cpp b/src/input_common.cpp index f473720d2..6a025d7a9 100644 --- a/src/input_common.cpp +++ b/src/input_common.cpp @@ -57,7 +57,7 @@ enum { }; using readb_result_t = int; -static readb_result_t readb(int in_fd) { +static readb_result_t readb(int in_fd, bool queue_is_empty) { assert(in_fd >= 0 && "Invalid in fd"); universal_notifier_t& notifier = universal_notifier_t::default_notifier(); select_wrapper_t fdset; @@ -98,7 +98,9 @@ static readb_result_t readb(int in_fd) { // This may come about through readability, or through a call to poll(). if (notifier.poll() || (fdset.test(notifier_fd) && notifier.notification_fd_became_readable(notifier_fd))) { - return readb_uvar_notified; + if (env_universal_barrier() && !queue_is_empty) { + return readb_uvar_notified; + } } // Check stdin. @@ -167,7 +169,7 @@ char_event_t input_event_queue_t::readch() { return mevt.acquire(); } - readb_result_t rr = readb(in_); + readb_result_t rr = readb(in_, queue_.empty()); switch (rr) { case readb_eof: return char_event_type_t::eof; @@ -178,7 +180,6 @@ char_event_t input_event_queue_t::readch() { break; case readb_uvar_notified: - env_universal_barrier(); break; case readb_ioport_notified: diff --git a/tests/checks/tmux-complete.fish b/tests/checks/tmux-complete.fish index 4dd86aed2..28cd8154b 100644 --- a/tests/checks/tmux-complete.fish +++ b/tests/checks/tmux-complete.fish @@ -3,9 +3,6 @@ # Don't run this on GitHub Actions since it's flaky. #REQUIRES: test "$CI" != true -# HACK: THIS IS DISABLED for now, since it doesn't work at the moment. -#REQUIRES: false - # Isolated tmux. set -g tmpdir (mktemp -d) set -g tmux tmux -S $tmpdir/.tmux-socket -f /dev/null