From 0a6ef3fda4caf96370e9132b0d971b5a52982565 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 25 Mar 2024 14:40:57 -0600 Subject: [PATCH 01/10] wip(terminal): Fast path for scroll regions --- src/terminal/PageList.zig | 78 +++++++++++++++++++++++++++++++++------ src/terminal/Screen.zig | 21 +++++------ 2 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 8086e9308..dea1a739e 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1986,6 +1986,60 @@ fn destroyPageExt( pool.nodes.destroy(page); } +/// Fast-path function to erase exactly 1 row. Erasing means that the row +/// is completely removed, not just cleared. All rows below the removed row +/// will be moved up by 1 to account for this. +pub fn eraseRow( + self: *PageList, + pt: point.Point, +) !void { + const pn = self.pin(pt).?; + + var page = pn.page; + var rows = page.data.rows.ptr(page.data.memory.ptr); + + std.mem.rotate(Row, rows[pn.y..page.data.size.rows], 1); + + { + var pin_it = self.tracked_pins.keyIterator(); + while (pin_it.next()) |p_ptr| { + const p = p_ptr.*; + if (p.page == page and p.y > pn.y) p.y -= 1; + } + } + + while (page.next) |next| { + const next_rows = next.data.rows.ptr(next.data.memory.ptr); + try page.data.cloneRowFrom(&next.data, &rows[page.data.size.rows - 1], &next_rows[0]); + + page = next; + rows = next_rows; + + std.mem.rotate(Row, rows[0..page.data.size.rows], 1); + + var pin_it = self.tracked_pins.keyIterator(); + while (pin_it.next()) |p_ptr| { + const p = p_ptr.*; + if (p.page != page) continue; + if (p.y == 0) { + p.page = page.prev.?; + p.y = p.page.data.size.rows - 1; + continue; + } + p.y -= 1; + } + } + + // The final row needs to be cleared in case we re-use it. + page.data.clearCells(&rows[page.data.size.rows - 1], 0, page.data.size.cols); + + // We don't trim off the final row if we erased active, since one of + // our invariants is that we always have full active space. + if (pt != .active) { + page.data.size.rows -= 1; + } +} + /// Erase the rows from the given top to bottom (inclusive). Erasing /// the rows doesn't clear them but actually physically REMOVES the rows. /// If the top or bottom point is in the middle of a page, the other @@ -2040,20 +2094,20 @@ pub fn eraseRows( dst.* = src.*; src.* = old_dst; - // Clear the old data in case we reuse these cells. - chunk.page.data.clearCells(src, 0, chunk.page.data.size.cols); + // // Clear the old data in case we reuse these cells. + // chunk.page.data.clearCells(src, 0, chunk.page.data.size.cols); } - // Clear our remaining cells that we didn't shift or swapped - // in case we grow back into them. - for (scroll_amount..chunk.page.data.size.rows) |i| { - const row: *Row = &rows[i]; - chunk.page.data.clearCells( - row, - 0, - chunk.page.data.size.cols, - ); - } + // // Clear our remaining cells that we didn't shift or swapped + // // in case we grow back into them. + // for (scroll_amount..chunk.page.data.size.rows) |i| { + // const row: *Row = &rows[i]; + // chunk.page.data.clearCells( + // row, + // 0, + // chunk.page.data.size.cols, + // ); + // } // Update any tracked pins to shift their y. If it was in the erased // row then we move it to the top of this page. diff --git a/src/terminal/Screen.zig b/src/terminal/Screen.zig index 6b09823a4..7c5d925f4 100644 --- a/src/terminal/Screen.zig +++ b/src/terminal/Screen.zig @@ -535,9 +535,15 @@ pub fn cursorDownScroll(self: *Screen) !void { // If we have a single-row screen, we have no rows to shift // so our cursor is in the correct place we just have to clear // the cells. - if (self.pages.rows > 1) { - // Erase rows will shift our rows up - self.pages.eraseRows(.{ .active = .{} }, .{ .active = .{} }); + if (self.pages.rows == 1) { + self.clearCells( + &self.cursor.page_pin.page.data, + self.cursor.page_row, + self.cursor.page_pin.page.data.getCells(self.cursor.page_row), + ); + } else { + // eraseRow will shift everything below it up. + try self.pages.eraseRow(.{ .active = .{} }); // The above may clear our cursor so we need to update that // again. If this fails (highly unlikely) we just reset @@ -561,15 +567,6 @@ pub fn cursorDownScroll(self: *Screen) !void { self.cursor.page_row = page_rac.row; self.cursor.page_cell = page_rac.cell; } - - // Erase rows does NOT clear the cells because in all other cases - // we never write those rows again. Active erasing is a bit - // different so we manually clear our one row. - self.clearCells( - &self.cursor.page_pin.page.data, - self.cursor.page_row, - self.cursor.page_pin.page.data.getCells(self.cursor.page_row), - ); } else { const old_pin = self.cursor.page_pin.*; From 9df9c999a7b9f1d674d417ff52c2139b9b4cf34b Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 25 Mar 2024 14:54:49 -0600 Subject: [PATCH 02/10] fix(terminal): clear erased rows Clearing these rows is necessary to avoid memory corruption, but the calls to `clearCells` in the first loop were redundant, since the rows in question are included in the second loop as well. --- src/terminal/PageList.zig | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index dea1a739e..9010407f7 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2093,21 +2093,18 @@ pub fn eraseRows( const old_dst = dst.*; dst.* = src.*; src.* = old_dst; - - // // Clear the old data in case we reuse these cells. - // chunk.page.data.clearCells(src, 0, chunk.page.data.size.cols); } - // // Clear our remaining cells that we didn't shift or swapped - // // in case we grow back into them. - // for (scroll_amount..chunk.page.data.size.rows) |i| { - // const row: *Row = &rows[i]; - // chunk.page.data.clearCells( - // row, - // 0, - // chunk.page.data.size.cols, - // ); - // } + // Clear our remaining cells that we didn't shift or swapped + // in case we grow back into them. + for (scroll_amount..chunk.page.data.size.rows) |i| { + const row: *Row = &rows[i]; + chunk.page.data.clearCells( + row, + 0, + chunk.page.data.size.cols, + ); + } // Update any tracked pins to shift their y. If it was in the erased // row then we move it to the top of this page. From ddd7f3e70666c4ff0ced3d89c07630ddf66f74e9 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 25 Mar 2024 15:15:56 -0600 Subject: [PATCH 03/10] comments --- src/terminal/PageList.zig | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 9010407f7..ce20071fc 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1998,8 +1998,13 @@ pub fn eraseRow( var page = pn.page; var rows = page.data.rows.ptr(page.data.memory.ptr); + // In order to move the following rows up we rotate the rows array by 1. + // The rotate operation turns e.g. [ 0 1 2 3 ] in to [ 1 2 3 0 ], which + // works perfectly to move all of our elements where they belong. std.mem.rotate(Row, rows[pn.y..page.data.size.rows], 1); + // We adjust the tracked pins in this page, moving up any that were below + // the removed row. { var pin_it = self.tracked_pins.keyIterator(); while (pin_it.next()) |p_ptr| { @@ -2008,8 +2013,26 @@ pub fn eraseRow( } } + // We iterate through all of the following pages in order to move their + // rows up by 1 as well. while (page.next) |next| { const next_rows = next.data.rows.ptr(next.data.memory.ptr); + + // We take the top row of the page and clone it in to the bottom + // row of the previous page, which gets rid of the top row that was + // rotated down in the previous page, and accounts for the row in + // this page that will be rotated down as well. + // + // rotate -> clone --> rotate -> result + // 0 -. 1 1 1 + // 1 | 2 2 2 + // 2 | 3 3 3 + // 3 <' 0 <. 4 4 + // --- --- | --- --- + // 4 4 -' 4 -. 5 + // 5 5 5 | 6 + // 6 6 6 | 7 + // 7 7 7 <' 4 try page.data.cloneRowFrom(&next.data, &rows[page.data.size.rows - 1], &next_rows[0]); page = next; @@ -2017,6 +2040,9 @@ pub fn eraseRow( std.mem.rotate(Row, rows[0..page.data.size.rows], 1); + // Our tracked pins for this page need to be updated. + // If the pin is in row 0 that means the corresponding row has + // been moved to the previous page. Otherwise, move it up by 1. var pin_it = self.tracked_pins.keyIterator(); while (pin_it.next()) |p_ptr| { const p = p_ptr.*; From d74ea890564ed018769fecd929226357feef2ff4 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 25 Mar 2024 19:12:55 -0600 Subject: [PATCH 04/10] fastmem: rotateOnce --- src/fastmem.zig | 8 ++++++++ src/terminal/PageList.zig | 5 +++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/fastmem.zig b/src/fastmem.zig index 0e9a444ee..d29087956 100644 --- a/src/fastmem.zig +++ b/src/fastmem.zig @@ -22,5 +22,13 @@ pub inline fn copy(comptime T: type, dest: []T, source: []const T) void { } } +/// Same as std.mem.rotate(T, items, 1) but more efficient by using memmove +/// and a tmp var for the single rotated item instead of 3 calls to reverse. +pub inline fn rotateOnce(comptime T: type, items: []T) void { + const tmp = items[0]; + move(T, items[0..items.len - 1], items[1..items.len]); + items[items.len - 1] = tmp; +} + extern "c" fn memcpy(*anyopaque, *const anyopaque, usize) *anyopaque; extern "c" fn memmove(*anyopaque, *const anyopaque, usize) *anyopaque; diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index ce20071fc..9b6caceaf 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -6,6 +6,7 @@ const PageList = @This(); const std = @import("std"); const Allocator = std.mem.Allocator; const assert = std.debug.assert; +const fastmem = @import("../fastmem.zig"); const point = @import("point.zig"); const pagepkg = @import("page.zig"); const stylepkg = @import("style.zig"); @@ -2001,7 +2002,7 @@ pub fn eraseRow( // In order to move the following rows up we rotate the rows array by 1. // The rotate operation turns e.g. [ 0 1 2 3 ] in to [ 1 2 3 0 ], which // works perfectly to move all of our elements where they belong. - std.mem.rotate(Row, rows[pn.y..page.data.size.rows], 1); + fastmem.rotateOnce(Row, rows[pn.y..page.data.size.rows]); // We adjust the tracked pins in this page, moving up any that were below // the removed row. @@ -2038,7 +2039,7 @@ pub fn eraseRow( page = next; rows = next_rows; - std.mem.rotate(Row, rows[0..page.data.size.rows], 1); + fastmem.rotateOnce(Row, rows[0..page.data.size.rows]); // Our tracked pins for this page need to be updated. // If the pin is in row 0 that means the corresponding row has From 23d32e248e6859158712cd4ce208d7af3f5a859f Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 25 Mar 2024 19:16:34 -0600 Subject: [PATCH 05/10] perf(terminal): fast-paths for scrolling regions --- src/terminal/PageList.zig | 103 ++++++++++++++++++++++++++++++++++++++ src/terminal/Terminal.zig | 43 +++++++++++++++- 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 9b6caceaf..83df038fe 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -2067,6 +2067,109 @@ pub fn eraseRow( } } +/// A special-case of eraseRow that shifts only a bounded number of following +/// rows up, filling the space they leave behind with blank rows. +/// +/// `limit` is exclusive of the erased row. A limit of 1 will erase the target +/// row and shift the row below in to its position, leaving a blank row below. +/// +/// This function has a lot of repeated code in it because it is a hot path. +pub fn eraseRowBounded( + self: *PageList, + pt: point.Point, + limit: usize, +) !void { + const pn = self.pin(pt).?; + + var page = pn.page; + var rows = page.data.rows.ptr(page.data.memory.ptr); + + // Special case where we'll reach the limit in the same page as the erased + // row, so we don't have to handle cloning rows between pages. + if (page.data.size.rows - pn.y > limit) { + page.data.clearCells(&rows[pn.y], 0, page.data.size.cols); + fastmem.rotateOnce(Row, rows[pn.y..][0..limit + 1]); + + // Update pins in the shifted region. + var pin_it = self.tracked_pins.keyIterator(); + while (pin_it.next()) |p_ptr| { + const p = p_ptr.*; + if (p.page == page and p.y > pn.y and p.y < pn.y + limit) p.y -= 1; + } + + return; + } + + var shifted: usize = 0; + + fastmem.rotateOnce(Row, rows[pn.y..page.data.size.rows]); + + shifted += page.data.size.rows - pn.y; + + // We adjust the tracked pins in this page, moving up any that were below + // the removed row. + { + var pin_it = self.tracked_pins.keyIterator(); + while (pin_it.next()) |p_ptr| { + const p = p_ptr.*; + if (p.page == page and p.y > pn.y) p.y -= 1; + } + } + + while (page.next) |next| { + const next_rows = next.data.rows.ptr(next.data.memory.ptr); + + try page.data.cloneRowFrom(&next.data, &rows[page.data.size.rows - 1], &next_rows[0]); + + page = next; + rows = next_rows; + + const shifted_limit = limit - shifted; + if (page.data.size.rows > shifted_limit) { + page.data.clearCells(&rows[0], 0, page.data.size.cols); + fastmem.rotateOnce(Row, rows[0..shifted_limit + 1]); + + // Update pins in the shifted region. + var pin_it = self.tracked_pins.keyIterator(); + while (pin_it.next()) |p_ptr| { + const p = p_ptr.*; + if (p.page != page or p.y > shifted_limit) continue; + if (p.y == 0) { + p.page = page.prev.?; + p.y = p.page.data.size.rows - 1; + continue; + } + p.y -= 1; + } + + return; + } + + fastmem.rotateOnce(Row, rows[0..page.data.size.rows]); + + shifted += page.data.size.rows; + + // Our tracked pins for this page need to be updated. + // If the pin is in row 0 that means the corresponding row has + // been moved to the previous page. Otherwise, move it up by 1. + var pin_it = self.tracked_pins.keyIterator(); + while (pin_it.next()) |p_ptr| { + const p = p_ptr.*; + if (p.page != page) continue; + if (p.y == 0) { + p.page = page.prev.?; + p.y = p.page.data.size.rows - 1; + continue; + } + p.y -= 1; + } + } + + // We reached the end of the page list before the limit, so we clear + // the final row since it was rotated down from the top of this page. + page.data.clearCells(&rows[page.data.size.rows - 1], 0, page.data.size.cols); +} + /// Erase the rows from the given top to bottom (inclusive). Erasing /// the rows doesn't clear them but actually physically REMOVES the rows. /// If the top or bottom point is in the middle of a page, the other diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 989086673..efce17816 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1084,7 +1084,48 @@ pub fn index(self: *Terminal) !void { { try self.screen.cursorDownScroll(); } else { - self.scrollUp(1); + // Slow path for left and right scrolling region margins. + if (self.scrolling_region.left != 0 and + self.scrolling_region.right != self.cols - 1) + { + self.scrollUp(1); + return; + } + + // Otherwise use a fast path function from PageList to efficiently + // scroll the contents of the scrolling region. + if (self.scrolling_region.bottom < self.rows) { + try self.screen.pages.eraseRowBounded( + .{ .active = .{ .y = self.scrolling_region.top } }, + self.scrolling_region.bottom - self.scrolling_region.top + ); + } else { + // If we have no bottom margin we don't need to worry about + // potentially damaging rows below the scrolling region, + // and eraseRow is cheaper than eraseRowBounded. + try self.screen.pages.eraseRow( + .{ .active = .{ .y = self.scrolling_region.top } }, + ); + } + + // The operations above can prune our cursor style so we need to + // update. This should never fail because the above can only FREE + // memory. + self.screen.manualStyleUpdate() catch |err| { + std.log.warn("deleteLines manualStyleUpdate err={}", .{err}); + self.screen.cursor.style = .{}; + self.screen.manualStyleUpdate() catch unreachable; + }; + + // We scrolled with the cursor on the bottom row of the scrolling + // region, so we should move the cursor to the bottom left. + self.screen.cursorAbsolute( + self.scrolling_region.left, + self.scrolling_region.bottom, + ); + + // Always unset pending wrap + self.screen.cursor.pending_wrap = false; } return; From aadf795d282530105b78d0a976ddbf5ba63e687e Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 25 Mar 2024 19:38:39 -0600 Subject: [PATCH 06/10] fix(terminal): correctly use slow path for left/right scroll margin --- src/terminal/Terminal.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index efce17816..a541c4e07 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1085,7 +1085,7 @@ pub fn index(self: *Terminal) !void { try self.screen.cursorDownScroll(); } else { // Slow path for left and right scrolling region margins. - if (self.scrolling_region.left != 0 and + if (self.scrolling_region.left != 0 or self.scrolling_region.right != self.cols - 1) { self.scrollUp(1); From 2274b8a912f772e8b25b52471563028ccae19a69 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Mon, 25 Mar 2024 19:39:35 -0600 Subject: [PATCH 07/10] fix(terminal): don't reset x when indexing in scroll region --- src/terminal/Terminal.zig | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index a541c4e07..43299df45 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1094,6 +1094,15 @@ pub fn index(self: *Terminal) !void { // Otherwise use a fast path function from PageList to efficiently // scroll the contents of the scrolling region. + + // eraseRow and eraseRowBounded will end up moving the cursor pin + // up by 1, so we save its current position and restore it after. + const cursor_x = self.screen.cursor.x; + const cursor_y = self.screen.cursor.y; + defer { + self.screen.cursorAbsolute(cursor_x, cursor_y); + } + if (self.scrolling_region.bottom < self.rows) { try self.screen.pages.eraseRowBounded( .{ .active = .{ .y = self.scrolling_region.top } }, @@ -1116,16 +1125,6 @@ pub fn index(self: *Terminal) !void { self.screen.cursor.style = .{}; self.screen.manualStyleUpdate() catch unreachable; }; - - // We scrolled with the cursor on the bottom row of the scrolling - // region, so we should move the cursor to the bottom left. - self.screen.cursorAbsolute( - self.scrolling_region.left, - self.scrolling_region.bottom, - ); - - // Always unset pending wrap - self.screen.cursor.pending_wrap = false; } return; From 492e147e26a98e219ac3b1e4fd712fbcee945478 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Tue, 26 Mar 2024 10:19:23 -0600 Subject: [PATCH 08/10] terminal: clean up some code and comments --- src/terminal/PageList.zig | 53 ++++++++++++++++++++++----------------- src/terminal/Terminal.zig | 17 +++---------- 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 83df038fe..bacb553b0 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -1988,8 +1988,12 @@ fn destroyPageExt( } /// Fast-path function to erase exactly 1 row. Erasing means that the row -/// is completely removed, not just cleared. All rows below the removed row -/// will be moved up by 1 to account for this. +/// is completely REMOVED, not just cleared. All rows following the removed +/// row will be shifted up by 1 to fill the empty space. +/// +/// Unlike eraseRows, eraseRow does not change the size of any pages. The +/// caller is responsible for adjusting the row count of the final page if +/// that behavior is required. pub fn eraseRow( self: *PageList, pt: point.Point, @@ -2029,7 +2033,7 @@ pub fn eraseRow( // 1 | 2 2 2 // 2 | 3 3 3 // 3 <' 0 <. 4 4 - // --- --- | --- --- + // --- --- | --- --- <- page boundary // 4 4 -' 4 -. 5 // 5 5 5 | 6 // 6 6 6 | 7 @@ -2057,35 +2061,34 @@ pub fn eraseRow( } } - // The final row needs to be cleared in case we re-use it. + // Clear the final row which was rotated from the top of the page. page.data.clearCells(&rows[page.data.size.rows - 1], 0, page.data.size.cols); - - // We don't trim off the final row if we erased active, since one of - // our invariants is that we always have full active space. - if (pt != .active) { - page.data.size.rows -= 1; - } } -/// A special-case of eraseRow that shifts only a bounded number of following +/// A variant of eraseRow that shifts only a bounded number of following /// rows up, filling the space they leave behind with blank rows. /// /// `limit` is exclusive of the erased row. A limit of 1 will erase the target /// row and shift the row below in to its position, leaving a blank row below. -/// -/// This function has a lot of repeated code in it because it is a hot path. pub fn eraseRowBounded( self: *PageList, pt: point.Point, limit: usize, ) !void { + // This function has a lot of repeated code in it because it is a hot path. + // + // To get a better idea of what's happening, read eraseRow first for more + // in-depth explanatory comments. To avoid repetition, the only comments for + // this function are for where it differs from eraseRow. + const pn = self.pin(pt).?; var page = pn.page; var rows = page.data.rows.ptr(page.data.memory.ptr); - // Special case where we'll reach the limit in the same page as the erased - // row, so we don't have to handle cloning rows between pages. + // If the row limit is less than the remaining rows before the end of the + // page, then we clear the row, rotate it to the end of the boundary limit + // and update our pins. if (page.data.size.rows - pn.y > limit) { page.data.clearCells(&rows[pn.y], 0, page.data.size.cols); fastmem.rotateOnce(Row, rows[pn.y..][0..limit + 1]); @@ -2100,14 +2103,14 @@ pub fn eraseRowBounded( return; } - var shifted: usize = 0; - fastmem.rotateOnce(Row, rows[pn.y..page.data.size.rows]); - shifted += page.data.size.rows - pn.y; + // We need to keep track of how many rows we've shifted so that we can + // determine at what point we need to do a partial shift on subsequent + // pages. + var shifted: usize = page.data.size.rows - pn.y; - // We adjust the tracked pins in this page, moving up any that were below - // the removed row. + // Update tracked pins. { var pin_it = self.tracked_pins.keyIterator(); while (pin_it.next()) |p_ptr| { @@ -2124,6 +2127,11 @@ pub fn eraseRowBounded( page = next; rows = next_rows; + // We check to see if this page contains enough rows to satisfy the + // specified limit, accounting for rows we've already shifted in prior + // pages. + // + // The logic here is very similar to the one before the loop. const shifted_limit = limit - shifted; if (page.data.size.rows > shifted_limit) { page.data.clearCells(&rows[0], 0, page.data.size.cols); @@ -2147,11 +2155,10 @@ pub fn eraseRowBounded( fastmem.rotateOnce(Row, rows[0..page.data.size.rows]); + // Account for the rows shifted in this page. shifted += page.data.size.rows; - // Our tracked pins for this page need to be updated. - // If the pin is in row 0 that means the corresponding row has - // been moved to the previous page. Otherwise, move it up by 1. + // Update tracked pins. var pin_it = self.tracked_pins.keyIterator(); while (pin_it.next()) |p_ptr| { const p = p_ptr.*; diff --git a/src/terminal/Terminal.zig b/src/terminal/Terminal.zig index 43299df45..52267fa13 100644 --- a/src/terminal/Terminal.zig +++ b/src/terminal/Terminal.zig @@ -1103,19 +1103,10 @@ pub fn index(self: *Terminal) !void { self.screen.cursorAbsolute(cursor_x, cursor_y); } - if (self.scrolling_region.bottom < self.rows) { - try self.screen.pages.eraseRowBounded( - .{ .active = .{ .y = self.scrolling_region.top } }, - self.scrolling_region.bottom - self.scrolling_region.top - ); - } else { - // If we have no bottom margin we don't need to worry about - // potentially damaging rows below the scrolling region, - // and eraseRow is cheaper than eraseRowBounded. - try self.screen.pages.eraseRow( - .{ .active = .{ .y = self.scrolling_region.top } }, - ); - } + try self.screen.pages.eraseRowBounded( + .{ .active = .{ .y = self.scrolling_region.top } }, + self.scrolling_region.bottom - self.scrolling_region.top + ); // The operations above can prune our cursor style so we need to // update. This should never fail because the above can only FREE From d72eb30a26433e24b511c9d25b18334b3ef4fc31 Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Tue, 26 Mar 2024 12:03:37 -0600 Subject: [PATCH 09/10] fastmem: fix doc comment --- src/fastmem.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fastmem.zig b/src/fastmem.zig index d29087956..8f32bc3c8 100644 --- a/src/fastmem.zig +++ b/src/fastmem.zig @@ -12,7 +12,7 @@ pub inline fn move(comptime T: type, dest: []T, source: []const T) void { } } -/// Same as std.mem.copyForwards but prefers libc memcpy if it is available +/// Same as @memcpy but prefers libc memcpy if it is available /// because it is generally much faster. pub inline fn copy(comptime T: type, dest: []T, source: []const T) void { if (builtin.link_libc) { From d17344b85502952ea793d4b37c44159a13b78cdb Mon Sep 17 00:00:00 2001 From: Qwerasd Date: Tue, 26 Mar 2024 12:04:41 -0600 Subject: [PATCH 10/10] perf(terminal/page): @memset micro-optimization --- src/terminal/page.zig | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/terminal/page.zig b/src/terminal/page.zig index dc73c9fce..d100acc89 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -182,7 +182,9 @@ pub const Page = struct { /// Reinitialize the page with the same capacity. pub fn reinit(self: *Page) void { - @memset(self.memory, 0); + // We zero the page memory as u64 instead of u8 because + // we can and it's empirically quite a bit faster. + @memset(@as([*]u64, @ptrCast(self.memory))[0..self.memory.len / 8], 0); self.* = initBuf(OffsetBuf.init(self.memory), layout(self.capacity)); } @@ -654,7 +656,10 @@ pub const Page = struct { // Clear our source row now that the copy is complete. We can NOT // use clearCells here because clearCells will garbage collect our // styles and graphames but we moved them above. - @memset(src_cells, .{}); + // + // Zero the cells as u64s since empirically this seems + // to be a bit faster than using @memset(src_cells, .{}) + @memset(@as([]u64, @ptrCast(src_cells)), 0); if (src_cells.len == self.size.cols) { src_row.grapheme = false; src_row.styled = false; @@ -734,7 +739,9 @@ pub const Page = struct { if (cells.len == self.size.cols) row.styled = false; } - @memset(cells, .{}); + // Zero the cells as u64s since empirically this seems + // to be a bit faster than using @memset(cells, .{}) + @memset(@as([]u64, @ptrCast(cells)), 0); } /// Append a codepoint to the given cell as a grapheme.