terminal: debounce compression by page activity

Compression scheduling previously postponed its idle timer after every renderer wake. The inspector redraw loop wakes the renderer continuously, so opening it could prevent pending scrollback compression from ever starting. Track compression-relevant PageList activity with a wrapping 48-bit token and restart the timer only when the composite Terminal token changes. This removes the separate dirty bit while reserving 16 bits for future Terminal-owned compression triggers. Expose target availability through the terminal package and leave renderer compression state undefined on unsupported targets so its timer is never initialized.
2026-07-11 11:49:40 +00:00 · 2026-07-09 09:16:56 -07:00
parent b9bb50c832
commit 181254d36a
4 changed files with 189 additions and 198 deletions
--- a/src/renderer/Thread.zig
+++ b/src/renderer/Thread.zig
@@ -10,6 +10,7 @@ const internal_os = @import("../os/main.zig");
 const rendererpkg = @import("../renderer.zig");
 const apprt = @import("../apprt.zig");
 const configpkg = @import("../config.zig");
+const terminalpkg = @import("../terminal/main.zig");
 const BlockingQueue = @import("../datastruct/main.zig").BlockingQueue;
 const App = @import("../App.zig");

@@ -34,91 +35,6 @@ const must_draw_from_app_thread =
 /// the future if we want it configurable.
 pub const Mailbox = BlockingQueue(rendererpkg.Message, 64);

-/// Schedules incremental terminal compression after renderer activity stops.
-///
-/// This owns all renderer-specific compression state. The terminal decides
-/// whether compression is required and performs the actual work; the renderer
-/// only provides idle scheduling and avoids waiting for the terminal lock.
-const Compression = struct {
-    const idle_interval = 250;
-    const step_interval = 1;
-
-    timer: xev.Timer,
-    completion: xev.Completion = .{},
-    reset_completion: xev.Completion = .{},
-
-    fn init() !Compression {
-        return .{ .timer = try xev.Timer.init() };
-    }
-
-    fn deinit(self: *Compression) void {
-        self.timer.deinit();
-    }
-
-    /// Start or postpone compression after a renderer wake.
-    fn wake(self: *Compression, thread: *Thread) void {
-        // An active timer already proves work was pending. Otherwise consult
-        // the terminal, without waiting if rendering or parsing holds its lock.
-        if (self.completion.state() != .active) {
-            if (thread.state.mutex.tryLock()) {
-                defer thread.state.mutex.unlock();
-                if (!thread.state.terminal.compressionRequired()) return;
-            }
-        }
-
-        self.schedule(thread, idle_interval);
-    }
-
-    /// Start the one-shot timer, or move its deadline if it is already active.
-    fn schedule(self: *Compression, thread: *Thread, delay_ms: u64) void {
-        self.timer.reset(
-            &thread.loop,
-            &self.completion,
-            &self.reset_completion,
-            delay_ms,
-            Thread,
-            thread,
-            timerCallback,
-        );
-    }
-
-    fn timerCallback(
-        thread_: ?*Thread,
-        _: *xev.Loop,
-        _: *xev.Completion,
-        result: xev.Timer.RunError!void,
-    ) xev.CallbackAction {
-        _ = result catch |err| switch (err) {
-            error.Canceled => return .disarm,
-            else => {
-                log.warn("error in compression timer err={}", .{err});
-                return .disarm;
-            },
-        };
-
-        const thread = thread_ orelse return .disarm;
-        const self = &thread.compression;
-
-        if (step(thread.state)) |delay| self.schedule(thread, delay);
-        return .disarm;
-    }
-
-    /// Try one bounded step without waiting for the terminal lock. The return
-    /// value is the delay before another attempt, or null when work is done.
-    fn step(state: *rendererpkg.State) ?u64 {
-        if (!state.mutex.tryLock()) return idle_interval;
-        defer state.mutex.unlock();
-
-        return switch (state.terminal.compress(.incremental)) {
-            .pending => step_interval,
-            .unsupported,
-            .deferred,
-            .complete,
-            => null,
-        };
-    }
-};
-
 /// Allocator used for some state
 alloc: std.mem.Allocator,

@@ -158,7 +74,7 @@ cursor_c: xev.Completion = .{},
 cursor_c_cancel: xev.Completion = .{},

 /// Incremental scrollback compression scheduling.
-compression: Compression,
+compression: Compression = undefined,

 /// The surface we're rendering to.
 surface: *apprt.Surface,
@@ -246,14 +162,11 @@ pub fn init(
    var cursor_timer = try xev.Timer.init();
    errdefer cursor_timer.deinit();

-    var compression = try Compression.init();
-    errdefer compression.deinit();
-
    // The mailbox for messaging this thread
    var mailbox = try Mailbox.create(alloc);
    errdefer mailbox.destroy(alloc);

-    return .{
+    var result: Thread = .{
        .alloc = alloc,
        .config = .init(config),
        .loop = loop,
@@ -263,13 +176,20 @@ pub fn init(
        .draw_h = draw_h,
        .draw_now = draw_now,
        .cursor_h = cursor_timer,
-        .compression = compression,
        .surface = surface,
        .renderer = renderer_impl,
        .state = state,
        .mailbox = mailbox,
        .app_mailbox = app_mailbox,
    };
+
+    // Only enable compression if we have it enabled... save some
+    // minor resources.
+    if (comptime terminalpkg.compression_enabled) {
+        result.compression = try .init();
+    }
+
+    return result;
 }

 /// Clean up the thread. This is only safe to call once the thread
@@ -281,7 +201,8 @@ pub fn deinit(self: *Thread) void {
    self.draw_h.deinit();
    self.draw_now.deinit();
    self.cursor_h.deinit();
-    self.compression.deinit();
+    if (comptime terminalpkg.compression_enabled)
+        self.compression.deinit();
    self.loop.deinit();

    // Nothing can possibly access the mailbox anymore, destroy it.
@@ -821,62 +742,102 @@ fn cursorBlinkInterval() u64 {
    return CURSOR_BLINK_INTERVAL;
 }

-test "compression timer advances primary history" {
-    const testing = std.testing;
-    const terminalpkg = @import("../terminal/main.zig");
+/// Schedules incremental terminal compression after renderer activity stops.
+///
+/// This owns all renderer-specific compression state. The terminal decides
+/// when compression-relevant activity changes and performs the actual work;
+/// the renderer only provides idle scheduling and avoids waiting for the
+/// terminal lock.
+const Compression = struct {
+    const idle_interval = 250;
+    const step_interval = 1;

-    var terminal = try terminalpkg.Terminal.init(testing.allocator, .{
-        .cols = 80,
-        .rows = 24,
-        .max_scrollback = 10 * 1024 * 1024,
-    });
-    defer terminal.deinit(testing.allocator);
+    timer: xev.Timer,
+    completion: xev.Completion = .{},
+    reset_completion: xev.Completion = .{},
+    activity: ?u64 = null,

-    const primary = terminal.screens.get(.primary).?;
-    primary.cursorAbsolute(0, primary.pages.rows - 1);
-    while (primary.pages.pages.first.? ==
-        primary.pages.getTopLeft(.active).node)
-    {
-        try primary.cursorDownScroll();
+    fn init() !Compression {
+        return .{ .timer = try xev.Timer.init() };
    }

-    // Compression always targets primary history, even while an alternate
-    // screen is active.
-    _ = try terminal.screens.getInit(testing.allocator, .alternate, .{
-        .cols = 80,
-        .rows = 24,
-        .max_scrollback = 0,
-    });
-    terminal.screens.switchTo(.alternate);
+    fn deinit(self: *Compression) void {
+        self.timer.deinit();
+    }

-    var mutex: std.Thread.Mutex = .{};
-    var state: rendererpkg.State = .{
-        .mutex = &mutex,
-        .terminal = &terminal,
-    };
-    try testing.expect(terminal.compressionRequired());
+    /// Start or postpone compression after a renderer wake.
+    fn wake(self: *Compression, thread: *Thread) void {
+        // If we have no compression then don't do anything.
+        if (comptime !terminalpkg.compression_enabled) return;

-    // Contention postpones background work instead of blocking output.
-    mutex.lock();
-    try testing.expectEqual(
-        @as(?u64, Compression.idle_interval),
-        Compression.step(&state),
-    );
-    mutex.unlock();
+        // PageList activity, rather than a generic renderer wake, restarts the
+        // idle interval. In particular, the inspector wakes the renderer every
+        // frame without changing terminal contents and must not starve this
+        // timer indefinitely.
+        if (thread.state.mutex.tryLock()) {
+            defer thread.state.mutex.unlock();
+            const activity = thread.state.terminal.compressionActivity();
+            if (self.activity == activity) return;
+            self.activity = activity;
+        }

-    try testing.expectEqual(
-        @as(?u64, Compression.step_interval),
-        Compression.step(&state),
-    );
-    try testing.expectEqual(
-        @as(usize, 1),
-        primary.pages.memoryStats().compressed_pages,
-    );
+        // Contention may mean parsing is active. Scheduling is a harmless
+        // false positive when no compression work is actually pending.
+        self.schedule(thread, idle_interval);
+    }

-    // The no-work verification pass completes and leaves the timer disarmed.
-    try testing.expectEqual(
-        @as(?u64, null),
-        Compression.step(&state),
-    );
-    try testing.expect(!terminal.compressionRequired());
-}
+    /// Start the one-shot timer, or move its deadline if it is already active.
+    fn schedule(self: *Compression, thread: *Thread, delay_ms: u64) void {
+        self.timer.reset(
+            &thread.loop,
+            &self.completion,
+            &self.reset_completion,
+            delay_ms,
+            Thread,
+            thread,
+            timerCallback,
+        );
+    }
+
+    fn timerCallback(
+        thread_: ?*Thread,
+        _: *xev.Loop,
+        _: *xev.Completion,
+        result: xev.Timer.RunError!void,
+    ) xev.CallbackAction {
+        _ = result catch |err| switch (err) {
+            error.Canceled => return .disarm,
+            else => {
+                log.warn("error in compression timer err={}", .{err});
+                return .disarm;
+            },
+        };
+
+        const thread = thread_ orelse return .disarm;
+        const self = &thread.compression;
+
+        if (self.step(thread.state)) |delay| self.schedule(thread, delay);
+        return .disarm;
+    }
+
+    /// Try one bounded step without waiting for the terminal lock. The return
+    /// value is the delay before another attempt, or null when work is done.
+    fn step(self: *Compression, state: *rendererpkg.State) ?u64 {
+        if (!state.mutex.tryLock()) return idle_interval;
+        defer state.mutex.unlock();
+
+        const activity = state.terminal.compressionActivity();
+        if (self.activity != activity) {
+            self.activity = activity;
+            return idle_interval;
+        }
+
+        return switch (state.terminal.compress(.incremental)) {
+            .pending => step_interval,
+            .unsupported,
+            .deferred,
+            .complete,
+            => null,
+        };
+    }
+};
--- a/src/terminal/PageList.zig
+++ b/src/terminal/PageList.zig
@@ -1219,7 +1219,7 @@ pub fn resize(self: *PageList, opts: Resize) Allocator.Error!void {
    // reschedule compression.
    // TODO(mitchellh): Deferred reflow on non-viewport/non-active pages.
    self.page_compression.reset();
-    self.page_compression.markDirty();
+    self.page_compression.markActivity();

    if (comptime std.debug.runtime_safety) {
        // Resize does not work with 0 values, this should be protected
@@ -2799,7 +2799,7 @@ pub fn scroll(self: *PageList, behavior: Scroll) void {
    const was_active = self.viewport == .active;
    defer if (was_active != (self.viewport == .active)) {
        self.page_compression.reset();
-        if (self.viewport == .active) self.page_compression.markDirty();
+        if (self.viewport == .active) self.page_compression.markActivity();
    };

    // Special case no-scrollback mode to never allow scrolling.
@@ -3433,7 +3433,7 @@ pub fn grow(self: *PageList) Allocator.Error!?*List.Node {
    defer self.assertIntegrity();

    // Growing can move a complete page behind the active boundary.
-    self.page_compression.markDirty();
+    self.page_compression.markActivity();

    const last = self.pages.last.?;
    if (last.capacity().rows > last.rows()) {
@@ -3866,11 +3866,6 @@ const CompressionScratch = union(enum) {
 /// without requiring consumers to maintain a separate attempt cursor.
 const IncrementalCompressionState = struct {
    flags: packed struct {
-        // Set to true when incremental compression should be run. It
-        // isn't guaranteed that incremental compression would produce
-        // results but some work was done that may require compression.
-        dirty: bool = false,
-
        /// Set after any page is compressed during the current traversal.
        /// We always run incremental compression until we get a fully
        /// no-compression pass (the verification pass). This lets us
@@ -3879,10 +3874,24 @@ const IncrementalCompressionState = struct {

        /// Set after a traversal first reaches the active boundary. When this
        /// verification traversal reaches the boundary, `did_compress`
-        /// determines whether to restart once more or clear the dirty state.
+        /// determines whether to restart once more or finish the pass.
        verifying: bool = false,
    } = .{},

+    /// Changes whenever PageList activity may affect compression work.
+    /// Callers use this value (via Terminal.compressionActivity) to
+    /// determine whether to recompress.
+    ///
+    /// The directionality of this doesn't matter. We overflow and
+    /// wrap when maxxed. The comparison of this value to your saved
+    /// value is all that matters. There is a possible edge case where you
+    /// don't compress, a full wraparound happens, and you get the same value,
+    /// but its so unlikely.
+    ///
+    /// This is 48-bits so that 16-bits can be reserved for Terminal to
+    /// add extra state.
+    activity_serial: u48 = 0,
+
    /// Serial of the last page inspected by the traversal. This is
    /// intentionally an implementation detail and is reset by PageList
    /// operations which restart traversal progress.
@@ -3892,14 +3901,15 @@ const IncrementalCompressionState = struct {
    /// node at or above this value before the saved marker requires a restart.
    next_serial: u64 = 0,

-    /// Schedule compression without disturbing valid traversal progress.
-    fn markDirty(self: *IncrementalCompressionState) void {
-        self.flags.dirty = true;
+    /// Record activity without disturbing valid traversal progress.
+    fn markActivity(self: *IncrementalCompressionState) void {
+        self.activity_serial +%= 1;
    }

-    /// Discard traversal state and leave no compression work pending.
+    /// Discard traversal progress without changing the activity token.
    fn reset(self: *IncrementalCompressionState) void {
-        self.* = .{};
+        const activity_serial: u48 = self.activity_serial;
+        self.* = .{ .activity_serial = activity_serial };
    }
 };

@@ -3984,8 +3994,6 @@ fn compressIncremental(self: *PageList) IncrementalCompressionResult {
        return .deferred;
    }

-    if (!state.flags.dirty) return .complete;
-
    // Find the node following the exact continuation marker within the cold
    // prefix. A missing marker means the list changed between steps, so begin
    // again at the current first page. This lookup does not touch page memory.
@@ -4042,10 +4050,11 @@ fn compressIncremental(self: *PageList) IncrementalCompressionResult {
    // do one pass after the first success so we can recompress nodes that
    // were possibly decompressed (e.g. by search, inspector, whatever).
    if (!state.flags.verifying or state.flags.did_compress) {
-        state.* = .{ .flags = .{
-            .dirty = true,
-            .verifying = true,
-        } };
+        const activity_serial: u48 = state.activity_serial;
+        state.* = .{
+            .flags = .{ .verifying = true },
+            .activity_serial = activity_serial,
+        };
        return .pending;
    }

@@ -6413,17 +6422,22 @@ test "PageList incremental compression defers and completes" {
    var s = try init(testing.allocator, 80, 24, null);
    defer s.deinit();
    try s.growColdPagesForTest(1);
-    try testing.expect(s.page_compression.flags.dirty);
+    const initial_activity = s.page_compression.activity_serial;
+    try testing.expect(initial_activity > 0);

    s.scroll(.top);
-    try testing.expect(!s.page_compression.flags.dirty);
+    try testing.expectEqual(
+        initial_activity,
+        s.page_compression.activity_serial,
+    );
    const deferred = s.compress(.incremental);
    try testing.expectEqual(IncrementalCompressionResult.deferred, deferred);
    const cold = s.pages.first.?;
    try testing.expect(!cold.isCompressed());

    s.scroll(.active);
-    try testing.expect(s.page_compression.flags.dirty);
+    const active_activity = s.page_compression.activity_serial;
+    try testing.expect(active_activity != initial_activity);
    const pending = s.compress(.incremental);
    try testing.expectEqual(IncrementalCompressionResult.pending, pending);
    try testing.expect(cold.isCompressed());
@@ -6432,15 +6446,21 @@ test "PageList incremental compression defers and completes" {
    // no work.
    const complete = s.compress(.incremental);
    try testing.expectEqual(IncrementalCompressionResult.complete, complete);
-    try testing.expectEqual(IncrementalCompressionState{}, s.page_compression);
-    try testing.expect(!s.page_compression.flags.dirty);
+    try testing.expect(!s.page_compression.flags.did_compress);
+    try testing.expect(!s.page_compression.flags.verifying);
+    try testing.expectEqual(
+        active_activity,
+        s.page_compression.activity_serial,
+    );
+    try testing.expectEqual(@as(?u64, null), s.page_compression.last_serial);
+    try testing.expectEqual(@as(u64, 0), s.page_compression.next_serial);

-    // Resetting and marking the state dirty begins at the oldest page and
+    // Resetting and recording activity begins at the oldest page and
    // recompresses restored content.
    _ = cold.page();
    try testing.expect(!cold.isCompressed());
    s.page_compression.reset();
-    s.page_compression.markDirty();
+    s.page_compression.markActivity();
    const reconsidered = s.compress(.incremental);
    try testing.expectEqual(IncrementalCompressionResult.pending, reconsidered);
    try testing.expect(cold.isCompressed());
@@ -6456,50 +6476,59 @@ test "PageList owns incremental compression state" {
    var s = try init(testing.allocator, 80, 24, null);
    defer s.deinit();

-    const dirty: IncrementalCompressionState = .{
+    const state: IncrementalCompressionState = .{
        .flags = .{
-            .dirty = true,
            .did_compress = true,
            .verifying = true,
        },
+        .activity_serial = 42,
        .last_serial = 42,
        .next_serial = 43,
    };

-    s.page_compression = dirty;
+    s.page_compression = state;
    s.scroll(.top);
-    try testing.expectEqual(IncrementalCompressionState{}, s.page_compression);
+    try testing.expectEqual(
+        IncrementalCompressionState{ .activity_serial = 42 },
+        s.page_compression,
+    );

    // Scrolling within history preserves progress, while crossing back to the
    // active viewport resets it again.
-    s.page_compression = dirty;
+    s.page_compression = state;
    s.scroll(.top);
-    try testing.expectEqual(dirty, s.page_compression);
+    try testing.expectEqual(state, s.page_compression);
    s.scroll(.active);
    try testing.expectEqual(
-        IncrementalCompressionState{ .flags = .{ .dirty = true } },
+        IncrementalCompressionState{ .activity_serial = 43 },
        s.page_compression,
    );

-    s.page_compression = dirty;
+    s.page_compression = state;
    try s.resize(.{ .cols = 80, .rows = 24 });
    try testing.expectEqual(
-        IncrementalCompressionState{ .flags = .{ .dirty = true } },
+        IncrementalCompressionState{ .activity_serial = 43 },
        s.page_compression,
    );

-    s.page_compression = dirty;
+    s.page_compression = state;
    s.reset();
-    try testing.expectEqual(IncrementalCompressionState{}, s.page_compression);
+    try testing.expectEqual(
+        IncrementalCompressionState{ .activity_serial = 42 },
+        s.page_compression,
+    );

-    s.page_compression = dirty;
+    s.page_compression = state;
    try testing.expectEqual(
        IncrementalCompressionResult.complete,
        s.compress(.full),
    );
-    try testing.expectEqual(IncrementalCompressionState{}, s.page_compression);
+    try testing.expectEqual(
+        IncrementalCompressionState{ .activity_serial = 42 },
+        s.page_compression,
+    );

-    s.page_compression = dirty;
+    s.page_compression = state;
    var cloned = try s.clone(testing.allocator, .{
        .top = .{ .active = .{} },
    });
@@ -6521,7 +6550,7 @@ test "PageList incremental compression bounds inspected pages" {
    // metadata-only skip budget without stopping at a resident attempt.
    _ = s.compress(.full);
    s.page_compression.reset();
-    s.page_compression.markDirty();
+    s.page_compression.markActivity();
    try testing.expectEqual(
        incremental_compression_max_inspected + 1,
        s.memoryStats().compressed_pages,
@@ -6781,7 +6810,7 @@ test "PageList incremental compression keeps progress after tail growth" {
    try s.growColdPagesForTest(incremental_compression_max_inspected + 1);
    _ = s.compress(.full);
    s.page_compression.reset();
-    s.page_compression.markDirty();
+    s.page_compression.markActivity();

    var expected_last = s.pages.first.?;
    for (1..incremental_compression_max_inspected) |_|
--- a/src/terminal/Terminal.zig
+++ b/src/terminal/Terminal.zig
@@ -28,7 +28,6 @@ const mouse = @import("mouse.zig");
 const Stream = @import("stream_terminal.zig").Stream;

 const size = @import("size.zig");
-const terminal_mem = @import("mem.zig");
 const pagepkg = @import("page.zig");
 const style = @import("style.zig");
 const PageList = @import("PageList.zig");
@@ -2295,12 +2294,11 @@ pub fn scrollViewport(self: *Terminal, behavior: ScrollViewport) void {
    });
 }

-/// Return whether a compression pass is worth running again.
+/// Return the current compression activity value.
 ///
-/// When this returns true, callers should schedule a `compress`
-/// call at some point. We recommend doing incremental compression because
-/// compression is CPU heavy and stalls IO processing (due to the terminal
-/// being blocked). See `compress` for more details.
+/// Callers should schedule a `compress` call whenever this value changes. The
+/// direction of the change has no meaning; this is an opaque change token
+/// rather than a monotonic sequence exposed by Terminal.
 ///
 /// It is up to the terminal what it decides to compress, but currently
 /// we compress cold (non-viewed, non-editable) scrollback history on
@@ -2308,12 +2306,12 @@ pub fn scrollViewport(self: *Terminal, behavior: ScrollViewport) void {
 ///
 /// Note that compression requires specific system features, namely
 /// the ability to retain virtual memory allocations while discarding their
-/// physical memory backings. If the system libghostty is running on
-/// doesn't support that this will always return false and compression
-/// does nothing.
-pub inline fn compressionRequired(self: *const Terminal) bool {
-    if (comptime !terminal_mem.canReclaim(.strict)) return false;
-    return self.screens.get(.primary).?.pages.page_compression.flags.dirty;
+/// physical memory backings. Callers must still use `compress` to determine
+/// whether compression is supported on the current target.
+pub fn compressionActivity(self: *const Terminal) u64 {
+    const state = &self.screens.get(.primary).?.pages.page_compression;
+    // For now we don't use the extra 16 bits.
+    return @as(u64, state.activity_serial);
 }

 /// Compress cold memory to save resident memory space.
--- a/src/terminal/main.zig
+++ b/src/terminal/main.zig
@@ -75,6 +75,9 @@ pub const Attribute = sgr.Attribute;
 pub const Options = @import("build_options.zig").Options;
 pub const options = @import("terminal_options");

+/// Whether this target supports terminal page compression.
+pub const compression_enabled = @import("mem.zig").canReclaim(.strict);
+
 /// This is set to true when we're building the C library.
 pub const c_api = if (options.c_abi) @import("c/main.zig") else void;