mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-07-10 11:19:45 +00:00
terminal: track page ownership explicitly on pagelist nodes (#13241)
PageList decided whether a page's backing memory belongs to the memory pool or the heap by comparing its length against std_size. This was super error-prone and the source of many bugs historically. We locked it down but its bothered me and has gotten in the way of another feature I've wanted to do: memory compaction. First, this commit records the ownership explicitly on each node and uses it everywhere ownership was previously inferred from size. Second, createPage gains an exact_size option that forces an exact-size heap allocation even when the layout would fit a pool item. Third, compact() now uses it to shrink any page to its minimum size, including standard pages. Nothing calls compact [YET!] but this is going to be the key to compressing scrollback history. This is groundwork for a ton of memory savings. Coming soon.
This commit is contained in:
@@ -46,6 +46,22 @@ const Node = struct {
|
||||
next: ?*Node = null,
|
||||
data: Page,
|
||||
serial: u64,
|
||||
|
||||
/// How the backing memory of `data` was allocated. Pool-owned
|
||||
/// memory is always a full standard-size item from the memory
|
||||
/// pool, regardless of the page layout size. Heap-owned memory
|
||||
/// is allocated directly with the page allocator and is exactly
|
||||
/// `data.memory.len` bytes.
|
||||
///
|
||||
/// This must never be inferred from the memory length: heap-owned
|
||||
/// pages can be smaller than the standard size (e.g. compacted
|
||||
/// pages), and returning one to the pool would corrupt it.
|
||||
///
|
||||
/// This has no default on purpose so that every construction site
|
||||
/// is forced to make an explicit decision.
|
||||
owned: Owned,
|
||||
|
||||
const Owned = enum { pool, heap };
|
||||
};
|
||||
|
||||
/// The memory pool we get page nodes from.
|
||||
@@ -444,13 +460,14 @@ fn initPages(
|
||||
// redundant here for safety.
|
||||
assert(layout.total_size <= size.max_page_size);
|
||||
|
||||
// If we have an error, we need to clean up our non-standard pages
|
||||
// If we have an error, we need to clean up our heap-owned pages
|
||||
// since they're not in the pool.
|
||||
errdefer {
|
||||
var it = page_list.first;
|
||||
while (it) |node| : (it = node.next) {
|
||||
if (node.data.memory.len > std_size) {
|
||||
page_alloc.free(node.data.memory);
|
||||
switch (node.owned) {
|
||||
.pool => {},
|
||||
.heap => page_alloc.free(node.data.memory),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -490,6 +507,7 @@ fn initPages(
|
||||
node.* = .{
|
||||
.data = .initBuf(.init(page_buf), layout),
|
||||
.serial = serial.*,
|
||||
.owned = if (pooled) .pool else .heap,
|
||||
};
|
||||
node.data.size.rows = @min(rem, node.data.capacity.rows);
|
||||
rem -= node.data.size.rows;
|
||||
@@ -634,12 +652,13 @@ pub fn deinit(self: *PageList) void {
|
||||
self.tracked_pins.deinit(self.pool.alloc);
|
||||
|
||||
// Go through our linked list and deallocate all pages that are
|
||||
// not standard size.
|
||||
// heap-owned (not in the pool).
|
||||
const page_alloc = self.pool.pages.arena.child_allocator;
|
||||
var it = self.pages.first;
|
||||
while (it) |node| : (it = node.next) {
|
||||
if (node.data.memory.len > std_size) {
|
||||
page_alloc.free(node.data.memory);
|
||||
switch (node.owned) {
|
||||
.pool => {},
|
||||
.heap => page_alloc.free(node.data.memory),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -678,14 +697,14 @@ pub fn reset(self: *PageList) void {
|
||||
) catch unreachable;
|
||||
|
||||
// Before resetting our pools we need to free any pages that
|
||||
// are non-standard size since those were allocated outside
|
||||
// the pool.
|
||||
// are heap-owned since those were allocated outside the pool.
|
||||
{
|
||||
const page_alloc = self.pool.pages.arena.child_allocator;
|
||||
var it = self.pages.first;
|
||||
while (it) |node| : (it = node.next) {
|
||||
if (node.data.memory.len > std_size) {
|
||||
page_alloc.free(node.data.memory);
|
||||
switch (node.owned) {
|
||||
.pool => {},
|
||||
.heap => page_alloc.free(node.data.memory),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -825,8 +844,9 @@ pub fn clone(
|
||||
const page_alloc = pool.pages.arena.child_allocator;
|
||||
var page_it = page_list.first;
|
||||
while (page_it) |node| : (page_it = node.next) {
|
||||
if (node.data.memory.len > std_size) {
|
||||
page_alloc.free(node.data.memory);
|
||||
switch (node.owned) {
|
||||
.pool => {},
|
||||
.heap => page_alloc.free(node.data.memory),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -840,7 +860,7 @@ pub fn clone(
|
||||
// we don't know if the source page has a standard size.
|
||||
const node = try createPageExt(
|
||||
&pool,
|
||||
chunk.node.data.capacity,
|
||||
.{ .cap = chunk.node.data.capacity },
|
||||
&page_serial,
|
||||
&page_size,
|
||||
);
|
||||
@@ -1102,7 +1122,7 @@ fn resizeCols(
|
||||
break :err cap;
|
||||
};
|
||||
|
||||
const node = try self.createPage(cap);
|
||||
const node = try self.createPage(.{ .cap = cap });
|
||||
node.data.size.rows = 1;
|
||||
break :node node;
|
||||
};
|
||||
@@ -1964,7 +1984,7 @@ const ReflowCursor = struct {
|
||||
// after reinitializing our cursor on the new page.
|
||||
const new_rows = self.new_rows;
|
||||
|
||||
const node = try list.createPage(cap);
|
||||
const node = try list.createPage(.{ .cap = cap });
|
||||
errdefer comptime unreachable;
|
||||
node.data.size.rows = 1;
|
||||
list.pages.insertAfter(self.node, node);
|
||||
@@ -2343,7 +2363,7 @@ fn resizeWithoutReflowGrowCols(
|
||||
// We need to loop because our col growth may force us
|
||||
// to split pages.
|
||||
while (copied < page.size.rows) {
|
||||
const new_node = try self.createPage(cap);
|
||||
const new_node = try self.createPage(.{ .cap = cap });
|
||||
defer new_node.data.assertIntegrity();
|
||||
|
||||
// The length we can copy into the new page is at most the number
|
||||
@@ -2808,13 +2828,15 @@ pub fn scrollClear(self: *PageList) Allocator.Error!void {
|
||||
|
||||
/// Compact a page to use the minimum required memory for the contents
|
||||
/// it stores. Returns the new node pointer if compaction occurred, or null
|
||||
/// if the page was already compact or compaction would not provide meaningful
|
||||
/// if the page was already compact or compaction would not provide any
|
||||
/// savings.
|
||||
///
|
||||
/// The current design of PageList at the time of writing this doesn't
|
||||
/// allow for smaller than `std_size` nodes so if the current node's backing
|
||||
/// page is standard size or smaller, no compaction will occur. In the
|
||||
/// future we should fix this up.
|
||||
/// The compacted page is always an exact-size heap allocation, never
|
||||
/// a pool item, since a pool item always retains a full std_size
|
||||
/// buffer regardless of the page layout. Note that this means that
|
||||
/// when compacting a pool-owned node, the freed pool item is returned
|
||||
/// to the pool free list, so the memory savings are only fully
|
||||
/// realized once the pool itself is reset or freed.
|
||||
///
|
||||
/// If this returns OOM, the PageList is left unchanged and no dangling
|
||||
/// memory references exist. It is safe to ignore the error and continue using
|
||||
@@ -2826,18 +2848,23 @@ pub fn compact(self: *PageList, node: *List.Node) Allocator.Error!?*List.Node {
|
||||
// We should never have empty rows in our pagelist anyways...
|
||||
assert(page.size.rows > 0);
|
||||
|
||||
// We never compact standard size or smaller pages because changing
|
||||
// the capacity to something smaller won't save memory.
|
||||
if (page.memory.len <= std_size) return null;
|
||||
|
||||
// Compute the minimum capacity required for this page's content
|
||||
const req_cap = page.exactRowCapacity(0, page.size.rows);
|
||||
const new_size = Page.layout(req_cap).total_size;
|
||||
const old_size = page.memory.len;
|
||||
|
||||
// The memory this node currently retains. A pool-owned node always
|
||||
// retains a full pool item no matter its layout size.
|
||||
const old_size: usize = switch (node.owned) {
|
||||
.pool => PagePool.item_size,
|
||||
.heap => page.memory.len,
|
||||
};
|
||||
if (new_size >= old_size) return null;
|
||||
|
||||
// Create the new smaller page
|
||||
const new_node = try self.createPage(req_cap);
|
||||
const new_node = try self.createPage(.{
|
||||
.cap = req_cap,
|
||||
.exact_size = true,
|
||||
});
|
||||
errdefer self.destroyNode(new_node);
|
||||
const new_page: *Page = &new_node.data;
|
||||
new_page.size = page.size;
|
||||
@@ -2916,7 +2943,7 @@ pub fn split(
|
||||
defer self.assertIntegrity();
|
||||
|
||||
// Create a new node with the same capacity of managed memory.
|
||||
const target = try self.createPage(page.capacity);
|
||||
const target = try self.createPage(.{ .cap = page.capacity });
|
||||
errdefer self.destroyNode(target);
|
||||
|
||||
// Determine how many rows we're copying
|
||||
@@ -3214,10 +3241,17 @@ pub fn grow(self: *PageList) Allocator.Error!?*List.Node {
|
||||
}
|
||||
self.viewport_pin.garbage = false;
|
||||
|
||||
// Non-standard pages can't be reused, just destroy them.
|
||||
if (first.data.memory.len > std_size) {
|
||||
self.destroyNode(first);
|
||||
break :prune;
|
||||
switch (first.owned) {
|
||||
// Pool-owned pages are reused below.
|
||||
.pool => {},
|
||||
|
||||
// Heap-owned pages can't be reused because they may be
|
||||
// any size (larger or smaller than a standard page), so
|
||||
// just destroy them.
|
||||
.heap => {
|
||||
self.destroyNode(first);
|
||||
break :prune;
|
||||
},
|
||||
}
|
||||
|
||||
// Reset our memory
|
||||
@@ -3247,7 +3281,7 @@ pub fn grow(self: *PageList) Allocator.Error!?*List.Node {
|
||||
}
|
||||
|
||||
// We need to allocate a new memory buffer.
|
||||
const next_node = try self.createPage(cap);
|
||||
const next_node = try self.createPage(.{ .cap = cap });
|
||||
// we don't errdefer this because we've added it to the linked
|
||||
// list and its fine to have dangling unused pages.
|
||||
self.pages.append(next_node);
|
||||
@@ -3343,7 +3377,7 @@ pub fn increaseCapacity(
|
||||
log.info("adjusting page capacity={}", .{cap});
|
||||
|
||||
// Create our new page and clone the old page into it.
|
||||
const new_node = try self.createPage(cap);
|
||||
const new_node = try self.createPage(.{ .cap = cap });
|
||||
errdefer self.destroyNode(new_node);
|
||||
const new_page: *Page = &new_node.data;
|
||||
assert(new_page.capacity.rows >= page.capacity.rows);
|
||||
@@ -3386,16 +3420,29 @@ pub fn increaseCapacity(
|
||||
return new_node;
|
||||
}
|
||||
|
||||
/// Options for createPage and createPageExt.
|
||||
const CreatePage = struct {
|
||||
/// The capacity to allocate the page with.
|
||||
cap: Capacity,
|
||||
|
||||
/// Force the page backing memory to be an exact-size heap
|
||||
/// allocation even if it would fit within a standard-size pool
|
||||
/// item. This is used when compacting pages to their minimum
|
||||
/// size, since a pool item always retains a full std_size buffer
|
||||
/// regardless of the page layout.
|
||||
exact_size: bool = false,
|
||||
};
|
||||
|
||||
/// Create a new page node. This does not add it to the list and this
|
||||
/// does not do any memory size accounting with max_size/page_size.
|
||||
inline fn createPage(
|
||||
self: *PageList,
|
||||
cap: Capacity,
|
||||
opts: CreatePage,
|
||||
) Allocator.Error!*List.Node {
|
||||
// log.debug("create page cap={}", .{cap});
|
||||
// log.debug("create page cap={}", .{opts.cap});
|
||||
return try createPageExt(
|
||||
&self.pool,
|
||||
cap,
|
||||
opts,
|
||||
&self.page_serial,
|
||||
&self.page_size,
|
||||
);
|
||||
@@ -3403,15 +3450,15 @@ inline fn createPage(
|
||||
|
||||
inline fn createPageExt(
|
||||
pool: *MemoryPool,
|
||||
cap: Capacity,
|
||||
opts: CreatePage,
|
||||
serial: *u64,
|
||||
total_size: ?*usize,
|
||||
) Allocator.Error!*List.Node {
|
||||
var page = try pool.nodes.create();
|
||||
errdefer pool.nodes.destroy(page);
|
||||
|
||||
const layout = Page.layout(cap);
|
||||
const pooled = layout.total_size <= std_size;
|
||||
const layout = Page.layout(opts.cap);
|
||||
const pooled = !opts.exact_size and layout.total_size <= std_size;
|
||||
const page_alloc = pool.pages.arena.child_allocator;
|
||||
|
||||
// It would be better to encode this into the Zig error handling
|
||||
@@ -3443,6 +3490,7 @@ inline fn createPageExt(
|
||||
page.* = .{
|
||||
.data = .initBuf(.init(page_buf), layout),
|
||||
.serial = serial.*,
|
||||
.owned = if (pooled) .pool else .heap,
|
||||
};
|
||||
page.data.size.rows = 0;
|
||||
serial.* += 1;
|
||||
@@ -3475,16 +3523,28 @@ fn destroyNodeExt(
|
||||
) void {
|
||||
const page: *Page = &node.data;
|
||||
|
||||
// Update our accounting for page size
|
||||
if (total_size) |v| v.* -= page.memory.len;
|
||||
// Update our accounting for page size. This must mirror what was
|
||||
// added at creation time: a pool-owned page always accounts for a
|
||||
// full pool item even if its layout is smaller, while a heap-owned
|
||||
// page accounts for its exact memory length.
|
||||
if (total_size) |v| v.* -= switch (node.owned) {
|
||||
.pool => PagePool.item_size,
|
||||
.heap => page.memory.len,
|
||||
};
|
||||
|
||||
if (page.memory.len <= std_size) {
|
||||
// Reset the memory to zero so it can be reused
|
||||
@memset(page.memory, 0);
|
||||
pool.pages.destroy(@ptrCast(page.memory.ptr));
|
||||
} else {
|
||||
const page_alloc = pool.pages.arena.child_allocator;
|
||||
page_alloc.free(page.memory);
|
||||
switch (node.owned) {
|
||||
.pool => {
|
||||
assert(page.memory.len <= std_size);
|
||||
|
||||
// Reset the memory to zero so it can be reused
|
||||
@memset(page.memory, 0);
|
||||
pool.pages.destroy(@ptrCast(page.memory.ptr));
|
||||
},
|
||||
|
||||
.heap => {
|
||||
const page_alloc = pool.pages.arena.child_allocator;
|
||||
page_alloc.free(page.memory);
|
||||
},
|
||||
}
|
||||
|
||||
pool.nodes.destroy(node);
|
||||
@@ -14071,23 +14131,108 @@ test "PageList resize (no reflow) more cols remaps pins in backfill path" {
|
||||
try testing.expectEqual(marker, cell.content.codepoint);
|
||||
}
|
||||
|
||||
test "PageList compact std_size page returns null" {
|
||||
test "PageList compact pool page produces exact-size heap page" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try init(alloc, 80, 24, 0);
|
||||
defer s.deinit();
|
||||
|
||||
// A freshly created page should be at std_size
|
||||
// A freshly created page is pool-owned at std_size.
|
||||
const node = s.pages.first.?;
|
||||
try testing.expectEqual(.pool, node.owned);
|
||||
try testing.expect(node.data.memory.len <= std_size);
|
||||
const original_size = node.data.size;
|
||||
|
||||
// compact should return null since there's nothing to compact
|
||||
const result = try s.compact(node);
|
||||
try testing.expectEqual(null, result);
|
||||
// Compacting it should produce a much smaller exact-size heap page.
|
||||
const new_node = (try s.compact(node)).?;
|
||||
try testing.expectEqual(.heap, new_node.owned);
|
||||
try testing.expect(new_node.data.memory.len < std_size);
|
||||
try testing.expectEqual(original_size.rows, new_node.data.size.rows);
|
||||
try testing.expectEqual(original_size.cols, new_node.data.size.cols);
|
||||
try testing.expectEqual(new_node, s.pages.first.?);
|
||||
|
||||
// Page should still be the same
|
||||
try testing.expectEqual(node, s.pages.first.?);
|
||||
// Our page size accounting should exactly match the compacted
|
||||
// page since it is the only page in the list.
|
||||
try testing.expectEqual(new_node.data.memory.len, s.page_size);
|
||||
|
||||
// Compacting again should be a no-op since it is already exact.
|
||||
try testing.expectEqual(null, try s.compact(new_node));
|
||||
}
|
||||
|
||||
test "PageList compact then grow allocates new page" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try init(alloc, 80, 24, null);
|
||||
defer s.deinit();
|
||||
|
||||
// Compact the only page. It now has no spare row capacity.
|
||||
const node = (try s.compact(s.pages.first.?)).?;
|
||||
try testing.expectEqual(node.data.size.rows, node.data.capacity.rows);
|
||||
|
||||
// Growing must allocate a fresh standard page from the pool,
|
||||
// exercising that a compacted page remains a valid live page.
|
||||
_ = try s.grow();
|
||||
try testing.expect(s.pages.first != s.pages.last);
|
||||
try testing.expectEqual(.pool, s.pages.last.?.owned);
|
||||
try testing.expectEqual(@as(usize, 25), s.totalRows());
|
||||
}
|
||||
|
||||
test "PageList compact then reset frees heap pages" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try init(alloc, 80, 24, 0);
|
||||
defer s.deinit();
|
||||
|
||||
// Compact the only page so the list contains a sub-std_size
|
||||
// heap-owned page.
|
||||
const node = (try s.compact(s.pages.first.?)).?;
|
||||
try testing.expectEqual(.heap, node.owned);
|
||||
try testing.expect(node.data.memory.len < std_size);
|
||||
|
||||
// Reset must free the heap page (testing allocator catches leaks
|
||||
// and invalid frees) and rebuild from the pool.
|
||||
s.reset();
|
||||
try testing.expectEqual(.pool, s.pages.first.?.owned);
|
||||
try testing.expectEqual(@as(usize, s.rows), s.totalRows());
|
||||
}
|
||||
|
||||
test "PageList compact then clone" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
var s = try init(alloc, 80, 24, null);
|
||||
defer s.deinit();
|
||||
|
||||
// Write a marker so we can verify contents survive.
|
||||
{
|
||||
const node = s.pages.first.?;
|
||||
const rac = node.data.getRowAndCell(1, 2);
|
||||
rac.cell.* = .{
|
||||
.content_tag = .codepoint,
|
||||
.content = .{ .codepoint = 'X' },
|
||||
};
|
||||
}
|
||||
|
||||
// Compact so the source list contains a sub-std_size heap page.
|
||||
const node = (try s.compact(s.pages.first.?)).?;
|
||||
try testing.expectEqual(.heap, node.owned);
|
||||
try testing.expect(node.data.memory.len < std_size);
|
||||
|
||||
var s2 = try s.clone(alloc, .{
|
||||
.top = .{ .screen = .{} },
|
||||
});
|
||||
defer s2.deinit();
|
||||
try testing.expectEqual(@as(usize, s.rows), s2.totalRows());
|
||||
|
||||
// Verify the marker survived the clone.
|
||||
{
|
||||
const node2 = s2.pages.first.?;
|
||||
const rac = node2.data.getRowAndCell(1, 2);
|
||||
try testing.expectEqual(@as(u21, 'X'), rac.cell.content.codepoint);
|
||||
}
|
||||
}
|
||||
|
||||
test "PageList compact oversized page" {
|
||||
@@ -14177,7 +14322,7 @@ test "PageList compact oversized page" {
|
||||
}
|
||||
}
|
||||
|
||||
test "PageList compact insufficient savings returns null" {
|
||||
test "PageList compact after increaseCapacity" {
|
||||
const testing = std.testing;
|
||||
const alloc = testing.allocator;
|
||||
|
||||
@@ -14186,23 +14331,15 @@ test "PageList compact insufficient savings returns null" {
|
||||
|
||||
var node = s.pages.first.?;
|
||||
|
||||
// Make the page slightly oversized (just one increase)
|
||||
// This might not provide enough savings to justify compaction
|
||||
// Grow the page capacity. The content is unchanged, so compaction
|
||||
// should always shrink it back down to an exact-size heap page.
|
||||
node = try s.increaseCapacity(node, .grapheme_bytes);
|
||||
const grown_len = node.data.memory.len;
|
||||
|
||||
// If the page is still at or below std_size, compact returns null
|
||||
if (node.data.memory.len <= std_size) {
|
||||
const result = try s.compact(node);
|
||||
try testing.expectEqual(null, result);
|
||||
} else {
|
||||
// If it did grow beyond std_size, verify that compaction
|
||||
// works or returns null based on savings calculation
|
||||
const result = try s.compact(node);
|
||||
// Either it compacted or determined insufficient savings
|
||||
if (result) |new_node| {
|
||||
try testing.expect(new_node.data.memory.len < node.data.memory.len);
|
||||
}
|
||||
}
|
||||
const new_node = (try s.compact(node)).?;
|
||||
try testing.expectEqual(.heap, new_node.owned);
|
||||
try testing.expect(new_node.data.memory.len < grown_len);
|
||||
try testing.expect(new_node.data.memory.len < std_size);
|
||||
}
|
||||
|
||||
test "PageList split at middle row" {
|
||||
|
||||
Reference in New Issue
Block a user