From 172ba231eec45b2bff30e80eeca2ccb54504cc01 Mon Sep 17 00:00:00 2001 From: Alex Pasmantier <47638216+alexpasmantier@users.noreply.github.com> Date: Sat, 25 Jan 2025 21:17:31 +0100 Subject: [PATCH] perf(async): make overall UI much smoother and snappier (#311) --- .config/config.toml | 2 +- Cargo.lock | 69 ++++++++++++ Cargo.toml | 4 +- benches/main.rs | 7 ++ benches/main/draw.rs | 54 ++++++++++ benches/{ => main}/results_list_benchmark.rs | 3 +- television/app.rs | 4 + television/cli.rs | 2 +- television/config/mod.rs | 2 +- television/render.rs | 105 +++++++++---------- television/television.rs | 37 ++++++- 11 files changed, 228 insertions(+), 61 deletions(-) create mode 100644 benches/main.rs create mode 100644 benches/main/draw.rs rename benches/{ => main}/results_list_benchmark.rs (99%) diff --git a/.config/config.toml b/.config/config.toml index c8675d7..776b1a0 100644 --- a/.config/config.toml +++ b/.config/config.toml @@ -15,7 +15,7 @@ # General settings # ---------------------------------------------------------------------------- -frame_rate = 60 +frame_rate = 60 # DEPRECATED: this option is no longer used tick_rate = 50 [ui] diff --git a/Cargo.lock b/Cargo.lock index a8ecd60..22dd093 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -473,6 +473,7 @@ dependencies = [ "ciborium", "clap", "criterion-plot", + "futures", "is-terminal", "itertools 0.10.5", "num-traits", @@ -485,6 +486,7 @@ dependencies = [ "serde_derive", "serde_json", "tinytemplate", + "tokio", "walkdir", ] @@ -727,6 +729,67 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a0d2fde1f7b3d48b8395d5f2de76c18a528bd6a9cdde438df747bfcba3e05d6f" +[[package]] +name = "futures" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "65bc07b1a8bc7c85c5f2e110c476c7389b4554ba72af57d8445ea63a576b0876" +dependencies = [ + "futures-channel", + "futures-core", + "futures-io", + "futures-sink", + "futures-task", + "futures-util", +] + +[[package]] +name = "futures-channel" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2dff15bf788c671c1934e366d07e30c1814a8ef514e1af724a602e8a2fbe1b10" +dependencies = [ + "futures-core", + "futures-sink", +] + +[[package]] +name = "futures-core" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05f29059c0c2090612e8d742178b0580d2dc940c837851ad723096f87af6663e" + +[[package]] +name = "futures-io" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e5c1b78ca4aae1ac06c48a526a655760685149f0d465d21f37abfe57ce075c6" + +[[package]] +name = "futures-sink" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e575fab7d1e0dcb8d0c7bcf9a63ee213816ab51902e6d244a95819acacf1d4f7" + +[[package]] +name = "futures-task" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f90f7dce0722e95104fcb095585910c0977252f286e354b5e3bd38902cd99988" + +[[package]] +name = "futures-util" +version = "0.3.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9fa08315bb612088cc391249efdc3bc77536f16c91f6cf495e6fbe85b20a4a81" +dependencies = [ + "futures-core", + "futures-sink", + "futures-task", + "pin-project-lite", + "pin-utils", +] + [[package]] name = "gag" version = "1.0.0" @@ -1279,6 +1342,12 @@ version = "0.2.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" +[[package]] +name = "pin-utils" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" + [[package]] name = "pkg-config" version = "0.3.31" diff --git a/Cargo.toml b/Cargo.toml index 27c7daa..c8e489b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -69,7 +69,7 @@ toml = "0.8" winapi-util = "0.1.9" [dev-dependencies] -criterion = "0.5" +criterion = { version = "0.5", features = ["async_tokio"] } [features] simd = ["dep:simdutf8"] @@ -82,7 +82,7 @@ path = "television/main.rs" name = "tv" [[bench]] -name = "results_list_benchmark" +name = "main" harness = false [target.'cfg(target_os = "macos")'.dependencies] diff --git a/benches/main.rs b/benches/main.rs new file mode 100644 index 0000000..8f47542 --- /dev/null +++ b/benches/main.rs @@ -0,0 +1,7 @@ +pub mod main { + pub mod draw; + pub mod results_list_benchmark; +} +pub use main::*; + +criterion::criterion_main!(results_list_benchmark::benches, draw::benches,); diff --git a/benches/main/draw.rs b/benches/main/draw.rs new file mode 100644 index 0000000..238e272 --- /dev/null +++ b/benches/main/draw.rs @@ -0,0 +1,54 @@ +use criterion::{black_box, Criterion}; +use ratatui::backend::TestBackend; +use ratatui::layout::Rect; +use ratatui::Terminal; +use std::path::PathBuf; +use television::channels::OnAir; +use television::channels::{files::Channel, TelevisionChannel}; +use television::config::Config; +use television::television::Television; +use tokio::runtime::Runtime; + +fn draw(c: &mut Criterion) { + let width = 250; + let height = 80; + + let rt = Runtime::new().unwrap(); + + c.bench_function("draw", |b| { + b.to_async(&rt).iter_batched( + // FIXME: this is kind of hacky + || { + let config = Config::new().unwrap(); + let backend = TestBackend::new(width, height); + let terminal = Terminal::new(backend).unwrap(); + let mut channel = + TelevisionChannel::Files(Channel::new(vec![ + PathBuf::from("."), + ])); + channel.find("television"); + // Wait for the channel to finish loading + for _ in 0..5 { + // tick the matcher + let _ = channel.results(10, 0); + std::thread::sleep(std::time::Duration::from_millis(10)); + } + let mut tv = Television::new(channel, config, None); + tv.select_next_entry(10); + (tv, terminal) + }, + // Measurement + |(mut tv, mut terminal)| async move { + tv.draw( + black_box(&mut terminal.get_frame()), + black_box(Rect::new(0, 0, width, height)), + ) + .unwrap(); + }, + criterion::BatchSize::SmallInput, + ); + }); +} + +criterion::criterion_group!(benches, draw); +criterion::criterion_main!(benches); diff --git a/benches/results_list_benchmark.rs b/benches/main/results_list_benchmark.rs similarity index 99% rename from benches/results_list_benchmark.rs rename to benches/main/results_list_benchmark.rs index f781e5f..88f5cf8 100644 --- a/benches/results_list_benchmark.rs +++ b/benches/main/results_list_benchmark.rs @@ -1,4 +1,4 @@ -use criterion::{criterion_group, criterion_main, Criterion}; +use criterion::{criterion_group, Criterion}; use devicons::FileIcon; use ratatui::layout::Alignment; use ratatui::prelude::{Line, Style}; @@ -664,4 +664,3 @@ pub fn results_list_benchmark(c: &mut Criterion) { } criterion_group!(benches, results_list_benchmark); -criterion_main!(benches); diff --git a/television/app.rs b/television/app.rs index 30329b0..8077d21 100644 --- a/television/app.rs +++ b/television/app.rs @@ -160,6 +160,7 @@ impl App { ) .await }); + render_tx.send(RenderingTask::Render)?; // event handling loop debug!("Starting event handling loop"); @@ -169,6 +170,9 @@ impl App { if let Some(event) = self.event_rx.recv().await { let action = self.convert_event_to_action(event).await; action_tx.send(action)?; + // it's fine to send a rendering task here, because the rendering loop + // batches and deduplicates rendering tasks + render_tx.send(RenderingTask::Render)?; } let action_outcome = self.handle_actions().await?; diff --git a/television/cli.rs b/television/cli.rs index 9cc1fbc..946d116 100644 --- a/television/cli.rs +++ b/television/cli.rs @@ -38,7 +38,7 @@ pub struct Cli { #[arg(short, long, value_name = "FLOAT")] pub tick_rate: Option, - /// Frame rate, i.e. number of frames per second + /// [DEPRECATED] Frame rate, i.e. number of frames per second #[arg(short, long, value_name = "FLOAT")] pub frame_rate: Option, diff --git a/television/config/mod.rs b/television/config/mod.rs index d8bc1c8..3448def 100644 --- a/television/config/mod.rs +++ b/television/config/mod.rs @@ -36,7 +36,7 @@ pub struct AppConfig { } #[allow(dead_code)] -#[derive(Clone, Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize, Default)] #[serde(deny_unknown_fields)] pub struct Config { /// General application configuration diff --git a/television/render.rs b/television/render.rs index 2bf21d9..6d4f448 100644 --- a/television/render.rs +++ b/television/render.rs @@ -6,15 +6,12 @@ use std::{ }; use tracing::{debug, warn}; -use tokio::{ - select, - sync::{mpsc, Mutex}, -}; +use tokio::sync::{mpsc, Mutex}; use crate::television::Television; use crate::{action::Action, tui::Tui}; -#[derive(Debug)] +#[derive(Debug, PartialEq, PartialOrd, Ord, Eq, Clone)] pub enum RenderingTask { ClearScreen, Render, @@ -64,60 +61,62 @@ pub async fn render( .await .register_action_handler(action_tx.clone())?; - // Rendering loop - loop { - select! { - () = tokio::time::sleep(tokio::time::Duration::from_secs_f64(1.0 / frame_rate)) => { - action_tx.send(Action::Render)?; - } - maybe_task = render_rx.recv() => { - if let Some(task) = maybe_task { - match task { - RenderingTask::ClearScreen => { - tui.terminal.clear()?; - } - RenderingTask::Render => { - let mut television = television.lock().await; - if let Ok(size) = tui.size() { - // Ratatui uses `u16`s to encode terminal dimensions and its - // content for each terminal cell is stored linearly in a - // buffer with a `u16` index which means we can't support - // terminal areas larger than `u16::MAX`. - if size.width.checked_mul(size.height).is_some() { - tui.terminal.draw(|frame| { - if let Err(err) = television.draw(frame, frame.area()) { - warn!("Failed to draw: {:?}", err); - let _ = action_tx - .send(Action::Error(format!("Failed to draw: {err:?}"))); - } - })?; + let mut buffer = Vec::with_capacity(128); - } else { - warn!("Terminal area too large"); + // Rendering loop + 'rendering: while render_rx.recv_many(&mut buffer, 128).await > 0 { + // deduplicate events + buffer.sort_unstable(); + buffer.dedup(); + for event in buffer.drain(..) { + match event { + RenderingTask::ClearScreen => { + tui.terminal.clear()?; + } + RenderingTask::Render => { + if let Ok(size) = tui.size() { + // Ratatui uses `u16`s to encode terminal dimensions and its + // content for each terminal cell is stored linearly in a + // buffer with a `u16` index which means we can't support + // terminal areas larger than `u16::MAX`. + if size.width.checked_mul(size.height).is_some() { + let mut television = television.lock().await; + tui.terminal.draw(|frame| { + if let Err(err) = + television.draw(frame, frame.area()) + { + warn!("Failed to draw: {:?}", err); + let _ = action_tx.send(Action::Error( + format!("Failed to draw: {err:?}"), + )); } - } - } - RenderingTask::Resize(w, h) => { - tui.resize(Rect::new(0, 0, w, h))?; - action_tx.send(Action::Render)?; - } - RenderingTask::Suspend => { - tui.suspend()?; - action_tx.send(Action::Resume)?; - action_tx.send(Action::ClearScreen)?; - tui.enter()?; - } - RenderingTask::Resume => { - tui.enter()?; - } - RenderingTask::Quit => { - debug!("Exiting rendering loop"); - tui.exit()?; - break Ok(()); + })?; + } else { + warn!("Terminal area too large"); } } } + RenderingTask::Resize(w, h) => { + tui.resize(Rect::new(0, 0, w, h))?; + action_tx.send(Action::Render)?; + } + RenderingTask::Suspend => { + tui.suspend()?; + action_tx.send(Action::Resume)?; + action_tx.send(Action::ClearScreen)?; + tui.enter()?; + } + RenderingTask::Resume => { + tui.enter()?; + } + RenderingTask::Quit => { + debug!("Exiting rendering loop"); + tui.exit()?; + break 'rendering; + } } } } + + Ok(()) } diff --git a/television/television.rs b/television/television.rs index 3ab0806..61d91b6 100644 --- a/television/television.rs +++ b/television/television.rs @@ -280,6 +280,36 @@ impl Television { Ok(()) } + fn should_render(&self, action: &Action) -> bool { + matches!( + action, + Action::AddInputChar(_) + | Action::DeletePrevChar + | Action::DeletePrevWord + | Action::DeleteNextChar + | Action::GoToPrevChar + | Action::GoToNextChar + | Action::GoToInputStart + | Action::GoToInputEnd + | Action::ToggleSelectionDown + | Action::ToggleSelectionUp + | Action::ConfirmSelection + | Action::SelectNextEntry + | Action::SelectPrevEntry + | Action::SelectNextPage + | Action::SelectPrevPage + | Action::ScrollPreviewDown + | Action::ScrollPreviewUp + | Action::ScrollPreviewHalfPageDown + | Action::ScrollPreviewHalfPageUp + | Action::ToggleRemoteControl + | Action::ToggleSendToChannel + | Action::ToggleHelp + | Action::TogglePreview + | Action::CopyEntryToClipboard + ) || self.channel.running() + } + #[allow(clippy::unused_async)] /// Update the state of the component based on a received action. /// @@ -447,7 +477,12 @@ impl Television { } _ => {} } - Ok(None) + + Ok(if self.should_render(&action) { + Some(Action::Render) + } else { + None + }) } /// Render the television on the screen.