mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
perf(tui): cap redraw scheduling to 60fps (#8499)
Clamp frame draw notifications in the `FrameRequester` scheduler so we don't redraw more frequently than a user can perceive. This applies to both `codex-tui` and `codex-tui2`, and keeps the draw/dispatch loops simple by centralizing the rate limiting in a small helper module. - Add `FrameRateLimiter` (pure, unit-tested) to clamp draw deadlines - Apply the limiter in the scheduler before emitting `TuiEvent::Draw` - Use immediate redraw requests for scroll paths (scheduler now coalesces + clamps) - Add scheduler tests covering immediate/delayed interactions
This commit is contained in:
committed by
GitHub
Unverified
parent
40de81e7af
commit
96a65ff0ed
@@ -47,6 +47,7 @@ use crate::tui::event_stream::TuiEventStream;
|
||||
use crate::tui::job_control::SuspendContext;
|
||||
|
||||
mod event_stream;
|
||||
mod frame_rate_limiter;
|
||||
mod frame_requester;
|
||||
#[cfg(unix)]
|
||||
mod job_control;
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
//! Limits how frequently frame draw notifications may be emitted.
|
||||
//!
|
||||
//! Widgets sometimes call `FrameRequester::schedule_frame()` more frequently than a user can
|
||||
//! perceive. This limiter clamps draw notifications to a maximum of 60 FPS to avoid wasted work.
|
||||
//!
|
||||
//! This is intentionally a small, pure helper so it can be unit-tested in isolation and used by
|
||||
//! the async frame scheduler without adding complexity to the app/event loop.
|
||||
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
/// A 60 FPS minimum frame interval (≈16.67ms).
|
||||
pub(super) const MIN_FRAME_INTERVAL: Duration = Duration::from_nanos(16_666_667);
|
||||
|
||||
/// Remembers the most recent emitted draw, allowing deadlines to be clamped forward.
|
||||
#[derive(Debug, Default)]
|
||||
pub(super) struct FrameRateLimiter {
|
||||
last_emitted_at: Option<Instant>,
|
||||
}
|
||||
|
||||
impl FrameRateLimiter {
|
||||
/// Returns `requested`, clamped forward if it would exceed the maximum frame rate.
|
||||
pub(super) fn clamp_deadline(&self, requested: Instant) -> Instant {
|
||||
let Some(last_emitted_at) = self.last_emitted_at else {
|
||||
return requested;
|
||||
};
|
||||
let min_allowed = last_emitted_at
|
||||
.checked_add(MIN_FRAME_INTERVAL)
|
||||
.unwrap_or(last_emitted_at);
|
||||
requested.max(min_allowed)
|
||||
}
|
||||
|
||||
/// Records that a draw notification was emitted at `emitted_at`.
|
||||
pub(super) fn mark_emitted(&mut self, emitted_at: Instant) {
|
||||
self.last_emitted_at = Some(emitted_at);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn default_does_not_clamp() {
|
||||
let t0 = Instant::now();
|
||||
let limiter = FrameRateLimiter::default();
|
||||
assert_eq!(limiter.clamp_deadline(t0), t0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clamps_to_min_interval_since_last_emit() {
|
||||
let t0 = Instant::now();
|
||||
let mut limiter = FrameRateLimiter::default();
|
||||
|
||||
assert_eq!(limiter.clamp_deadline(t0), t0);
|
||||
limiter.mark_emitted(t0);
|
||||
|
||||
let too_soon = t0 + Duration::from_millis(1);
|
||||
assert_eq!(limiter.clamp_deadline(too_soon), t0 + MIN_FRAME_INTERVAL);
|
||||
}
|
||||
}
|
||||
@@ -18,6 +18,8 @@ use std::time::Instant;
|
||||
use tokio::sync::broadcast;
|
||||
use tokio::sync::mpsc;
|
||||
|
||||
use super::frame_rate_limiter::FrameRateLimiter;
|
||||
|
||||
/// A requester for scheduling future frame draws on the TUI event loop.
|
||||
///
|
||||
/// This is the handler side of an actor/handler pair with `FrameScheduler`, which coalesces
|
||||
@@ -68,15 +70,23 @@ impl FrameRequester {
|
||||
/// A scheduler for coalescing frame draw requests and notifying the TUI event loop.
|
||||
///
|
||||
/// This type is internal to `FrameRequester` and is spawned as a task to handle scheduling logic.
|
||||
///
|
||||
/// To avoid wasted redraw work, draw notifications are clamped to a maximum of 60 FPS (see
|
||||
/// [`FrameRateLimiter`]).
|
||||
struct FrameScheduler {
|
||||
receiver: mpsc::UnboundedReceiver<Instant>,
|
||||
draw_tx: broadcast::Sender<()>,
|
||||
rate_limiter: FrameRateLimiter,
|
||||
}
|
||||
|
||||
impl FrameScheduler {
|
||||
/// Create a new FrameScheduler with the provided receiver and draw notification sender.
|
||||
fn new(receiver: mpsc::UnboundedReceiver<Instant>, draw_tx: broadcast::Sender<()>) -> Self {
|
||||
Self { receiver, draw_tx }
|
||||
Self {
|
||||
receiver,
|
||||
draw_tx,
|
||||
rate_limiter: FrameRateLimiter::default(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Run the scheduling loop, coalescing frame requests and notifying the TUI event loop.
|
||||
@@ -97,6 +107,7 @@ impl FrameScheduler {
|
||||
// All senders dropped; exit the scheduler.
|
||||
break
|
||||
};
|
||||
let draw_at = self.rate_limiter.clamp_deadline(draw_at);
|
||||
next_deadline = Some(next_deadline.map_or(draw_at, |cur| cur.min(draw_at)));
|
||||
|
||||
// Do not send a draw immediately here. By continuing the loop,
|
||||
@@ -107,6 +118,7 @@ impl FrameScheduler {
|
||||
_ = &mut deadline => {
|
||||
if next_deadline.is_some() {
|
||||
next_deadline = None;
|
||||
self.rate_limiter.mark_emitted(target);
|
||||
let _ = self.draw_tx.send(());
|
||||
}
|
||||
}
|
||||
@@ -116,6 +128,7 @@ impl FrameScheduler {
|
||||
}
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::super::frame_rate_limiter::MIN_FRAME_INTERVAL;
|
||||
use super::*;
|
||||
use tokio::time;
|
||||
use tokio_util::time::FutureExt;
|
||||
@@ -218,6 +231,98 @@ mod tests {
|
||||
assert!(second.is_err(), "unexpected extra draw received");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
async fn test_limits_draw_notifications_to_60fps() {
|
||||
let (draw_tx, mut draw_rx) = broadcast::channel(16);
|
||||
let requester = FrameRequester::new(draw_tx);
|
||||
|
||||
requester.schedule_frame();
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let first = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for first draw");
|
||||
assert!(first.is_ok(), "broadcast closed unexpectedly");
|
||||
|
||||
requester.schedule_frame();
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
|
||||
assert!(
|
||||
early.is_err(),
|
||||
"draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})"
|
||||
);
|
||||
|
||||
time::advance(MIN_FRAME_INTERVAL).await;
|
||||
let second = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for second draw");
|
||||
assert!(second.is_ok(), "broadcast closed unexpectedly");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
async fn test_rate_limit_clamps_early_delayed_requests() {
|
||||
let (draw_tx, mut draw_rx) = broadcast::channel(16);
|
||||
let requester = FrameRequester::new(draw_tx);
|
||||
|
||||
requester.schedule_frame();
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let first = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for first draw");
|
||||
assert!(first.is_ok(), "broadcast closed unexpectedly");
|
||||
|
||||
requester.schedule_frame_in(Duration::from_millis(1));
|
||||
|
||||
time::advance(Duration::from_millis(10)).await;
|
||||
let too_early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
|
||||
assert!(
|
||||
too_early.is_err(),
|
||||
"draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})"
|
||||
);
|
||||
|
||||
time::advance(MIN_FRAME_INTERVAL).await;
|
||||
let second = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for clamped draw");
|
||||
assert!(second.is_ok(), "broadcast closed unexpectedly");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
async fn test_rate_limit_does_not_delay_future_draws() {
|
||||
let (draw_tx, mut draw_rx) = broadcast::channel(16);
|
||||
let requester = FrameRequester::new(draw_tx);
|
||||
|
||||
requester.schedule_frame();
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let first = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for first draw");
|
||||
assert!(first.is_ok(), "broadcast closed unexpectedly");
|
||||
|
||||
requester.schedule_frame_in(Duration::from_millis(50));
|
||||
|
||||
time::advance(Duration::from_millis(49)).await;
|
||||
let early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
|
||||
assert!(early.is_err(), "draw fired too early");
|
||||
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let second = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for delayed draw");
|
||||
assert!(second.is_ok(), "broadcast closed unexpectedly");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
async fn test_multiple_delayed_requests_coalesce_to_earliest() {
|
||||
let (draw_tx, mut draw_rx) = broadcast::channel(16);
|
||||
|
||||
@@ -1114,9 +1114,8 @@ impl App {
|
||||
.scrolled_by(delta_lines, &line_meta, visible_lines);
|
||||
|
||||
if schedule_frame {
|
||||
// Delay redraws slightly so scroll bursts coalesce into a single frame.
|
||||
tui.frame_requester()
|
||||
.schedule_frame_in(Duration::from_millis(16));
|
||||
// Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps.
|
||||
tui.frame_requester().schedule_frame();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
use std::io::Result;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::history_cell::HistoryCell;
|
||||
use crate::history_cell::UserHistoryCell;
|
||||
@@ -280,8 +279,8 @@ impl PagerView {
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
tui.frame_requester()
|
||||
.schedule_frame_in(Duration::from_millis(16));
|
||||
// Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps.
|
||||
tui.frame_requester().schedule_frame();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -298,8 +297,8 @@ impl PagerView {
|
||||
return Ok(());
|
||||
}
|
||||
}
|
||||
tui.frame_requester()
|
||||
.schedule_frame_in(Duration::from_millis(16));
|
||||
// Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps.
|
||||
tui.frame_requester().schedule_frame();
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -47,6 +47,7 @@ use crate::tui::job_control::SUSPEND_KEY;
|
||||
use crate::tui::job_control::SuspendContext;
|
||||
|
||||
mod alt_screen_nesting;
|
||||
mod frame_rate_limiter;
|
||||
mod frame_requester;
|
||||
#[cfg(unix)]
|
||||
mod job_control;
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
//! Limits how frequently frame draw notifications may be emitted.
|
||||
//!
|
||||
//! Widgets sometimes call `FrameRequester::schedule_frame()` more frequently than a user can
|
||||
//! perceive. This limiter clamps draw notifications to a maximum of 60 FPS to avoid wasted work.
|
||||
//!
|
||||
//! This is intentionally a small, pure helper so it can be unit-tested in isolation and used by
|
||||
//! the async frame scheduler without adding complexity to the app/event loop.
|
||||
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
/// A 60 FPS minimum frame interval (≈16.67ms).
|
||||
pub(super) const MIN_FRAME_INTERVAL: Duration = Duration::from_nanos(16_666_667);
|
||||
|
||||
/// Remembers the most recent emitted draw, allowing deadlines to be clamped forward.
|
||||
#[derive(Debug, Default)]
|
||||
pub(super) struct FrameRateLimiter {
|
||||
last_emitted_at: Option<Instant>,
|
||||
}
|
||||
|
||||
impl FrameRateLimiter {
|
||||
/// Returns `requested`, clamped forward if it would exceed the maximum frame rate.
|
||||
pub(super) fn clamp_deadline(&self, requested: Instant) -> Instant {
|
||||
let Some(last_emitted_at) = self.last_emitted_at else {
|
||||
return requested;
|
||||
};
|
||||
let min_allowed = last_emitted_at
|
||||
.checked_add(MIN_FRAME_INTERVAL)
|
||||
.unwrap_or(last_emitted_at);
|
||||
requested.max(min_allowed)
|
||||
}
|
||||
|
||||
/// Records that a draw notification was emitted at `emitted_at`.
|
||||
pub(super) fn mark_emitted(&mut self, emitted_at: Instant) {
|
||||
self.last_emitted_at = Some(emitted_at);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn default_does_not_clamp() {
|
||||
let t0 = Instant::now();
|
||||
let limiter = FrameRateLimiter::default();
|
||||
assert_eq!(limiter.clamp_deadline(t0), t0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clamps_to_min_interval_since_last_emit() {
|
||||
let t0 = Instant::now();
|
||||
let mut limiter = FrameRateLimiter::default();
|
||||
|
||||
assert_eq!(limiter.clamp_deadline(t0), t0);
|
||||
limiter.mark_emitted(t0);
|
||||
|
||||
let too_soon = t0 + Duration::from_millis(1);
|
||||
assert_eq!(limiter.clamp_deadline(too_soon), t0 + MIN_FRAME_INTERVAL);
|
||||
}
|
||||
}
|
||||
@@ -18,6 +18,8 @@ use std::time::Instant;
|
||||
use tokio::sync::broadcast;
|
||||
use tokio::sync::mpsc;
|
||||
|
||||
use super::frame_rate_limiter::FrameRateLimiter;
|
||||
|
||||
/// A requester for scheduling future frame draws on the TUI event loop.
|
||||
///
|
||||
/// This is the handler side of an actor/handler pair with `FrameScheduler`, which coalesces
|
||||
@@ -68,15 +70,23 @@ impl FrameRequester {
|
||||
/// A scheduler for coalescing frame draw requests and notifying the TUI event loop.
|
||||
///
|
||||
/// This type is internal to `FrameRequester` and is spawned as a task to handle scheduling logic.
|
||||
///
|
||||
/// To avoid wasted redraw work, draw notifications are clamped to a maximum of 60 FPS (see
|
||||
/// [`FrameRateLimiter`]).
|
||||
struct FrameScheduler {
|
||||
receiver: mpsc::UnboundedReceiver<Instant>,
|
||||
draw_tx: broadcast::Sender<()>,
|
||||
rate_limiter: FrameRateLimiter,
|
||||
}
|
||||
|
||||
impl FrameScheduler {
|
||||
/// Create a new FrameScheduler with the provided receiver and draw notification sender.
|
||||
fn new(receiver: mpsc::UnboundedReceiver<Instant>, draw_tx: broadcast::Sender<()>) -> Self {
|
||||
Self { receiver, draw_tx }
|
||||
Self {
|
||||
receiver,
|
||||
draw_tx,
|
||||
rate_limiter: FrameRateLimiter::default(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Run the scheduling loop, coalescing frame requests and notifying the TUI event loop.
|
||||
@@ -97,6 +107,7 @@ impl FrameScheduler {
|
||||
// All senders dropped; exit the scheduler.
|
||||
break
|
||||
};
|
||||
let draw_at = self.rate_limiter.clamp_deadline(draw_at);
|
||||
next_deadline = Some(next_deadline.map_or(draw_at, |cur| cur.min(draw_at)));
|
||||
|
||||
// Do not send a draw immediately here. By continuing the loop,
|
||||
@@ -107,6 +118,7 @@ impl FrameScheduler {
|
||||
_ = &mut deadline => {
|
||||
if next_deadline.is_some() {
|
||||
next_deadline = None;
|
||||
self.rate_limiter.mark_emitted(target);
|
||||
let _ = self.draw_tx.send(());
|
||||
}
|
||||
}
|
||||
@@ -116,6 +128,7 @@ impl FrameScheduler {
|
||||
}
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::super::frame_rate_limiter::MIN_FRAME_INTERVAL;
|
||||
use super::*;
|
||||
use tokio::time;
|
||||
use tokio_util::time::FutureExt;
|
||||
@@ -218,6 +231,98 @@ mod tests {
|
||||
assert!(second.is_err(), "unexpected extra draw received");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
async fn test_limits_draw_notifications_to_60fps() {
|
||||
let (draw_tx, mut draw_rx) = broadcast::channel(16);
|
||||
let requester = FrameRequester::new(draw_tx);
|
||||
|
||||
requester.schedule_frame();
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let first = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for first draw");
|
||||
assert!(first.is_ok(), "broadcast closed unexpectedly");
|
||||
|
||||
requester.schedule_frame();
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
|
||||
assert!(
|
||||
early.is_err(),
|
||||
"draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})"
|
||||
);
|
||||
|
||||
time::advance(MIN_FRAME_INTERVAL).await;
|
||||
let second = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for second draw");
|
||||
assert!(second.is_ok(), "broadcast closed unexpectedly");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
async fn test_rate_limit_clamps_early_delayed_requests() {
|
||||
let (draw_tx, mut draw_rx) = broadcast::channel(16);
|
||||
let requester = FrameRequester::new(draw_tx);
|
||||
|
||||
requester.schedule_frame();
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let first = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for first draw");
|
||||
assert!(first.is_ok(), "broadcast closed unexpectedly");
|
||||
|
||||
requester.schedule_frame_in(Duration::from_millis(1));
|
||||
|
||||
time::advance(Duration::from_millis(10)).await;
|
||||
let too_early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
|
||||
assert!(
|
||||
too_early.is_err(),
|
||||
"draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})"
|
||||
);
|
||||
|
||||
time::advance(MIN_FRAME_INTERVAL).await;
|
||||
let second = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for clamped draw");
|
||||
assert!(second.is_ok(), "broadcast closed unexpectedly");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
async fn test_rate_limit_does_not_delay_future_draws() {
|
||||
let (draw_tx, mut draw_rx) = broadcast::channel(16);
|
||||
let requester = FrameRequester::new(draw_tx);
|
||||
|
||||
requester.schedule_frame();
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let first = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for first draw");
|
||||
assert!(first.is_ok(), "broadcast closed unexpectedly");
|
||||
|
||||
requester.schedule_frame_in(Duration::from_millis(50));
|
||||
|
||||
time::advance(Duration::from_millis(49)).await;
|
||||
let early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
|
||||
assert!(early.is_err(), "draw fired too early");
|
||||
|
||||
time::advance(Duration::from_millis(1)).await;
|
||||
let second = draw_rx
|
||||
.recv()
|
||||
.timeout(Duration::from_millis(50))
|
||||
.await
|
||||
.expect("timed out waiting for delayed draw");
|
||||
assert!(second.is_ok(), "broadcast closed unexpectedly");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread", start_paused = true)]
|
||||
async fn test_multiple_delayed_requests_coalesce_to_earliest() {
|
||||
let (draw_tx, mut draw_rx) = broadcast::channel(16);
|
||||
|
||||
Reference in New Issue
Block a user