diff --git a/CHANGELOG.md b/CHANGELOG.md index 415ae709..e99d1be7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) * chore(deps): Use workspace dependencies (https://github.com/zellij-org/zellij/pull/4085) * build: Don't use default features (https://github.com/zellij-org/zellij/pull/4086) * build: Don't re-export foreign crates (https://github.com/zellij-org/zellij/pull/4087) +* performance(terminal): reduce render count to mitigate flickering issues in apps that don't implement synchronized renders (https://github.com/zellij-org/zellij/pull/4100) ## [0.42.1] - 2025-03-21 * fix(mouse): fix mouse handling in windows terminal (https://github.com/zellij-org/zellij/pull/4076) diff --git a/zellij-server/src/background_jobs.rs b/zellij-server/src/background_jobs.rs index bb428c0e..6a660f09 100644 --- a/zellij-server/src/background_jobs.rs +++ b/zellij-server/src/background_jobs.rs @@ -55,6 +55,7 @@ pub enum BackgroundJob { Vec, // body BTreeMap, // context ), + RenderToClients, Exit, } @@ -74,6 +75,7 @@ impl From<&BackgroundJob> for BackgroundJobContext { BackgroundJob::RunCommand(..) => BackgroundJobContext::RunCommand, BackgroundJob::WebRequest(..) => BackgroundJobContext::WebRequest, BackgroundJob::ReportPluginList(..) => BackgroundJobContext::ReportPluginList, + BackgroundJob::RenderToClients => BackgroundJobContext::ReportPluginList, BackgroundJob::Exit => BackgroundJobContext::Exit, } } @@ -83,6 +85,7 @@ static FLASH_DURATION_MS: u64 = 1000; static PLUGIN_ANIMATION_OFFSET_DURATION_MD: u64 = 500; static SESSION_READ_DURATION: u64 = 1000; static DEFAULT_SERIALIZATION_INTERVAL: u64 = 60000; +static REPAINT_DELAY_MS: u64 = 10; pub(crate) fn background_jobs_main( bus: Bus, @@ -100,6 +103,7 @@ pub(crate) fn background_jobs_main( let last_serialization_time = Arc::new(Mutex::new(Instant::now())); let serialization_interval = serialization_interval.map(|s| s * 1000); // convert to // milliseconds + let last_render_request: Arc>> = Arc::new(Mutex::new(None)); let http_client = HttpClient::builder() // TODO: timeout? @@ -360,6 +364,53 @@ pub(crate) fn background_jobs_main( } }); }, + BackgroundJob::RenderToClients => { + // last_render_request being Some() represents a render request that is pending + // last_render_request is only ever set to Some() if an async task is spawned to + // send the actual render instruction + // + // given this: + // - if last_render_request is None and we received this job, we should spawn an + // async task to send the render instruction and log the current task time + // - if last_render_request is Some(), it means we're currently waiting to render, + // so we should log the render request and do nothing, once the async task has + // finished running, it will check to see if the render time was updated while it + // was running, and if so send this instruction again so the process can start anew + let (should_run_task, current_time) = { + let mut last_render_request = last_render_request.lock().unwrap(); + let should_run_task = last_render_request.is_none(); + let current_time = Instant::now(); + *last_render_request = Some(current_time); + (should_run_task, current_time) + }; + if should_run_task { + task::spawn({ + let senders = bus.senders.clone(); + let last_render_request = last_render_request.clone(); + let task_start_time = current_time; + async move { + task::sleep(std::time::Duration::from_millis(REPAINT_DELAY_MS)).await; + let _ = senders.send_to_screen(ScreenInstruction::Render); + { + let mut last_render_request = last_render_request.lock().unwrap(); + if let Some(last_render_request) = *last_render_request { + if last_render_request > task_start_time { + // another render request was received while we were + // sleeping, schedule this job again so that we can also + // render that request + let _ = senders.send_to_background_jobs( + BackgroundJob::RenderToClients, + ); + } + } + // reset the last_render_request so that the task will be spawned + // again once a new request is received + *last_render_request = None; + } + } + }); + } + }, BackgroundJob::Exit => { for loading_plugin in loading_plugins.values() { loading_plugin.store(false, Ordering::SeqCst); diff --git a/zellij-server/src/output/mod.rs b/zellij-server/src/output/mod.rs index 38a1585b..537e4cf2 100644 --- a/zellij-server/src/output/mod.rs +++ b/zellij-server/src/output/mod.rs @@ -433,7 +433,6 @@ impl Output { client_serialized_render_instructions.push_str(&vte_instruction); } } - serialized_render_instructions.insert(client_id, client_serialized_render_instructions); } Ok(serialized_render_instructions) diff --git a/zellij-server/src/screen.rs b/zellij-server/src/screen.rs index ace0dea7..0932151d 100644 --- a/zellij-server/src/screen.rs +++ b/zellij-server/src/screen.rs @@ -2900,6 +2900,10 @@ pub(crate) fn screen_thread_main( break; } } + let _ = screen + .bus + .senders + .send_to_background_jobs(BackgroundJob::RenderToClients); }, ScreenInstruction::PluginBytes(mut plugin_render_assets) => { for plugin_render_asset in plugin_render_assets.iter_mut() { diff --git a/zellij-server/src/terminal_bytes.rs b/zellij-server/src/terminal_bytes.rs index c6d901c2..7eb19204 100644 --- a/zellij-server/src/terminal_bytes.rs +++ b/zellij-server/src/terminal_bytes.rs @@ -3,7 +3,7 @@ use crate::{ screen::ScreenInstruction, thread_bus::ThreadSenders, }; -use async_std::{future::timeout as async_timeout, task}; +use async_std::task; use std::{ os::unix::io::RawFd, time::{Duration, Instant}, @@ -13,32 +13,12 @@ use zellij_utils::{ logging::debug_to_file, }; -enum ReadResult { - Ok(usize), - Timeout, - Err(std::io::Error), -} - -impl From> for ReadResult { - fn from(e: std::io::Result) -> ReadResult { - match e { - Err(e) => ReadResult::Err(e), - Ok(n) => ReadResult::Ok(n), - } - } -} - pub(crate) struct TerminalBytes { pid: RawFd, terminal_id: u32, senders: ThreadSenders, async_reader: Box, debug: bool, - render_deadline: Option, - backed_up: bool, - minimum_render_send_time: Option, - buffering_pause: Duration, - last_render: Instant, } impl TerminalBytes { @@ -55,11 +35,6 @@ impl TerminalBytes { senders, debug, async_reader: os_input.async_file_reader(pid), - render_deadline: None, - backed_up: false, - minimum_render_send_time: None, - buffering_pause: Duration::from_millis(30), - last_render: Instant::now(), } } pub async fn listen(&mut self) -> Result<()> { @@ -80,25 +55,13 @@ impl TerminalBytes { err_ctx.add_call(ContextType::AsyncTask); let mut buf = [0u8; 65536]; loop { - match self.deadline_read(&mut buf).await { - // EOF - ReadResult::Ok(0) => break, - // Some error occured - ReadResult::Err(err) => { + match self.async_reader.read(&mut buf).await { + Ok(0) => break, // EOF + Err(err) => { log::error!("{}", err); break; }, - ReadResult::Timeout => { - let time_to_send_render = self - .async_send_to_screen(ScreenInstruction::Render) - .await - .with_context(err_context)?; - self.update_render_send_time(time_to_send_render); - // next read does not need a deadline as we just rendered everything - self.render_deadline = None; - self.last_render = Instant::now(); - }, - ReadResult::Ok(n_bytes) => { + Ok(n_bytes) => { let bytes = &buf[..n_bytes]; if self.debug { let _ = debug_to_file(bytes, self.pid); @@ -109,19 +72,6 @@ impl TerminalBytes { )) .await .with_context(err_context)?; - if !self.backed_up { - // we're not backed up, let's send an immediate render instruction - let time_to_send_render = self - .async_send_to_screen(ScreenInstruction::Render) - .await - .with_context(err_context)?; - self.update_render_send_time(time_to_send_render); - self.last_render = Instant::now(); - } - // if we already have a render_deadline we keep it, otherwise we set it - // to buffering_pause since the last time we rendered. - self.render_deadline - .get_or_insert(self.last_render + self.buffering_pause); }, } } @@ -156,44 +106,4 @@ impl TerminalBytes { .context("failed to async-send to screen")?; Ok(sent_at.elapsed()) } - fn update_render_send_time(&mut self, time_to_send_render: Duration) { - match self.minimum_render_send_time.as_mut() { - Some(minimum_render_time) => { - if time_to_send_render < *minimum_render_time { - *minimum_render_time = time_to_send_render; - } - if time_to_send_render > *minimum_render_time * 10 { - // sending the render instruction took an especially long time, we can safely - // assume the screen thread is backed up and we should only send render - // instructions sparingly - self.backed_up = true; - } else if time_to_send_render < *minimum_render_time * 5 { - // the screen thread is not backed up, we atomically unset the backed_up - // indication - self.backed_up = false; - } - }, - None => { - self.minimum_render_send_time = Some(time_to_send_render); - }, - } - } - async fn deadline_read(&mut self, buf: &mut [u8]) -> ReadResult { - if !self.backed_up { - self.async_reader.read(buf).await.into() - } else if let Some(deadline) = self.render_deadline { - let timeout = deadline.checked_duration_since(Instant::now()); - if let Some(timeout) = timeout { - match async_timeout(timeout, self.async_reader.read(buf)).await { - Ok(res) => res.into(), - _ => ReadResult::Timeout, - } - } else { - // deadline has already elapsed - ReadResult::Timeout - } - } else { - self.async_reader.read(buf).await.into() - } - } } diff --git a/zellij-utils/src/errors.rs b/zellij-utils/src/errors.rs index 7c5ccc6d..73e97e02 100644 --- a/zellij-utils/src/errors.rs +++ b/zellij-utils/src/errors.rs @@ -511,6 +511,7 @@ pub enum BackgroundJobContext { RunCommand, WebRequest, ReportPluginList, + RenderToClients, Exit, }