Refactor main TUI loop to reduce latency in handling messages

Previously, the main loop was rate-limited by the terminal input poll() call. This means when no user input was present, it would block for 250ms before handling any async messages. This could cause some annoying UI delays for changes triggered by background IO (e.g. HTTP responses could take up to 250ms *extra* to appear). By breaking the input loop into a separate task, and having the main loop block directly on the message queue, we ensure that every message is handled immediately and we update the UI immediately.

I measured CPU usage on this, and it's the same as master. Locally I see ~5% usage whlie running in debug and ~0.7% while running with --release. This makes sense because the CPU is doing all the same work, just rearranged slightly.

This has the added benefit that it accidentally fixed #136, which is great because I really didn't want to debug that anymore.

Cargo.toml

@@ -20,7 +20,7 @@ bytesize = {version = "1.3.0", default-features = false}
 chrono = {version = "^0.4.31", default-features = false, features = ["clock", "serde", "std"]}
 clap = {version = "^4.4.2", features = ["derive"]}
 cli-clipboard = "0.4.0"
-crossterm = "^0.27.0"
+crossterm = {version = "^0.27.0", features = ["event-stream"]}
 derive_more = {version = "1.0.0-beta.6", features = ["debug", "deref", "deref_mut", "display", "from", "from_str"]}
 dialoguer = {version = "^0.11.0", default-features = false, features = ["password"]}
 dirs = "^5.0.1"

src/tui.rs

@@ -12,7 +12,7 @@ use crate::{
     template::{Prompter, Template, TemplateChunk, TemplateContext},
     tui::{
         context::TuiContext,
-        input::Action,
+        input::{Action, InputEngine},
         message::{Message, MessageSender, RequestConfig},
         signal::signals,
         view::{ModalPriority, PreviewPrompter, RequestState, View},
@@ -21,31 +21,27 @@ use crate::{
 };
 use anyhow::{anyhow, Context};
 use crossterm::{
-    event::{DisableMouseCapture, EnableMouseCapture},
+    event::{DisableMouseCapture, EnableMouseCapture, EventStream},
     terminal::{EnterAlternateScreen, LeaveAlternateScreen},
 };
-use futures::Future;
+use futures::{Future, StreamExt};
 use notify::{event::ModifyKind, RecursiveMode, Watcher};
 use ratatui::{prelude::CrosstermBackend, Terminal};
 use std::{
     io::{self, Stdout},
     ops::Deref,
     path::PathBuf,
     sync::{Arc, OnceLock},
-    time::{Duration, Instant},
+    time::Duration,
+};
+use tokio::{
+    sync::mpsc::{self, UnboundedReceiver},
+    time,
 };
-use tokio::sync::mpsc::{self, UnboundedReceiver};
 use tracing::{debug, error, info, trace};
 
-/// Main controller struct for the TUI. The app uses a React-like architecture
-/// for the view, with a wrapping controller (this struct). The main loop goes
-/// through the following phases on each iteration:
-///
-/// - Input phase: Check for input from the user
-/// - Message phase: Process any async messages from input or external sources
-///   (HTTP, file system, etc.)
-/// - Event phase: Handle any *view* events that have been queued up
-/// - Draw phase: Draw the entire UI
+/// Main controller struct for the TUI. The app uses a React-ish architecture
+/// for the view, with a wrapping controller (this struct)
 #[derive(Debug)]
 pub struct Tui {
     terminal: Term,
@@ -61,7 +57,7 @@ pub struct Tui {
 type Term = Terminal<CrosstermBackend<Stdout>>;
 
 impl Tui {
-    /// Rough maximum time for each iteration of the main loop
+    /// Rough **maximum** time for each iteration of the main loop
     const TICK_TIME: Duration = Duration::from_millis(250);
 
     /// Start the TUI. Any errors that occur during startup will be panics,
@@ -109,44 +105,39 @@ impl Tui {
             view: Replaceable::new(view),
         };
 
-        app.run()
+        app.run().await
     }
 
     /// Run the main TUI update loop. Any error returned from this is fatal. See
     /// the struct definition for a description of the different phases of the
     /// run loop.
-    fn run(mut self) -> anyhow::Result<()> {
+    async fn run(mut self) -> anyhow::Result<()> {
         self.listen_for_signals();
         // Hang onto this because it stops running when dropped
         let _watcher = self.watch_collection()?;
 
-        let mut last_tick = Instant::now();
+        // Listen for input in a separate task
+        tokio::spawn(Self::input_loop());
 
+        // This loop is limited by the rate that messages come in, with a
+        // minimum rate enforced by a timeout
         while self.should_run {
-            // ===== Input Phase =====
-            let timeout = Self::TICK_TIME
-                .checked_sub(last_tick.elapsed())
-                .unwrap_or_else(|| Duration::from_secs(0));
-            // This is where the tick rate is enforced
-            if crossterm::event::poll(timeout)? {
-                // Forward input to the view. Include the raw event for text
-                // editors and such
-                let event = crossterm::event::read()?;
-                let action = TuiContext::get().input_engine.action(&event);
-                if let Some(Action::ForceQuit) = action {
-                    // Short-circuit the view/message cycle, to make sure this
-                    // doesn't get ate
-                    self.quit();
-                } else {
-                    self.view.handle_input(event, action);
-                }
-            }
-            if last_tick.elapsed() >= Self::TICK_TIME {
-                last_tick = Instant::now();
-            }
+            // ===== Draw Phase =====
+            // Draw *first* so initial UI state is rendered immediately
+            self.terminal.draw(|f| self.view.draw(f))?;
 
             // ===== Message Phase =====
-            while let Ok(message) = self.messages_rx.try_recv() {
+            // Grab one message out of the queue and handle it. This will block
+            // while the queue is empty so we don't waste CPU cycles. The
+            // timeout here makes sure we don't block forever, so thinks like
+            // time displays during in-flight requests will update.
+            let future =
+                time::timeout(Self::TICK_TIME, self.messages_rx.recv());
+            if let Ok(message) = future.await {
+                // Error would indicate a very weird and fatal bug so we wanna
+                // know about it
+                let message =
+                    message.expect("Message channel dropped while running");
                 trace!(?message, "Handling message");
                 // If an error occurs, store it so we can show the user
                 if let Err(error) = self.handle_message(message) {
@@ -157,14 +148,37 @@ impl Tui {
             // ===== Event Phase =====
             // Let the view handle all queued events
             self.view.handle_events();
-
-            // ===== Draw Phase =====
-            self.terminal.draw(|f| self.view.draw(f))?;
         }
 
         Ok(())
     }
 
+    /// Blocking loop to read input from the terminal. Each event is pushed
+    /// into the message queue for handling by the main loop.
+    async fn input_loop() {
+        let mut stream = EventStream::new();
+
+        while let Some(result) = stream.next().await {
+            // Failure to read input is both exceptional and terminal, so panic
+            let event = result.expect("Error reading terminal input");
+
+            // Filter out junk events so we don't clog the message queue
+            if InputEngine::should_kill(&event) {
+                continue;
+            }
+
+            let action = TuiContext::get().input_engine.action(&event);
+            if let Some(Action::ForceQuit) = action {
+                // Short-circuit the view/message cycle, to make sure this
+                // doesn't get ate
+                TuiContext::send_message(Message::Quit);
+                break;
+            } else {
+                TuiContext::send_message(Message::Input { event, action });
+            }
+        }
+    }
+
     /// Handle an incoming message. Any error here will be displayed as a modal
     fn handle_message(&mut self, message: Message) -> anyhow::Result<()> {
         match message {
@@ -243,6 +257,10 @@ impl Tui {
                 self.view.set_request_state(profile_id, recipe_id, state);
             }
 
+            Message::Input { event, action } => {
+                self.view.handle_input(event, action);
+            }
+
             Message::RequestLoad {
                 profile_id,
                 recipe_id,

src/tui/input.rs

@@ -101,6 +101,22 @@ impl InputEngine {
         // ^^^^^ If making changes, make sure to update the docs ^^^^^
     ]);
 
+    /// Should this event immediately be killed? This means it will never be
+    /// handled by a component. This is used to filter out junk events that we
+    /// don't care about, to prevent clogging up the message queue
+    pub fn should_kill(event: &Event) -> bool {
+        matches!(
+            event,
+            Event::FocusGained
+                | Event::FocusLost
+                | Event::Resize(_, _)
+                | Event::Mouse(MouseEvent {
+                    kind: MouseEventKind::Moved,
+                    ..
+                })
+        )
+    }
+
     pub fn new(user_bindings: IndexMap<Action, InputBinding>) -> Self {
         let mut new = Self::default();
         // User bindings should overwrite any default ones

src/tui/message.rs

@@ -7,6 +7,7 @@ use crate::{
         RecipeOptions, Request, RequestBuildError, RequestError, RequestRecord,
     },
     template::{Prompt, Prompter, Template, TemplateChunk},
+    tui::input::Action,
     util::ResultExt,
 };
 use anyhow::Context;
@@ -89,6 +90,14 @@ pub enum Message {
     /// these two cases saves a bit of boilerplate.
     HttpComplete(Result<RequestRecord, RequestError>),
 
+    /// User input from the terminal
+    Input {
+        /// Raw input event
+        event: crossterm::event::Event,
+        /// Action mapped via input bindings. This is what most consumers use
+        action: Option<Action>,
+    },
+
     /// Show a prompt to the user, asking for some input. Use the included
     /// channel to return the value.
     PromptStart(Prompt),

src/tui/view.rs

@@ -112,12 +112,6 @@ impl View {
     pub fn handle_events(&mut self) {
         // It's possible for components to queue additional events
         while let Some(event) = EventQueue::pop() {
-            // Certain events *just don't matter*, AT ALL. They're not even
-            // supposed to be around, like, in the area
-            if event.should_kill() {
-                continue;
-            }
-
             trace_span!("View event", ?event).in_scope(|| {
                 match Self::update_all(self.root.as_child(), event) {
                     Update::Consumed => {

src/tui/view/event.rs

@@ -12,7 +12,6 @@ use crate::{
         },
     },
 };
-use crossterm::event::{MouseEvent, MouseEventKind};
 use std::{any::Any, cell::RefCell, collections::VecDeque, fmt::Debug};
 use tracing::trace;
 
@@ -155,26 +154,6 @@ impl Event {
 }
 
 impl Event {
-    /// Should this event immediately be killed, meaning it will never be
-    /// handled by a component. This is used to filter out junk events that will
-    /// never be handled, mostly to make debug logging cleaner.
-    pub fn should_kill(&self) -> bool {
-        use crossterm::event::Event;
-        matches!(
-            self,
-            Self::Input {
-                event: Event::FocusGained
-                    | Event::FocusLost
-                    | Event::Resize(_, _)
-                    | Event::Mouse(MouseEvent {
-                        kind: MouseEventKind::Moved,
-                        ..
-                    }),
-                ..
-            }
-        )
-    }
-
     /// Is this event pertinent to the component? Most events should be handled,
     /// but some (e.g. cursor events) need to be selectively filtered
     pub fn should_handle<T>(&self, component: &Component<T>) -> bool {