From e23ef4b0bedb163f4ceafd276d32c914967c0ea0 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 13 Feb 2026 12:34:27 +0800 Subject: [PATCH] Remove custom-arenas, use ArenaPool instead This removes the browser-specific arenas (session, transfer, page, call) in favor of the arena pool. This is a bit of a win-lose commit. It exists as (the last?) step before I can really start working on frames. Frames will require their own "page" and "call" arenas, so there isn't just 1 per browser now, but rather N, where N is the number of frames + 1 page. This change was already done for Contexts when ExecutionWorld was removed, and the idea is the same: making these units more self contained so to support cases where we break out of the "1" model we currently have (1 browser, 1 session, 1 page, 1 context, ...). But it's a bit of a step backwards because the ArenaPool is dumb and just resets everything to a single hard-coded (for now) value: 16KB. But in my mind, an arena that's used for 1 thing (e.g. the page or call arenas) is more likely to be well-sized for that specific role in the future, even on a different page/navigate. I think ultimately, we'll move to an ArenaPool that has different levels, e.g. acquire() and acquireLarge() which can reset to different sizes, so that a page arena can use acquireLarge() and retain a larger amount of memory between use. --- src/ArenaPool.zig | 5 +++++ src/browser/Browser.zig | 13 ------------- src/browser/Page.zig | 17 ++++++++++++----- src/browser/Session.zig | 22 +++++++++++++--------- src/http/Client.zig | 1 - 5 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/ArenaPool.zig b/src/ArenaPool.zig index f7d7015f..32257e30 100644 --- a/src/ArenaPool.zig +++ b/src/ArenaPool.zig @@ -85,3 +85,8 @@ pub fn release(self: *ArenaPool, allocator: Allocator) void { self.free_list_len = free_list_len + 1; self.free_list = entry; } + +pub fn reset(_: *const ArenaPool, allocator: Allocator, retain: usize) void { + const arena: *std.heap.ArenaAllocator = @ptrCast(@alignCast(allocator.ptr)); + _ = arena.reset(.{ .retain_with_limit = retain }); +} diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index 54668987..b89e23c1 100644 --- a/src/browser/Browser.zig +++ b/src/browser/Browser.zig @@ -44,10 +44,6 @@ session: ?Session, allocator: Allocator, arena_pool: *ArenaPool, http_client: *HttpClient, -call_arena: ArenaAllocator, -page_arena: ArenaAllocator, -session_arena: ArenaAllocator, -transfer_arena: ArenaAllocator, const InitOpts = struct { env: js.Env.InitOpts = .{}, @@ -66,20 +62,12 @@ pub fn init(app: *App, opts: InitOpts) !Browser { .allocator = allocator, .arena_pool = &app.arena_pool, .http_client = app.http.client, - .call_arena = ArenaAllocator.init(allocator), - .page_arena = ArenaAllocator.init(allocator), - .session_arena = ArenaAllocator.init(allocator), - .transfer_arena = ArenaAllocator.init(allocator), }; } pub fn deinit(self: *Browser) void { self.closeSession(); self.env.deinit(); - self.call_arena.deinit(); - self.page_arena.deinit(); - self.session_arena.deinit(); - self.transfer_arena.deinit(); } pub fn newSession(self: *Browser, notification: *Notification) !*Session { @@ -94,7 +82,6 @@ pub fn closeSession(self: *Browser) void { if (self.session) |*session| { session.deinit(); self.session = null; - _ = self.session_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); self.env.memoryPressureNotification(.critical); } } diff --git a/src/browser/Page.zig b/src/browser/Page.zig index b5f41df9..d35a8471 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -225,8 +225,12 @@ pub fn init(self: *Page, session: *Session) !void { log.debug(.page, "page.init", .{}); } const browser = session.browser; - const page_arena = browser.page_arena.allocator(); - errdefer _ = browser.page_arena.reset(.free_all); + const arena_pool = browser.arena_pool; + const page_arena = try arena_pool.acquire(); + errdefer arena_pool.release(page_arena); + + const call_arena = try arena_pool.acquire(); + errdefer arena_pool.release(call_arena); var factory = Factory.init(page_arena, self); @@ -239,8 +243,8 @@ pub fn init(self: *Page, session: *Session) !void { .arena = page_arena, .document = document, .window = undefined, - .arena_pool = browser.arena_pool, - .call_arena = browser.call_arena.allocator(), + .arena_pool = arena_pool, + .call_arena = call_arena, ._session = session, ._factory = factory, ._script_manager = undefined, @@ -305,6 +309,9 @@ pub fn deinit(self: *Page) void { } } } + + self.arena_pool.release(self.call_arena); + self.arena_pool.release(self.arena); } pub fn base(self: *const Page) [:0]const u8 { @@ -807,7 +814,7 @@ fn pageErrorCallback(ctx: *anyopaque, err: anyerror) void { // why would we want to) and requires the body to live until the transfer // is complete. fn clearTransferArena(self: *Page) void { - _ = self._session.browser.transfer_arena.reset(.{ .retain_with_limit = 4 * 1024 }); + self.arena_pool.reset(self._session.transfer_arena, 4 * 1024); } pub fn wait(self: *Page, wait_ms: u32) Session.WaitResult { diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 5a10469d..e66360fb 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -65,18 +65,22 @@ page: ?Page, pub fn init(self: *Session, browser: *Browser, notification: *Notification) !void { const allocator = browser.app.allocator; - const session_allocator = browser.session_arena.allocator(); + const arena = try browser.arena_pool.acquire(); + errdefer browser.arena_pool.release(arena); + + const transfer_arena = try browser.arena_pool.acquire(); + errdefer browser.arena_pool.release(transfer_arena); self.* = .{ .page = null, + .arena = arena, .history = .{}, .navigation = .{}, .storage_shed = .{}, .browser = browser, .notification = notification, - .arena = session_allocator, + .transfer_arena = transfer_arena, .cookie_jar = storage.Cookie.Jar.init(allocator), - .transfer_arena = browser.transfer_arena.allocator(), }; } @@ -84,8 +88,12 @@ pub fn deinit(self: *Session) void { if (self.page != null) { self.removePage(); } + const browser = self.browser; + self.cookie_jar.deinit(); - self.storage_shed.deinit(self.browser.app.allocator); + self.storage_shed.deinit(browser.app.allocator); + browser.arena_pool.release(self.transfer_arena); + browser.arena_pool.release(self.arena); } // NOTE: the caller is not the owner of the returned value, @@ -93,8 +101,6 @@ pub fn deinit(self: *Session) void { pub fn createPage(self: *Session) !*Page { lp.assert(self.page == null, "Session.createPage - page not null", .{}); - _ = self.browser.page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); - self.page = @as(Page, undefined); const page = &self.page.?; try Page.init(page, self); @@ -134,8 +140,6 @@ pub fn replacePage(self: *Session) !*Page { lp.assert(self.page != null, "Session.replacePage null page", .{}); self.page.?.deinit(); - - _ = self.browser.page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); self.browser.env.memoryPressureNotification(.moderate); self.page = @as(Page, undefined); @@ -175,7 +179,7 @@ pub fn wait(self: *Session, wait_ms: u32) WaitResult { } fn processScheduledNavigation(self: *Session) !void { - defer _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 8 * 1024 }); + defer self.browser.arena_pool.reset(self.transfer_arena, 4 * 1024); const url, const opts = blk: { const qn = self.page.?._queued_navigation.?; // qn might not be safe to use after self.removePage is called, hence diff --git a/src/http/Client.zig b/src/http/Client.zig index cda2adeb..6e317d10 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -1255,7 +1255,6 @@ pub const Transfer = struct { client.endTransfer(self); } self.deinit(); - } pub fn terminate(self: *Transfer) void {