Skip to content

Commit 96a65ff

Browse files
authored
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
1 parent 40de81e commit 96a65ff

File tree

8 files changed

+344
-10
lines changed

8 files changed

+344
-10
lines changed

codex-rs/tui/src/tui.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ use crate::tui::event_stream::TuiEventStream;
4747
use crate::tui::job_control::SuspendContext;
4848

4949
mod event_stream;
50+
mod frame_rate_limiter;
5051
mod frame_requester;
5152
#[cfg(unix)]
5253
mod job_control;
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
//! Limits how frequently frame draw notifications may be emitted.
2+
//!
3+
//! Widgets sometimes call `FrameRequester::schedule_frame()` more frequently than a user can
4+
//! perceive. This limiter clamps draw notifications to a maximum of 60 FPS to avoid wasted work.
5+
//!
6+
//! This is intentionally a small, pure helper so it can be unit-tested in isolation and used by
7+
//! the async frame scheduler without adding complexity to the app/event loop.
8+
9+
use std::time::Duration;
10+
use std::time::Instant;
11+
12+
/// A 60 FPS minimum frame interval (≈16.67ms).
13+
pub(super) const MIN_FRAME_INTERVAL: Duration = Duration::from_nanos(16_666_667);
14+
15+
/// Remembers the most recent emitted draw, allowing deadlines to be clamped forward.
16+
#[derive(Debug, Default)]
17+
pub(super) struct FrameRateLimiter {
18+
last_emitted_at: Option<Instant>,
19+
}
20+
21+
impl FrameRateLimiter {
22+
/// Returns `requested`, clamped forward if it would exceed the maximum frame rate.
23+
pub(super) fn clamp_deadline(&self, requested: Instant) -> Instant {
24+
let Some(last_emitted_at) = self.last_emitted_at else {
25+
return requested;
26+
};
27+
let min_allowed = last_emitted_at
28+
.checked_add(MIN_FRAME_INTERVAL)
29+
.unwrap_or(last_emitted_at);
30+
requested.max(min_allowed)
31+
}
32+
33+
/// Records that a draw notification was emitted at `emitted_at`.
34+
pub(super) fn mark_emitted(&mut self, emitted_at: Instant) {
35+
self.last_emitted_at = Some(emitted_at);
36+
}
37+
}
38+
39+
#[cfg(test)]
40+
mod tests {
41+
use super::*;
42+
use pretty_assertions::assert_eq;
43+
44+
#[test]
45+
fn default_does_not_clamp() {
46+
let t0 = Instant::now();
47+
let limiter = FrameRateLimiter::default();
48+
assert_eq!(limiter.clamp_deadline(t0), t0);
49+
}
50+
51+
#[test]
52+
fn clamps_to_min_interval_since_last_emit() {
53+
let t0 = Instant::now();
54+
let mut limiter = FrameRateLimiter::default();
55+
56+
assert_eq!(limiter.clamp_deadline(t0), t0);
57+
limiter.mark_emitted(t0);
58+
59+
let too_soon = t0 + Duration::from_millis(1);
60+
assert_eq!(limiter.clamp_deadline(too_soon), t0 + MIN_FRAME_INTERVAL);
61+
}
62+
}

codex-rs/tui/src/tui/frame_requester.rs

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ use std::time::Instant;
1818
use tokio::sync::broadcast;
1919
use tokio::sync::mpsc;
2020

21+
use super::frame_rate_limiter::FrameRateLimiter;
22+
2123
/// A requester for scheduling future frame draws on the TUI event loop.
2224
///
2325
/// This is the handler side of an actor/handler pair with `FrameScheduler`, which coalesces
@@ -68,15 +70,23 @@ impl FrameRequester {
6870
/// A scheduler for coalescing frame draw requests and notifying the TUI event loop.
6971
///
7072
/// This type is internal to `FrameRequester` and is spawned as a task to handle scheduling logic.
73+
///
74+
/// To avoid wasted redraw work, draw notifications are clamped to a maximum of 60 FPS (see
75+
/// [`FrameRateLimiter`]).
7176
struct FrameScheduler {
7277
receiver: mpsc::UnboundedReceiver<Instant>,
7378
draw_tx: broadcast::Sender<()>,
79+
rate_limiter: FrameRateLimiter,
7480
}
7581

7682
impl FrameScheduler {
7783
/// Create a new FrameScheduler with the provided receiver and draw notification sender.
7884
fn new(receiver: mpsc::UnboundedReceiver<Instant>, draw_tx: broadcast::Sender<()>) -> Self {
79-
Self { receiver, draw_tx }
85+
Self {
86+
receiver,
87+
draw_tx,
88+
rate_limiter: FrameRateLimiter::default(),
89+
}
8090
}
8191

8292
/// Run the scheduling loop, coalescing frame requests and notifying the TUI event loop.
@@ -97,6 +107,7 @@ impl FrameScheduler {
97107
// All senders dropped; exit the scheduler.
98108
break
99109
};
110+
let draw_at = self.rate_limiter.clamp_deadline(draw_at);
100111
next_deadline = Some(next_deadline.map_or(draw_at, |cur| cur.min(draw_at)));
101112

102113
// Do not send a draw immediately here. By continuing the loop,
@@ -107,6 +118,7 @@ impl FrameScheduler {
107118
_ = &mut deadline => {
108119
if next_deadline.is_some() {
109120
next_deadline = None;
121+
self.rate_limiter.mark_emitted(target);
110122
let _ = self.draw_tx.send(());
111123
}
112124
}
@@ -116,6 +128,7 @@ impl FrameScheduler {
116128
}
117129
#[cfg(test)]
118130
mod tests {
131+
use super::super::frame_rate_limiter::MIN_FRAME_INTERVAL;
119132
use super::*;
120133
use tokio::time;
121134
use tokio_util::time::FutureExt;
@@ -218,6 +231,98 @@ mod tests {
218231
assert!(second.is_err(), "unexpected extra draw received");
219232
}
220233

234+
#[tokio::test(flavor = "current_thread", start_paused = true)]
235+
async fn test_limits_draw_notifications_to_60fps() {
236+
let (draw_tx, mut draw_rx) = broadcast::channel(16);
237+
let requester = FrameRequester::new(draw_tx);
238+
239+
requester.schedule_frame();
240+
time::advance(Duration::from_millis(1)).await;
241+
let first = draw_rx
242+
.recv()
243+
.timeout(Duration::from_millis(50))
244+
.await
245+
.expect("timed out waiting for first draw");
246+
assert!(first.is_ok(), "broadcast closed unexpectedly");
247+
248+
requester.schedule_frame();
249+
time::advance(Duration::from_millis(1)).await;
250+
let early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
251+
assert!(
252+
early.is_err(),
253+
"draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})"
254+
);
255+
256+
time::advance(MIN_FRAME_INTERVAL).await;
257+
let second = draw_rx
258+
.recv()
259+
.timeout(Duration::from_millis(50))
260+
.await
261+
.expect("timed out waiting for second draw");
262+
assert!(second.is_ok(), "broadcast closed unexpectedly");
263+
}
264+
265+
#[tokio::test(flavor = "current_thread", start_paused = true)]
266+
async fn test_rate_limit_clamps_early_delayed_requests() {
267+
let (draw_tx, mut draw_rx) = broadcast::channel(16);
268+
let requester = FrameRequester::new(draw_tx);
269+
270+
requester.schedule_frame();
271+
time::advance(Duration::from_millis(1)).await;
272+
let first = draw_rx
273+
.recv()
274+
.timeout(Duration::from_millis(50))
275+
.await
276+
.expect("timed out waiting for first draw");
277+
assert!(first.is_ok(), "broadcast closed unexpectedly");
278+
279+
requester.schedule_frame_in(Duration::from_millis(1));
280+
281+
time::advance(Duration::from_millis(10)).await;
282+
let too_early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
283+
assert!(
284+
too_early.is_err(),
285+
"draw fired too early; expected max 60fps (min interval {MIN_FRAME_INTERVAL:?})"
286+
);
287+
288+
time::advance(MIN_FRAME_INTERVAL).await;
289+
let second = draw_rx
290+
.recv()
291+
.timeout(Duration::from_millis(50))
292+
.await
293+
.expect("timed out waiting for clamped draw");
294+
assert!(second.is_ok(), "broadcast closed unexpectedly");
295+
}
296+
297+
#[tokio::test(flavor = "current_thread", start_paused = true)]
298+
async fn test_rate_limit_does_not_delay_future_draws() {
299+
let (draw_tx, mut draw_rx) = broadcast::channel(16);
300+
let requester = FrameRequester::new(draw_tx);
301+
302+
requester.schedule_frame();
303+
time::advance(Duration::from_millis(1)).await;
304+
let first = draw_rx
305+
.recv()
306+
.timeout(Duration::from_millis(50))
307+
.await
308+
.expect("timed out waiting for first draw");
309+
assert!(first.is_ok(), "broadcast closed unexpectedly");
310+
311+
requester.schedule_frame_in(Duration::from_millis(50));
312+
313+
time::advance(Duration::from_millis(49)).await;
314+
let early = draw_rx.recv().timeout(Duration::from_millis(1)).await;
315+
assert!(early.is_err(), "draw fired too early");
316+
317+
time::advance(Duration::from_millis(1)).await;
318+
let second = draw_rx
319+
.recv()
320+
.timeout(Duration::from_millis(50))
321+
.await
322+
.expect("timed out waiting for delayed draw");
323+
assert!(second.is_ok(), "broadcast closed unexpectedly");
324+
}
325+
221326
#[tokio::test(flavor = "current_thread", start_paused = true)]
222327
async fn test_multiple_delayed_requests_coalesce_to_earliest() {
223328
let (draw_tx, mut draw_rx) = broadcast::channel(16);

codex-rs/tui2/src/app.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,9 +1114,8 @@ impl App {
11141114
.scrolled_by(delta_lines, &line_meta, visible_lines);
11151115

11161116
if schedule_frame {
1117-
// Delay redraws slightly so scroll bursts coalesce into a single frame.
1118-
tui.frame_requester()
1119-
.schedule_frame_in(Duration::from_millis(16));
1117+
// Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps.
1118+
tui.frame_requester().schedule_frame();
11201119
}
11211120
}
11221121

codex-rs/tui2/src/pager_overlay.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use std::io::Result;
22
use std::sync::Arc;
3-
use std::time::Duration;
43

54
use crate::history_cell::HistoryCell;
65
use crate::history_cell::UserHistoryCell;
@@ -280,8 +279,8 @@ impl PagerView {
280279
return Ok(());
281280
}
282281
}
283-
tui.frame_requester()
284-
.schedule_frame_in(Duration::from_millis(16));
282+
// Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps.
283+
tui.frame_requester().schedule_frame();
285284
Ok(())
286285
}
287286

@@ -298,8 +297,8 @@ impl PagerView {
298297
return Ok(());
299298
}
300299
}
301-
tui.frame_requester()
302-
.schedule_frame_in(Duration::from_millis(16));
300+
// Request a redraw; the frame scheduler coalesces bursts and clamps to 60fps.
301+
tui.frame_requester().schedule_frame();
303302
Ok(())
304303
}
305304

codex-rs/tui2/src/tui.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ use crate::tui::job_control::SUSPEND_KEY;
4747
use crate::tui::job_control::SuspendContext;
4848

4949
mod alt_screen_nesting;
50+
mod frame_rate_limiter;
5051
mod frame_requester;
5152
#[cfg(unix)]
5253
mod job_control;
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
//! Limits how frequently frame draw notifications may be emitted.
2+
//!
3+
//! Widgets sometimes call `FrameRequester::schedule_frame()` more frequently than a user can
4+
//! perceive. This limiter clamps draw notifications to a maximum of 60 FPS to avoid wasted work.
5+
//!
6+
//! This is intentionally a small, pure helper so it can be unit-tested in isolation and used by
7+
//! the async frame scheduler without adding complexity to the app/event loop.
8+
9+
use std::time::Duration;
10+
use std::time::Instant;
11+
12+
/// A 60 FPS minimum frame interval (≈16.67ms).
13+
pub(super) const MIN_FRAME_INTERVAL: Duration = Duration::from_nanos(16_666_667);
14+
15+
/// Remembers the most recent emitted draw, allowing deadlines to be clamped forward.
16+
#[derive(Debug, Default)]
17+
pub(super) struct FrameRateLimiter {
18+
last_emitted_at: Option<Instant>,
19+
}
20+
21+
impl FrameRateLimiter {
22+
/// Returns `requested`, clamped forward if it would exceed the maximum frame rate.
23+
pub(super) fn clamp_deadline(&self, requested: Instant) -> Instant {
24+
let Some(last_emitted_at) = self.last_emitted_at else {
25+
return requested;
26+
};
27+
let min_allowed = last_emitted_at
28+
.checked_add(MIN_FRAME_INTERVAL)
29+
.unwrap_or(last_emitted_at);
30+
requested.max(min_allowed)
31+
}
32+
33+
/// Records that a draw notification was emitted at `emitted_at`.
34+
pub(super) fn mark_emitted(&mut self, emitted_at: Instant) {
35+
self.last_emitted_at = Some(emitted_at);
36+
}
37+
}
38+
39+
#[cfg(test)]
40+
mod tests {
41+
use super::*;
42+
use pretty_assertions::assert_eq;
43+
44+
#[test]
45+
fn default_does_not_clamp() {
46+
let t0 = Instant::now();
47+
let limiter = FrameRateLimiter::default();
48+
assert_eq!(limiter.clamp_deadline(t0), t0);
49+
}
50+
51+
#[test]
52+
fn clamps_to_min_interval_since_last_emit() {
53+
let t0 = Instant::now();
54+
let mut limiter = FrameRateLimiter::default();
55+
56+
assert_eq!(limiter.clamp_deadline(t0), t0);
57+
limiter.mark_emitted(t0);
58+
59+
let too_soon = t0 + Duration::from_millis(1);
60+
assert_eq!(limiter.clamp_deadline(too_soon), t0 + MIN_FRAME_INTERVAL);
61+
}
62+
}

0 commit comments

Comments
 (0)