test: big perf win by pausing integ checks while growing pages (#2956)

In multiple tests we create 1 or more pages by growing them 1 row at a
time, which results in an integrity check of the page for each row grown
which is just... horrible. By simply pausing integrity checks while
growing these pages (since growing them is not the point of the test) we
MASSIVELY speed up all of these tests.

Also reduced grapheme bytes during testing and made the Terminal "glitch
text" test actually assert what it intends to achieve, rather than just
blindly assuming 100 copies of the text will be the right amount -- this
lets us stop a lot earlier, making it take practically no time.
This commit is contained in:
Mitchell Hashimoto
2024-12-12 16:12:22 -08:00
committed by GitHub
4 changed files with 114 additions and 11 deletions

View File

@ -3803,10 +3803,18 @@ test "PageList pointFromPin active from prior page" {
var s = try init(alloc, 80, 24, null);
defer s.deinit();
// Grow so we take up at least 5 pages.
const page = &s.pages.last.?.data;
var cur_page = s.pages.last.?;
cur_page.data.pauseIntegrityChecks(true);
for (0..page.capacity.rows * 5) |_| {
_ = try s.grow();
if (try s.grow()) |new_page| {
cur_page.data.pauseIntegrityChecks(false);
cur_page = new_page;
cur_page.data.pauseIntegrityChecks(true);
}
}
cur_page.data.pauseIntegrityChecks(false);
{
try testing.expectEqual(point.Point{
@ -3837,10 +3845,19 @@ test "PageList pointFromPin traverse pages" {
var s = try init(alloc, 80, 24, null);
defer s.deinit();
// Grow so we take up at least 2 pages.
const page = &s.pages.last.?.data;
var cur_page = s.pages.last.?;
cur_page.data.pauseIntegrityChecks(true);
for (0..page.capacity.rows * 2) |_| {
_ = try s.grow();
if (try s.grow()) |new_page| {
cur_page.data.pauseIntegrityChecks(false);
cur_page = new_page;
cur_page.data.pauseIntegrityChecks(true);
}
}
cur_page.data.pauseIntegrityChecks(false);
{
const pages = s.totalPages();
@ -4530,9 +4547,11 @@ test "PageList pageIterator two pages" {
// Grow to capacity
const page1_node = s.pages.last.?;
const page1 = page1_node.data;
page1_node.data.pauseIntegrityChecks(true);
for (0..page1.capacity.rows - page1.size.rows) |_| {
try testing.expect(try s.grow() == null);
}
page1_node.data.pauseIntegrityChecks(false);
try testing.expect(try s.grow() != null);
// Iterate the active area
@ -4564,9 +4583,11 @@ test "PageList pageIterator history two pages" {
// Grow to capacity
const page1_node = s.pages.last.?;
const page1 = page1_node.data;
page1_node.data.pauseIntegrityChecks(true);
for (0..page1.capacity.rows - page1.size.rows) |_| {
try testing.expect(try s.grow() == null);
}
page1_node.data.pauseIntegrityChecks(false);
try testing.expect(try s.grow() != null);
// Iterate the active area
@ -4615,9 +4636,11 @@ test "PageList pageIterator reverse two pages" {
// Grow to capacity
const page1_node = s.pages.last.?;
const page1 = page1_node.data;
page1_node.data.pauseIntegrityChecks(true);
for (0..page1.capacity.rows - page1.size.rows) |_| {
try testing.expect(try s.grow() == null);
}
page1_node.data.pauseIntegrityChecks(false);
try testing.expect(try s.grow() != null);
// Iterate the active area
@ -4653,9 +4676,11 @@ test "PageList pageIterator reverse history two pages" {
// Grow to capacity
const page1_node = s.pages.last.?;
const page1 = page1_node.data;
page1_node.data.pauseIntegrityChecks(true);
for (0..page1.capacity.rows - page1.size.rows) |_| {
try testing.expect(try s.grow() == null);
}
page1_node.data.pauseIntegrityChecks(false);
try testing.expect(try s.grow() != null);
// Iterate the active area
@ -4781,9 +4806,16 @@ test "PageList erase" {
// Grow so we take up at least 5 pages.
const page = &s.pages.last.?.data;
var cur_page = s.pages.last.?;
cur_page.data.pauseIntegrityChecks(true);
for (0..page.capacity.rows * 5) |_| {
_ = try s.grow();
if (try s.grow()) |new_page| {
cur_page.data.pauseIntegrityChecks(false);
cur_page = new_page;
cur_page.data.pauseIntegrityChecks(true);
}
}
cur_page.data.pauseIntegrityChecks(false);
try testing.expectEqual(@as(usize, 6), s.totalPages());
// Our total rows should be large
@ -4808,9 +4840,16 @@ test "PageList erase reaccounts page size" {
// Grow so we take up at least 5 pages.
const page = &s.pages.last.?.data;
var cur_page = s.pages.last.?;
cur_page.data.pauseIntegrityChecks(true);
for (0..page.capacity.rows * 5) |_| {
_ = try s.grow();
if (try s.grow()) |new_page| {
cur_page.data.pauseIntegrityChecks(false);
cur_page = new_page;
cur_page.data.pauseIntegrityChecks(true);
}
}
cur_page.data.pauseIntegrityChecks(false);
try testing.expect(s.page_size > start_size);
// Erase the entire history, we should be back to just our active set.
@ -4827,9 +4866,16 @@ test "PageList erase row with tracked pin resets to top-left" {
// Grow so we take up at least 5 pages.
const page = &s.pages.last.?.data;
var cur_page = s.pages.last.?;
cur_page.data.pauseIntegrityChecks(true);
for (0..page.capacity.rows * 5) |_| {
_ = try s.grow();
if (try s.grow()) |new_page| {
cur_page.data.pauseIntegrityChecks(false);
cur_page = new_page;
cur_page.data.pauseIntegrityChecks(true);
}
}
cur_page.data.pauseIntegrityChecks(false);
// Our total rows should be large
try testing.expect(s.totalRows() > s.rows);
@ -4899,9 +4945,16 @@ test "PageList erase resets viewport to active if moves within active" {
// Grow so we take up at least 5 pages.
const page = &s.pages.last.?.data;
var cur_page = s.pages.last.?;
cur_page.data.pauseIntegrityChecks(true);
for (0..page.capacity.rows * 5) |_| {
_ = try s.grow();
if (try s.grow()) |new_page| {
cur_page.data.pauseIntegrityChecks(false);
cur_page = new_page;
cur_page.data.pauseIntegrityChecks(true);
}
}
cur_page.data.pauseIntegrityChecks(false);
// Move our viewport to the top
s.scroll(.{ .delta_row = -@as(isize, @intCast(s.totalRows())) });
@ -4922,9 +4975,16 @@ test "PageList erase resets viewport if inside erased page but not active" {
// Grow so we take up at least 5 pages.
const page = &s.pages.last.?.data;
var cur_page = s.pages.last.?;
cur_page.data.pauseIntegrityChecks(true);
for (0..page.capacity.rows * 5) |_| {
_ = try s.grow();
if (try s.grow()) |new_page| {
cur_page.data.pauseIntegrityChecks(false);
cur_page = new_page;
cur_page.data.pauseIntegrityChecks(true);
}
}
cur_page.data.pauseIntegrityChecks(false);
// Move our viewport to the top
s.scroll(.{ .delta_row = -@as(isize, @intCast(s.totalRows())) });
@ -4946,9 +5006,16 @@ test "PageList erase resets viewport to active if top is inside active" {
// Grow so we take up at least 5 pages.
const page = &s.pages.last.?.data;
var cur_page = s.pages.last.?;
cur_page.data.pauseIntegrityChecks(true);
for (0..page.capacity.rows * 5) |_| {
_ = try s.grow();
if (try s.grow()) |new_page| {
cur_page.data.pauseIntegrityChecks(false);
cur_page = new_page;
cur_page.data.pauseIntegrityChecks(true);
}
}
cur_page.data.pauseIntegrityChecks(false);
// Move our viewport to the top
s.scroll(.{ .top = {} });
@ -5106,7 +5173,9 @@ test "PageList eraseRowBounded full rows two pages" {
// Grow to two pages so our active area straddles
{
const page = &s.pages.last.?.data;
page.pauseIntegrityChecks(true);
for (0..page.capacity.rows - page.size.rows) |_| _ = try s.grow();
page.pauseIntegrityChecks(false);
try s.growRows(5);
try testing.expectEqual(@as(usize, 2), s.totalPages());
try testing.expectEqual(@as(usize, 5), s.pages.last.?.data.size.rows);
@ -6435,9 +6504,11 @@ test "PageList resize reflow more cols wrap across page boundary" {
// Grow to the capacity of the first page.
{
const page = &s.pages.first.?.data;
page.pauseIntegrityChecks(true);
for (page.size.rows..page.capacity.rows) |_| {
_ = try s.grow();
}
page.pauseIntegrityChecks(false);
try testing.expectEqual(@as(usize, 1), s.totalPages());
try s.growRows(1);
try testing.expectEqual(@as(usize, 2), s.totalPages());
@ -6564,9 +6635,11 @@ test "PageList resize reflow more cols wrap across page boundary cursor in secon
// Grow to the capacity of the first page.
{
const page = &s.pages.first.?.data;
page.pauseIntegrityChecks(true);
for (page.size.rows..page.capacity.rows) |_| {
_ = try s.grow();
}
page.pauseIntegrityChecks(false);
try testing.expectEqual(@as(usize, 1), s.totalPages());
try s.growRows(1);
try testing.expectEqual(@as(usize, 2), s.totalPages());
@ -6648,9 +6721,11 @@ test "PageList resize reflow less cols wrap across page boundary cursor in secon
// Grow to the capacity of the first page.
{
const page = &s.pages.first.?.data;
page.pauseIntegrityChecks(true);
for (page.size.rows..page.capacity.rows) |_| {
_ = try s.grow();
}
page.pauseIntegrityChecks(false);
try testing.expectEqual(@as(usize, 1), s.totalPages());
try s.growRows(5);
try testing.expectEqual(@as(usize, 2), s.totalPages());

View File

@ -3048,9 +3048,11 @@ test "Screen cursorCopy style deref new page" {
// Fill the scrollback with blank lines until
// there are only 5 rows left on the first page.
s2.pages.pages.first.?.data.pauseIntegrityChecks(true);
for (0..first_page_size - 5) |_| {
try s2.testWriteString("\n");
}
s2.pages.pages.first.?.data.pauseIntegrityChecks(false);
try s2.testWriteString("1\n2\n3\n4\n5\n6\n7\n8\n9\n10");
@ -3157,9 +3159,11 @@ test "Screen cursorCopy hyperlink deref new page" {
// Fill the scrollback with blank lines until
// there are only 5 rows left on the first page.
s2.pages.pages.first.?.data.pauseIntegrityChecks(true);
for (0..first_page_size - 5) |_| {
try s2.testWriteString("\n");
}
s2.pages.pages.first.?.data.pauseIntegrityChecks(false);
try s2.testWriteString("1\n2\n3\n4\n5\n6\n7\n8\n9\n10");
@ -3588,7 +3592,9 @@ test "Screen: cursorDown across pages preserves style" {
// Scroll down enough to go to another page
const start_page = &s.pages.pages.last.?.data;
const rem = start_page.capacity.rows;
start_page.pauseIntegrityChecks(true);
for (0..rem) |_| try s.cursorDownOrScroll();
start_page.pauseIntegrityChecks(false);
// We need our page to change for this test o make sense. If this
// assertion fails then the bug is in the test: we should be scrolling
@ -3638,7 +3644,9 @@ test "Screen: cursorUp across pages preserves style" {
// Scroll down enough to go to another page
const start_page = &s.pages.pages.last.?.data;
const rem = start_page.capacity.rows;
start_page.pauseIntegrityChecks(true);
for (0..rem) |_| try s.cursorDownOrScroll();
start_page.pauseIntegrityChecks(false);
// We need our page to change for this test o make sense. If this
// assertion fails then the bug is in the test: we should be scrolling
@ -3683,7 +3691,9 @@ test "Screen: cursorAbsolute across pages preserves style" {
// Scroll down enough to go to another page
const start_page = &s.pages.pages.last.?.data;
const rem = start_page.capacity.rows;
start_page.pauseIntegrityChecks(true);
for (0..rem) |_| try s.cursorDownOrScroll();
start_page.pauseIntegrityChecks(false);
// We need our page to change for this test o make sense. If this
// assertion fails then the bug is in the test: we should be scrolling
@ -3822,7 +3832,9 @@ test "Screen: scrolling across pages preserves style" {
// Scroll down enough to go to another page
const rem = start_page.capacity.rows - start_page.size.rows + 1;
for (0..rem) |_| try s.cursorDownScroll();
start_page.pauseIntegrityChecks(true);
for (0..rem) |_| try s.cursorDownOrScroll();
start_page.pauseIntegrityChecks(false);
// We need our page to change for this test o make sense. If this
// assertion fails then the bug is in the test: we should be scrolling
@ -4303,7 +4315,9 @@ test "Screen: scroll above same page but cursor on previous page" {
// We need to get the cursor to a new page
const first_page_size = s.pages.pages.first.?.data.capacity.rows;
s.pages.pages.first.?.data.pauseIntegrityChecks(true);
for (0..first_page_size - 3) |_| try s.testWriteString("\n");
s.pages.pages.first.?.data.pauseIntegrityChecks(false);
try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } });
try s.testWriteString("1A\n2B\n3C\n4D\n5E");
@ -4361,7 +4375,9 @@ test "Screen: scroll above same page but cursor on previous page last row" {
// We need to get the cursor to a new page
const first_page_size = s.pages.pages.first.?.data.capacity.rows;
s.pages.pages.first.?.data.pauseIntegrityChecks(true);
for (0..first_page_size - 2) |_| try s.testWriteString("\n");
s.pages.pages.first.?.data.pauseIntegrityChecks(false);
try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } });
try s.testWriteString("1A\n2B\n3C\n4D\n5E");
@ -4436,7 +4452,9 @@ test "Screen: scroll above creates new page" {
// We need to get the cursor to a new page
const first_page_size = s.pages.pages.first.?.data.capacity.rows;
s.pages.pages.first.?.data.pauseIntegrityChecks(true);
for (0..first_page_size - 3) |_| try s.testWriteString("\n");
s.pages.pages.first.?.data.pauseIntegrityChecks(false);
try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } });
try s.testWriteString("1ABCD\n2EFGH\n3IJKL");
@ -4477,7 +4495,9 @@ test "Screen: scroll above no scrollback bottom of page" {
defer s.deinit();
const first_page_size = s.pages.pages.first.?.data.capacity.rows;
s.pages.pages.first.?.data.pauseIntegrityChecks(true);
for (0..first_page_size - 3) |_| try s.testWriteString("\n");
s.pages.pages.first.?.data.pauseIntegrityChecks(false);
try s.setAttribute(.{ .direct_color_bg = .{ .r = 155 } });
try s.testWriteString("1ABCD\n2EFGH\n3IJKL");
@ -8254,9 +8274,11 @@ test "Screen: selectionString multi-page" {
const first_page_size = s.pages.pages.first.?.data.capacity.rows;
// Lazy way to seek to the first page boundary.
s.pages.pages.first.?.data.pauseIntegrityChecks(true);
for (0..first_page_size - 1) |_| {
try s.testWriteString("\n");
}
s.pages.pages.first.?.data.pauseIntegrityChecks(false);
try s.testWriteString("y\ny\ny");

View File

@ -2763,9 +2763,15 @@ test "Terminal: input glitch text" {
var t = try init(alloc, .{ .cols = 30, .rows = 30 });
defer t.deinit(alloc);
for (0..100) |_| {
const page = t.screen.pages.pages.first.?;
const grapheme_cap = page.data.capacity.grapheme_bytes;
while (page.data.capacity.grapheme_bytes == grapheme_cap) {
try t.printString(glitch);
}
// We're testing to make sure that grapheme capacity gets increased.
try testing.expect(page.data.capacity.grapheme_bytes > grapheme_cap);
}
test "Terminal: zero-width character at start" {

View File

@ -1795,7 +1795,7 @@ pub const std_capacity: Capacity = .{
.cols = 215,
.rows = 215,
.styles = 128,
.grapheme_bytes = 8192,
.grapheme_bytes = if (builtin.is_test) 512 else 8192,
};
/// The size of this page.