Skip to content

Commit 3b5d351

Browse files
Fix high CPU usage from cursor position request feedback loop (#1310)
* Fix high CPU usage from cursor position request feedback loop (#1302) In non-alternate-screen modes, every Draw() sends a \x1b[6n cursor position request. Two compounding bugs turned this into a permanent ~60fps redraw loop, pegging both the app and the terminal emulator (most visibly tmux): 1. ThrottledRequest::Send() never updated last_request_time_, so the intended 500ms throttle never engaged: a new request was sent as soon as the previous reply arrived. 2. The terminal input parser sink requested an animation frame for every parsed event, including cursor position reports, which are internal replies to our own request. The armed animation frame invalidated the frame, triggering a full redraw that sent the next request, whose reply armed the next frame, and so on. Fix: record last_request_time_ in Send(), and stop requesting animation frames for cursor position reports (they are still delivered to the event buffer and consumed as before). Measured with the gallery example idling inside tmux: the app drops from ~24% CPU to ~0.4%, the tmux server from ~8% (detached; ~100% of a core with an attached client) to ~0%. Fixes #1302 * Request animation frames when handling events, not when receiving them Move RequestAnimationFrame() from the two event producers (the TerminalInputParser callback and App::PostEvent) to the Event branch of App::Internal::HandleTask, below the internal-reply early-returns. A single place now decides which events grant an animation frame, next to the code discriminating event kinds. As a result, every internal terminal reply (cursor shape, terminal capabilities, name/version, emulator) stops arming animation frames: previously only cursor position reports were exempted, and only on the parser path. This also moves the animation_requested_ write onto the main loop thread, removing an unsynchronized write when PostEvent is called from another thread. Add a test covering the event -> animation tick pipeline. See #1302. * Only produce animation frames when requested Restore the FTXUI 6 behavior: animation frames are produced only when requested via animation::RequestAnimationFrame(), typically by an animation::Animator which arms it on construction and re-arms it from OnAnimation until the transition completes. Handling an event no longer implicitly grants an animation tick. That behavior was introduced accidentally by the v7 event loop rework (33a40ee) and is what made the #1302 feedback loop possible in the first place: internal terminal replies are events too.
1 parent 8bf276b commit 3b5d351

3 files changed

Lines changed: 75 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,15 @@ Changelog
44
Next
55
====
66

7+
### Component
8+
- Bugfix: Fix high CPU usage (app and terminal emulator, e.g. tmux) caused by a
9+
cursor position request/reply feedback loop redrawing the screen at ~60fps in
10+
the non-alternate-screen modes. See #1302.
11+
- Animation frames are now produced only when requested via
12+
`animation::RequestAnimationFrame()` (e.g. by `animation::Animator`), as in
13+
FTXUI 6. Receiving an event no longer implicitly triggers an animation
14+
frame.
15+
716
### Dom
817
- Performance: `text` computes its requirement once and renders only the
918
visible lines. This makes scrolling a large text inside a `frame`

src/ftxui/component/animation_test.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,18 @@
33
// the LICENSE file.
44

55
#include <gtest/gtest.h>
6+
#include <chrono> // for milliseconds
67
#include <functional> // for function
8+
#include <thread> // for sleep_for
79
#include <vector> // for allocator, vector
810

911
#include "ftxui/component/animation.hpp" // for Function, BackIn, BackInOut, BackOut, BounceIn, BounceInOut, BounceOut, CircularIn, CircularInOut, CircularOut, CubicIn, CubicInOut, CubicOut, ElasticIn, ElasticInOut, ElasticOut, ExponentialIn, ExponentialInOut, ExponentialOut, Linear, QuadraticIn, QuadraticInOut, QuadraticOut, QuarticIn, QuarticInOut, QuarticOut, QuinticIn, QuinticInOut, QuinticOut, SineIn, SineInOut, SineOut
12+
#include "ftxui/component/app.hpp" // for App
13+
#include "ftxui/component/component.hpp" // for Make
14+
#include "ftxui/component/component_base.hpp" // for ComponentBase
15+
#include "ftxui/component/event.hpp" // for Event
16+
#include "ftxui/component/loop.hpp" // for Loop
17+
#include "ftxui/dom/elements.hpp" // for text
1018

1119
namespace ftxui {
1220

@@ -35,4 +43,61 @@ TEST(AnimationTest, StartAndEnd) {
3543
}
3644
}
3745

46+
namespace {
47+
48+
class AnimationCounter : public ComponentBase {
49+
public:
50+
int count = 0;
51+
52+
private:
53+
Element OnRender() override { return text(""); }
54+
void OnAnimation(animation::Params& /*params*/) override { count++; }
55+
bool OnEvent(Event event) override {
56+
if (event == Event::Character('a')) {
57+
animation::RequestAnimationFrame();
58+
return true;
59+
}
60+
return false;
61+
}
62+
};
63+
64+
// Give the recurring AnimationTask (reposted every 15ms) time to become due,
65+
// then run the loop to execute it.
66+
void RunAnimationTask(Loop& loop) {
67+
std::this_thread::sleep_for(std::chrono::milliseconds(20));
68+
loop.RunOnce();
69+
}
70+
71+
} // namespace
72+
73+
// OnAnimation must tick only when a frame was requested with
74+
// animation::RequestAnimationFrame(), typically by an Animator. In particular,
75+
// handling an event must not implicitly produce a tick: internal terminal
76+
// replies are events too, and animating on them caused the #1302 feedback
77+
// loop.
78+
TEST(AnimationTest, TicksOnlyWhenRequested) {
79+
auto component = Make<AnimationCounter>();
80+
auto screen = App::FixedSize(10, 1);
81+
Loop loop(&screen, component);
82+
loop.RunOnce();
83+
84+
// No request: the recurring AnimationTask is a no-op.
85+
RunAnimationTask(loop);
86+
EXPECT_EQ(component->count, 0);
87+
88+
// An event alone doesn't produce a tick.
89+
screen.PostEvent(Event::Custom);
90+
loop.RunOnce();
91+
RunAnimationTask(loop);
92+
EXPECT_EQ(component->count, 0);
93+
94+
// A component requesting a frame gets exactly one tick.
95+
screen.PostEvent(Event::Character('a'));
96+
loop.RunOnce();
97+
RunAnimationTask(loop);
98+
EXPECT_EQ(component->count, 1);
99+
RunAnimationTask(loop);
100+
EXPECT_EQ(component->count, 1);
101+
}
102+
38103
} // namespace ftxui

src/ftxui/component/app.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ struct App::Internal {
228228
private:
229229
void Send() {
230230
last_sent_time_ = std::chrono::steady_clock::now();
231+
last_request_time_ = last_sent_time_;
231232
pending_request_ = true;
232233
send_();
233234
}
@@ -556,7 +557,6 @@ App::Internal::Internal(App* app,
556557
use_alternative_screen_(use_alternative_screen),
557558
terminal_input_parser([&](Event event) {
558559
event_buffer.Push(std::move(event));
559-
public_->RequestAnimationFrame();
560560
}),
561561
cursor_position_request(this, [this] {
562562
TerminalSend(DeviceStatusReport(DSRMode::kCursor));
@@ -1533,7 +1533,6 @@ void App::Post(Task task) {
15331533

15341534
void App::PostEvent(Event event) {
15351535
internal_->event_buffer.Push(std::move(event));
1536-
RequestAnimationFrame();
15371536
}
15381537

15391538
// static

0 commit comments

Comments
 (0)