diff --git a/src/datastruct/main.zig b/src/datastruct/main.zig index 32c5ff609..9fb0bae9e 100644 --- a/src/datastruct/main.zig +++ b/src/datastruct/main.zig @@ -14,6 +14,7 @@ pub const IntrusiveDoublyLinkedList = intrusive_linked_list.DoublyLinkedList; pub const LimitedAllocator = @import("limited_allocator.zig").LimitedAllocator; pub const MessageData = @import("message_data.zig").MessageData; pub const SplitTree = split_tree.SplitTree; +pub const UntouchedPool = @import("untouched_pool.zig").UntouchedPool; pub const WasmPagePool = @import("wasm_page_pool.zig").WasmPagePool; test { diff --git a/src/datastruct/untouched_pool.zig b/src/datastruct/untouched_pool.zig new file mode 100644 index 000000000..a8ed4d314 --- /dev/null +++ b/src/datastruct/untouched_pool.zig @@ -0,0 +1,297 @@ +const std = @import("std"); +const assert = std.debug.assert; +const Allocator = std.mem.Allocator; +const Alignment = std.mem.Alignment; + +/// A fixed-size item pool whose bookkeeping never touches item memory. +/// +/// std.heap.MemoryPool keeps its free list inside the items intrusively, +/// meaning it has to touch allocated item memory. For memory that is +/// demand-paged, that write forces the OS to create a physical page +/// for nothing (to mark it free!). +/// +/// This pool keeps the free list in a separate array allocated from a +/// general-purpose allocator and allocates every item individually from +/// the item allocator with the requested alignment. The pool never reads +/// or writes an item until it is needed. +/// +/// Contracts: +/// +/// - The pool never zeroes. Callers that need zeroed items must use an +/// item allocator that returns zeroed memory and must zero (or +/// decommit) an item before destroy(). +/// - destroy() never allocates and never fails: create() reserves a +/// free-list slot for every live item before allocating a new one. +/// - The pool tracks free items only. Every item must be returned with +/// destroy() or release() before reset() or deinit(), or it leaks. +/// This is asserted in safe builds. +/// +/// The tradeoff is that this isn't as fast as std.heap.MemoryPool, its +/// not as cache friendly. But benchmarks show that the cost is minimal +/// and if the tradeoff of not touching the memory is important, then +/// this pays off. +pub fn UntouchedPool(comptime Item: type, comptime alignment: Alignment) type { + return struct { + const Self = @This(); + + pub const item_size = @sizeOf(Item); + pub const item_alignment: Alignment = alignment.max(.of(Item)); + pub const ItemPtr = *align(item_alignment.toByteUnits()) Item; + pub const ResetMode = std.heap.ArenaAllocator.ResetMode; + + /// The allocator items are allocated from. + allocator: Allocator, + + /// The general-purpose allocator for the free list. + gpa: Allocator, + + /// Free items. The most recently destroyed item is handed out first. + free: std.ArrayList(ItemPtr), + + /// Number of items currently allocated from the item allocator, + /// free or in use. `free.capacity >= live` always holds so that + /// destroy() never has to grow the free list. + live: usize, + + /// Create a pool with `preheat` items already allocated. + pub fn initCapacity( + gpa: Allocator, + item_alloc: Allocator, + preheat: usize, + ) Allocator.Error!Self { + var self: Self = .{ + .allocator = item_alloc, + .gpa = gpa, + .free = .empty, + .live = 0, + }; + errdefer self.deinit(); + + try self.free.ensureTotalCapacityPrecise(gpa, preheat); + for (0..preheat) |_| { + const item = try self.allocItem(); + self.free.appendAssumeCapacity(item); + } + + return self; + } + + /// Free all items and the free list. Every item must have been + /// returned with destroy() or release(). + pub fn deinit(self: *Self) void { + assert(self.live == self.free.items.len); + for (self.free.items) |item| self.allocator.free(bytes(item)); + self.free.deinit(self.gpa); + self.* = undefined; + } + + /// Free items so that the retained free items fit `mode`. Every + /// item must have been returned with destroy() or release(). + /// + /// This mirrors std.heap.MemoryPool.reset; it always succeeds + /// (returns true) because nothing is reallocated. + pub fn reset(self: *Self, mode: ResetMode) bool { + assert(self.live == self.free.items.len); + const retain_bytes: usize = switch (mode) { + .free_all => 0, + .retain_capacity => return true, + .retain_with_limit => |limit| limit, + }; + + const retain_items = retain_bytes / item_size; + while (self.free.items.len > retain_items) { + const item = self.free.pop().?; + self.allocator.free(bytes(item)); + self.live -= 1; + } + + return true; + } + + /// Get an item. This pops a free item without touching it or, + /// when none is free, allocates a new one from the item + /// allocator. + pub fn create(self: *Self) Allocator.Error!ItemPtr { + if (self.free.pop()) |item| return item; + + // Reserve the free-list slot for the new item before + // allocating it so that destroy() can never fail. + try self.free.ensureTotalCapacity(self.gpa, self.live + 1); + return try self.allocItem(); + } + + /// Return an item to the free list for reuse. The item is not + /// modified or zeroed so it is up to the caller. + pub fn destroy(self: *Self, item: ItemPtr) void { + assert(self.free.items.len < self.live); + self.free.appendAssumeCapacity(item); + } + + /// Return an item straight to the item allocator instead of the + /// free list. This is for teardown paths that would otherwise + /// have to zero an item only for it to be freed moments later. + pub fn release(self: *Self, item: ItemPtr) void { + assert(self.free.items.len < self.live); + self.allocator.free(bytes(item)); + self.live -= 1; + } + + fn allocItem(self: *Self) Allocator.Error!ItemPtr { + assert(self.free.capacity > self.live); + const memory = try self.allocator.alignedAlloc( + u8, + item_alignment, + item_size, + ); + self.live += 1; + return @ptrCast(memory.ptr); + } + + fn bytes(item: ItemPtr) []align(item_alignment.toByteUnits()) u8 { + return std.mem.asBytes(item); + } + }; +} + +/// Test item: one minimum OS page, page-aligned, like a terminal page. +const TestPool = UntouchedPool( + [std.heap.page_size_min]u8, + .fromByteUnits(std.heap.page_size_min), +); + +test "UntouchedPool: create, destroy, reuse" { + const testing = std.testing; + var pool: TestPool = try .initCapacity(testing.allocator, testing.allocator, 0); + defer pool.deinit(); + + const a = try pool.create(); + const b = try pool.create(); + const c = try pool.create(); + try testing.expect(a != b); + try testing.expect(a != c); + try testing.expect(b != c); + + // Freed items are recycled, most recent first. + pool.destroy(a); + pool.destroy(b); + try testing.expectEqual(b, try pool.create()); + try testing.expectEqual(a, try pool.create()); + + pool.destroy(a); + pool.destroy(b); + pool.destroy(c); +} + +test "UntouchedPool: create and destroy never touch items" { + const testing = std.testing; + const preheat = 4; + + // Back the item allocator with memory we can inspect. + const backing = try testing.allocator.alignedAlloc( + u8, + .fromByteUnits(std.heap.page_size_min), + preheat * TestPool.item_size, + ); + defer testing.allocator.free(backing); + var fba: std.heap.FixedBufferAllocator = .init(backing); + + var pool: TestPool = try .initCapacity( + testing.allocator, + fba.allocator(), + preheat, + ); + defer pool.deinit(); + try testing.expectEqual(preheat * TestPool.item_size, fba.end_index); + + // Lay the sentinel down after preheat: allocation itself may write + // (the Allocator interface fills fresh memory with undefined in + // safe builds, which valgrind also tracks). The sentinel must differ + // from Zig's 0xAA undefined pattern so that any write is visible. + const sentinel: u8 = 0x5A; + @memset(backing, sentinel); + + // Every preheated item comes out untouched and without allocating. + var items: [preheat]TestPool.ItemPtr = undefined; + for (&items) |*item| { + item.* = try pool.create(); + try testing.expectEqual(preheat * TestPool.item_size, fba.end_index); + try testing.expect(std.mem.allEqual(u8, item.*, sentinel)); + } + + // Destroying and re-creating doesn't touch them either. + for (items) |item| pool.destroy(item); + try testing.expect(std.mem.allEqual(u8, backing, sentinel)); + for (&items) |*item| { + item.* = try pool.create(); + try testing.expect(std.mem.allEqual(u8, item.*, sentinel)); + } + for (items) |item| pool.destroy(item); +} + +test "UntouchedPool: destroy never allocates from the general allocator" { + const testing = std.testing; + + // A general allocator that fails every allocation: after create has + // reserved the free-list slot, destroy must not need it. + var failing: std.testing.FailingAllocator = .init(testing.allocator, .{ + .fail_index = 0, + }); + + var pool: TestPool = try .initCapacity(testing.allocator, testing.allocator, 0); + defer pool.deinit(); + + // The pool grows the free list through its own gpa, so swap in the + // failing one only around destroy. + var items: [64]TestPool.ItemPtr = undefined; + for (&items) |*item| item.* = try pool.create(); + const gpa = pool.gpa; + pool.gpa = failing.allocator(); + for (items) |item| pool.destroy(item); + pool.gpa = gpa; + try testing.expectEqual(items.len, pool.free.items.len); +} + +test "UntouchedPool: reset retains at most the limit" { + const testing = std.testing; + var pool: TestPool = try .initCapacity(testing.allocator, testing.allocator, 4); + defer pool.deinit(); + + // Free everything above the limit + try testing.expect(pool.reset(.{ .retain_with_limit = 2 * TestPool.item_size })); + try testing.expectEqual(2, pool.free.items.len); + try testing.expectEqual(2, pool.live); + + // retain_capacity keeps everything + try testing.expect(pool.reset(.retain_capacity)); + try testing.expectEqual(2, pool.free.items.len); + + // Retained items are handed out without allocating; new items + // beyond them are allocated as needed. + const a = try pool.create(); + const b = try pool.create(); + try testing.expectEqual(2, pool.live); + const c = try pool.create(); + try testing.expectEqual(3, pool.live); + pool.destroy(a); + pool.destroy(b); + pool.destroy(c); + + try testing.expect(pool.reset(.free_all)); + try testing.expectEqual(0, pool.free.items.len); + try testing.expectEqual(0, pool.live); +} + +test "UntouchedPool: release frees immediately" { + const testing = std.testing; + var pool: TestPool = try .initCapacity(testing.allocator, testing.allocator, 1); + defer pool.deinit(); + + const a = try pool.create(); + const b = try pool.create(); + try testing.expectEqual(2, pool.live); + pool.release(a); + try testing.expectEqual(1, pool.live); + try testing.expectEqual(0, pool.free.items.len); + pool.destroy(b); + try testing.expectEqual(1, pool.free.items.len); +} diff --git a/src/datastruct/wasm_page_pool.zig b/src/datastruct/wasm_page_pool.zig index 7f1193825..0f73eec64 100644 --- a/src/datastruct/wasm_page_pool.zig +++ b/src/datastruct/wasm_page_pool.zig @@ -42,9 +42,9 @@ const Allocator = std.mem.Allocator; /// memory.grow are zero by the wasm spec; recycled items retain /// whatever the caller left, so zero either on destroy or on create). /// -/// The API mirrors the subset of std.heap.memory_pool.AlignedManaged -/// that callers use (initCapacity/deinit/reset/create/destroy plus the -/// managed allocator field), so it can be swapped in comptime. +/// The API mirrors the native page pool (datastruct.UntouchedPool: +/// initCapacity/deinit/reset/create/destroy plus the allocator field), +/// so PageList can swap them at comptime. pub fn WasmPagePool(comptime Item: type) type { return struct { const Self = @This(); @@ -78,9 +78,14 @@ pub fn WasmPagePool(comptime Item: type) type { var free_list: std.SinglyLinkedList = .{}; pub fn initCapacity( + gpa: Allocator, allocator: Allocator, preheat: usize, ) Allocator.Error!Self { + // The free list is intrusive so we never allocate from the + // general purpose allocator. + _ = gpa; + // Preheating is intentionally a no-op: item creation is a // cheap memory.grow or free-list pop. And we want to avoid // the preallocation mem cost. @@ -135,7 +140,7 @@ test WasmPagePool { const testing = std.testing; const Pool = WasmPagePool([64 * 1024]u8); - var pool: Pool = try .initCapacity(testing.allocator, 0); + var pool: Pool = try .initCapacity(testing.allocator, testing.allocator, 0); defer pool.deinit(); const a = try pool.create(); @@ -147,7 +152,7 @@ test WasmPagePool { try testing.expectEqual(b, try pool.create()); // The free list is shared across pools of the same Item type. - var pool2: Pool = try .initCapacity(testing.allocator, 0); + var pool2: Pool = try .initCapacity(testing.allocator, testing.allocator, 0); defer pool2.deinit(); pool.destroy(a); try testing.expectEqual(a, try pool2.create()); diff --git a/src/terminal/PageList.zig b/src/terminal/PageList.zig index 1e818efcf..6c4d892c1 100644 --- a/src/terminal/PageList.zig +++ b/src/terminal/PageList.zig @@ -12,7 +12,7 @@ const fastmem = @import("../fastmem.zig"); const simd = @import("../simd/main.zig"); const tripwire = @import("../tripwire.zig"); const DoublyLinkedList = @import("../datastruct/main.zig").IntrusiveDoublyLinkedList; -const WasmPagePool = @import("../datastruct/main.zig").WasmPagePool; +const datastruct = @import("../datastruct/main.zig"); const color = @import("color.zig"); const compression = @import("compress.zig"); const highlight = @import("highlight.zig"); @@ -313,7 +313,7 @@ const std_size = Page.layout(std_capacity).total_size; /// through a free list shared by the whole module instance instead of /// dying with the pool. /// -/// Test builds use the std pool even on wasm so that pool memory goes +/// Test builds use the native pool even on wasm so that pool memory goes /// through the testing allocator and participates in leak detection. const wasm_page_pool = builtin.target.cpu.arch.isWasm() and !builtin.is_test; @@ -321,12 +321,20 @@ const wasm_page_pool = builtin.target.cpu.arch.isWasm() and !builtin.is_test; /// so we can allocate these with a page allocator. We have to use a page /// allocator because we need memory that is zero-initialized and page-aligned. const PagePool = if (wasm_page_pool) - WasmPagePool([std_size]u8) -else - std.heap.memory_pool.AlignedManaged( + datastruct.WasmPagePool([std_size]u8) +else untouched: { + // Untouched pools never read/write to the items so that we can + // use demand-paging on operating systems that support it. This makes + // it so that an idle item in the pool costs no physical memory, + // only virtual memory. + // + // Contract for PageList is that every path that returns an item + // to the pool must zero it. + break :untouched datastruct.UntouchedPool( [std_size]u8, .fromByteUnits(std.heap.page_size_min), ); +}; /// List of pins, known as "tracked" pins. These are pins that are kept /// up to date automatically through page-modifying operations. @@ -350,7 +358,7 @@ pub const MemoryPool = struct { ) Allocator.Error!MemoryPool { var node_pool = try NodePool.initCapacity(gen_alloc, preheat); errdefer node_pool.deinit(); - var page_pool = try PagePool.initCapacity(page_alloc, preheat); + var page_pool = try PagePool.initCapacity(gen_alloc, page_alloc, preheat); errdefer page_pool.deinit(); var pin_pool = try PinPool.initCapacity(gen_alloc, 8); errdefer pin_pool.deinit(); @@ -636,6 +644,7 @@ pub fn init( cols, rows, ); + errdefer releasePages(&pool, page_list); var limits: Limits = .init(cols, rows); limits.set(.bytes, opts.max_size); @@ -698,17 +707,8 @@ 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 pages: heap-owned - // pages are freed directly and pool-owned pages are reclaimed. - errdefer { - var it = page_list.first; - while (it) |node| : (it = node.next) { - switch (node.owned) { - .pool => reclaimPoolPage(pool, node.page()), - .heap => page_alloc.free(node.page().memory), - } - } - } + // If we have an error, we need to release the pages we created. + errdefer releasePages(pool, page_list); var rem = rows; while (rem > 0) { @@ -897,18 +897,42 @@ fn verifyIntegrity(self: *const PageList) IntegrityError!void { } } -/// Return a pool-owned page buffer to the page pool during a teardown -/// walk, zeroed for reuse (mirroring destroyNodeExt). Teardown walks -/// call this unconditionally for every pool-owned page. -fn reclaimPoolPage(pool: *MemoryPool, page: *const Page) void { - // Only wasm requires this - if (comptime !wasm_page_pool) return; +/// Release every page in the list during a teardown walk (deinit, +/// reset, or an errdefer unwinding a partially built list): heap-owned +/// pages go back to the page allocator and pool-owned pages back to the +/// page pool. The nodes themselves are not touched; they live in the +/// node pool. +fn releasePages(pool: *MemoryPool, list: List) void { + const page_alloc = pool.pages.allocator; + var it = list.first; + while (it) |node| : (it = node.next) { + const page = node.restore(.discard); + switch (node.owned) { + .pool => releasePoolPage(pool, page), + .heap => page_alloc.free(page.memory), + } + } +} +/// Release a pool-owned page during a teardown walk. Unlike +/// destroyNodeExt, this does not zero the page: on native, the item goes +/// straight back to the page allocator, and zeroing it first would write +/// the whole page (and fault a decommitted mapping back in) only for it +/// to be unmapped. +fn releasePoolPage(pool: *MemoryPool, page: *const Page) void { const item: *align(std.heap.page_size_min) [std_size]u8 = @ptrCast(@alignCast(page.memory.ptr)); - // We have to zero the item - _ = terminal_mem.decommit(.zero, item, page.memory.len); - pool.pages.destroy(item); + + // The wasm pool's items are shared by every pool in the module and + // never return to the allocator, so they go back to the free list + // zeroed for reuse (mirroring destroyNodeExt). + if (comptime wasm_page_pool) { + _ = terminal_mem.decommit(.zero, item, page.memory.len); + pool.pages.destroy(item); + return; + } + + pool.pages.release(item); } /// Deinit the pagelist, freeing all page memory and the memory pool. @@ -919,20 +943,9 @@ pub fn deinit(self: *PageList) void { // Always deallocate our hashmap. self.tracked_pins.deinit(self.pool.alloc); - // Go through our linked list and release every page: heap-owned - // pages are freed directly and pool-owned pages are reclaimed. - const page_alloc = self.pool.pages.allocator; - var it = self.pages.first; - while (it) |node| : (it = node.next) { - const page = node.restore(.discard); - switch (node.owned) { - .pool => reclaimPoolPage(&self.pool, page), - .heap => page_alloc.free(page.memory), - } - } - - // Deallocate all the pages. We don't need to deallocate the list or + // Release every page. We don't need to deallocate the list or // nodes because they all reside in the pool. + releasePages(&self.pool, self.pages); self.pool.deinit(); } @@ -968,25 +981,18 @@ pub fn reset(self: *PageList) void { cap.rows, ) catch unreachable; - // Before resetting our pools we need to release our pages: - // heap-owned pages are freed since they were allocated outside - // the pool, and pool-owned pages are reclaimed. - { - const page_alloc = self.pool.pages.allocator; - var it = self.pages.first; - while (it) |node| : (it = node.next) { - const page = node.restore(.discard); - switch (node.owned) { - .pool => reclaimPoolPage(&self.pool, page), - .heap => page_alloc.free(page.memory), - } - } - } + // Before resetting our pools we need to release our pages: heap-owned + // pages go back to the page allocator and pool-owned pages back to + // the page pool. + releasePages(&self.pool, self.pages); // Reset our pools to free as much memory as possible while retaining // the capacity for at least the minimum number of pages we need. // The return value is whether memory was reclaimed or not, but in // either case the pool is left in a valid state. + // + // Retained page pool items are zero (see PagePool), so there is + // nothing to scrub before initPages reuses them. _ = self.pool.pages.reset(.{ .retain_with_limit = page_count * PagePool.item_size, }); @@ -994,58 +1000,6 @@ pub fn reset(self: *PageList) void { .retain_with_limit = page_count * NodePool.item_size, }); - // Our page pool relies on mmap to zero our page memory. Since we're - // retaining a certain amount of memory, it won't use mmap and won't - // be zeroed. This block zeroes out all the memory in the pool arena. - // - // The wasm page pool has no arena to scrub: its free-list items were - // zeroed by reclaimPoolPage above. - // - // Note: we only have to do this for the page pool because the nodes are - // always fully overwritten on each allocation. - if (comptime !wasm_page_pool) { - inline for (.{ - self.pool.pages.unmanaged.arena_state.used_list, - self.pool.pages.unmanaged.arena_state.free_list, - }) |first| { - var node_ = first; - while (node_) |node| : (node_ = node.next) { - // NOTE: Zig 0.16.0's arenas don't use the linked list types - // anymore, so we can just reference fields directly. The node - // type is still private though, so we have to parse out some - // of the internal methods to work with the buffer - namely - // Node.loadBuf and Node.Size.toInt. They are combined below. - // - // PS: My (vancluever's) reading of the code gives me the - // impression that we no longer need to offset the data by the - // header, because there's no linked list overhead anymore. But - // I'm sure we'll see pretty quick when I run the tests. :) - // - const BufNode = struct { - size: Size, - end_index: usize, - next: ?*@This(), - - const Size = packed struct(usize) { - resizing: bool, - _: @Int(.unsigned, @bitSizeOf(usize) - 1) = 0, - - fn toInt(s: Size) usize { - var int = s; - int.resizing = false; - return @bitCast(int); - } - }; - }; - - const buf_node_ptr: *BufNode = @ptrCast(node); - const buf_node_size = @atomicLoad(BufNode.Size, &buf_node_ptr.size, .monotonic); - const buf = @as([*]u8, @ptrCast(node))[0..buf_node_size.toInt()][@sizeOf(BufNode)..]; - @memset(buf, 0); - } - } - } - // Initialize our pages. This should not be able to fail since // we retained the capacity for the minimum number of pages we need. self.pages, self.page_size = initPages( @@ -1140,16 +1094,7 @@ pub fn clone( // Our list of pages var page_list: List = .{}; - errdefer { - const page_alloc = pool.pages.allocator; - var page_it = page_list.first; - while (page_it) |node| : (page_it = node.next) { - switch (node.owned) { - .pool => reclaimPoolPage(&pool, node.page()), - .heap => page_alloc.free(node.page().memory), - } - } - } + errdefer releasePages(&pool, page_list); // Copy our pages var page_serial: u64 = 0; @@ -1164,6 +1109,11 @@ pub fn clone( &page_serial, &page_size, ); + + // Add the page to the list immediately so that the errdefer + // above releases it if cloning fails. + page_list.append(node); + const dst_page = node.page(); const src_page = chunk.node.page(); assert(node.capacity().rows >= chunk.end - chunk.start); @@ -1178,8 +1128,6 @@ pub fn clone( dst_page.dirty = src_page.dirty; - page_list.append(node); - total_rows += node.rows(); // Remap our tracked pins by changing the page and @@ -4586,12 +4534,9 @@ inline fn createPageExt( // (WASM), the WasmAllocator reuses freed slots without zeroing. // // Otherwise, we rely on pool item buffers being zeroed: fresh items - // come from the OS page allocator (zeroed pages) and destroyNodeExt - // zeroes buffers before returning them to the pool. The one - // exception is the first pointer-size bytes, which hold the pool's - // free list node while a buffer is in the free list; initBuf below - // always overwrites those since the page rows start at offset 0 - // (comptime-asserted in Page). + // come from the OS page allocator (zeroed pages), destroyNodeExt + // zeroes buffers before returning them to the pool, and the pool + // never writes into its items (see PagePool). if (comptime std.debug.runtime_safety or builtin.os.tag == .freestanding) @memset(page_buf, 0); @@ -20402,3 +20347,75 @@ test "PageList resize trimmed rows have default state" { try testing.expect(rac.cell.isZero()); } } + +test "PageList memory pool never touches idle page memory" { + const testing = std.testing; + const preheat = page_preheat; + + // Back the page allocator with memory we can inspect. + const backing = try testing.allocator.alignedAlloc( + u8, + .fromByteUnits(std.heap.page_size_min), + preheat * std_size, + ); + defer testing.allocator.free(backing); + var fba: std.heap.FixedBufferAllocator = .init(backing); + + var pool: MemoryPool = try .init(testing.allocator, fba.allocator(), preheat); + defer pool.deinit(); + + // Preheat allocated exactly the items. + try testing.expectEqual(preheat * std_size, fba.end_index); + + // Lay the sentinel down after preheat: allocation itself may write + // (the Allocator interface fills fresh memory with undefined in + // safe builds, which valgrind also tracks). The sentinel must differ + // from Zig's 0xAA undefined pattern so that any write is visible. + const sentinel: u8 = 0x5A; + @memset(backing, sentinel); + + // Every preheated item is handed out untouched and without going + // back to the page allocator, and destroying it doesn't touch it. + var items: [preheat]PagePool.ItemPtr = undefined; + for (&items) |*item| { + item.* = try pool.pages.create(); + try testing.expectEqual(preheat * std_size, fba.end_index); + try testing.expect(std.mem.allEqual(u8, item.*, sentinel)); + } + for (items) |item| pool.pages.destroy(item); + try testing.expect(std.mem.allEqual(u8, backing, sentinel)); +} + +test "PageList memory pool fast path does not allocate" { + const testing = std.testing; + var counting: std.testing.FailingAllocator = .init(testing.allocator, .{}); + + var pool: MemoryPool = try .init( + testing.allocator, + counting.allocator(), + page_preheat, + ); + defer pool.deinit(); + try testing.expectEqual(page_preheat, counting.allocations); + + // Cycle a few thousand pages through the preheated items. As long + // as no more than the preheat are live at once, create is a + // free-list pop and never touches the page allocator. + var items: [page_preheat]PagePool.ItemPtr = undefined; + for (0..1024) |_| { + for (&items) |*item| item.* = try pool.pages.create(); + for (items) |item| pool.pages.destroy(item); + } + try testing.expectEqual(page_preheat, counting.allocations); + try testing.expectEqual(0, counting.deallocations); + + // Going past the preheat allocates the extra items once; they are + // recycled from then on. + var extra: [page_preheat + 2]PagePool.ItemPtr = undefined; + for (0..1024) |_| { + for (&extra) |*item| item.* = try pool.pages.create(); + for (extra) |item| pool.pages.destroy(item); + } + try testing.expectEqual(extra.len, counting.allocations); + try testing.expectEqual(0, counting.deallocations); +} diff --git a/src/terminal/page.zig b/src/terminal/page.zig index de3cdf1cf..94898f83d 100644 --- a/src/terminal/page.zig +++ b/src/terminal/page.zig @@ -151,15 +151,6 @@ pub const Page = struct { @alignOf(Cell), StyleSet.base_align.toByteUnits(), ) == 0); - - // The PageList memory pool requires that initBuf overwrites at - // least the first pointer-size bytes of the backing buffer: - // std.heap.MemoryPool stores its free list node there when a - // page buffer is returned to it, and pool reuse skips zeroing - // in release builds. This holds because the rows array is at - // offset 0 (see layout), a page always has at least one row, - // and initBuf fully rewrites every row. - assert(@sizeOf(Row) >= @sizeOf(usize)); } /// The backing memory for the page. A page is always made up of a @@ -254,10 +245,7 @@ pub const Page = struct { pub inline fn initBuf(buf: OffsetBuf, l: Layout) Page { const cap = l.capacity; - // A page must always have at least one row. Aside from being - // useless otherwise, the row initialization below must always - // overwrite the start of the buffer for pool reuse. See the - // comptime assert at the top of Page. + // A page must always have at least one row. assert(cap.rows > 0); const rows = buf.member(Row, l.rows_start); @@ -1745,10 +1733,6 @@ pub const Page = struct { pub inline fn layout(cap: Capacity) Layout { const rows_count: usize = @intCast(cap.rows); - // The rows array must stay at offset 0: the PageList memory - // pool relies on initBuf overwriting the first bytes of a - // reused page buffer, which hold the pool's free list node. - // See the comptime assert at the top of Page. const rows_start = 0; const rows_end: usize = rows_start + (rows_count * @sizeOf(Row));