diff --git a/src/renderer/Thread.zig b/src/renderer/Thread.zig index bb07028ad..b0566f9d3 100644 --- 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, + }; + } +}; diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 032efe4d3..8f2f3c397 100644 --- 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) |_| diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 65a227e06..2996f365e 100644 --- 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. diff --git a/src/terminal/main.zig b/src/terminal/main.zig index 0a2e7434f..c5b27fa9b 100644 --- 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;