diff --git a/src/ArenaPool.zig b/src/ArenaPool.zig index 6adbac564..053d55bbe 100644 --- a/src/ArenaPool.zig +++ b/src/ArenaPool.zig @@ -34,6 +34,7 @@ const SAFETY = Arena.SAFETY; pub const BucketSize = enum { tiny, small, medium, large }; pub const Bucket = struct { + size: BucketSize, free_list: ?*Arena = null, free_list_len: u16 = 0, free_list_max: u16, @@ -66,10 +67,10 @@ pub fn init(allocator: Allocator, config: Config) ArenaPool { return .{ .allocator = allocator, .entry_pool = .empty, - .tiny = .{ .free_list_max = config.tiny.max, .retain_bytes = config.tiny.retain }, - .small = .{ .free_list_max = config.small.max, .retain_bytes = config.small.retain }, - .medium = .{ .free_list_max = config.medium.max, .retain_bytes = config.medium.retain }, - .large = .{ .free_list_max = config.large.max, .retain_bytes = config.large.retain }, + .tiny = .{ .size = .tiny, .free_list_max = config.tiny.max, .retain_bytes = config.tiny.retain }, + .small = .{ .size = .small, .free_list_max = config.small.max, .retain_bytes = config.small.retain }, + .medium = .{ .size = .medium, .free_list_max = config.medium.max, .retain_bytes = config.medium.retain }, + .large = .{ .size = .large, .free_list_max = config.large.max, .retain_bytes = config.large.retain }, }; } @@ -100,6 +101,13 @@ pub fn deinit(self: *ArenaPool) void { self.entry_pool.deinit(self.allocator); } +pub fn bucketFor(self: *const ArenaPool, size: usize) BucketSize { + if (size <= self.tiny.retain_bytes) return .tiny; + if (size <= self.small.retain_bytes) return .small; + if (size <= self.medium.retain_bytes) return .medium; + return .large; +} + // Acquire an arena from the pool. // - Pass a BucketSize (.tiny, .small, .medium, .large) for explicit bucket selection // - Pass a usize for automatic bucket selection based on expected size @@ -120,10 +128,7 @@ fn _acquire(self: *ArenaPool, account: ?*Arena.Account, size_or_bucket: anytype, break :blk @as(BucketSize, size_or_bucket); } if (T == usize or T == comptime_int) { - if (size_or_bucket <= self.tiny.retain_bytes) break :blk .tiny; - if (size_or_bucket <= self.small.retain_bytes) break :blk .small; - if (size_or_bucket <= self.medium.retain_bytes) break :blk .medium; - break :blk .large; + break :blk self.bucketFor(size_or_bucket); } @compileError("acquire expects BucketSize or usize, got " ++ @typeName(T)); }; @@ -135,6 +140,8 @@ fn _acquire(self: *ArenaPool, account: ?*Arena.Account, size_or_bucket: anytype, .large => &self.large, }; + lp.metrics.arena_inflight.incr(bucket_size); + self.mutex.lockUncancelable(lp.io); defer self.mutex.unlock(lp.io); @@ -186,6 +193,8 @@ pub fn release(self: *ArenaPool, entry: *Arena) void { const arena = &entry._arena; const bucket = entry.bucket; + lp.metrics.arena_inflight.decr(bucket.size); + if (IS_DEBUG) { self.mutex.lockUncancelable(lp.io); defer self.mutex.unlock(lp.io); diff --git a/src/Metrics.zig b/src/Metrics.zig index 53fbc9eef..0791e4031 100644 --- a/src/Metrics.zig +++ b/src/Metrics.zig @@ -31,6 +31,7 @@ script_errors: Counter = .{}, js_errors: CounterEnum("kind", enum { js_exception, other }) = .{}, arena_hit: CounterEnum("size", @import("ArenaPool.zig").BucketSize) = .{}, arena_miss: CounterEnum("size", @import("ArenaPool.zig").BucketSize) = .{}, +arena_inflight: GaugeEnum("size", @import("ArenaPool.zig").BucketSize) = .{}, arena_memory_bytes: Gauge = .{}, navigate: CounterEnum("type", @import("telemetry/telemetry.zig").Event.Navigate.Context) = .{}, js_heap_size_bytes: Histogram(&.{ @@ -87,6 +88,7 @@ const help = .{ .js_errors = "Uncaught JS errors (script exceptions, listener/callback throws, unhandled promise rejections); kind=js_exception is a thrown JS value, other is an internal failure (e.g. compilation error, terminated execution)", .arena_hit = "Arena pool acquisitions served from the free list", .arena_miss = "Arena pool acquisitions that had to allocate a new arena", + .arena_inflight = "Arenas currently checked out of the pool. Above the bucket's max, every acquisition is a miss and every release is discarded", .arena_memory_bytes = "Backing memory held by pooled arenas, including capacity retained on the free list", .navigate = "Navigations by initiating frame type", .js_heap_size_bytes = "V8 heap physical size, sampled when a page is closed", @@ -164,6 +166,33 @@ const Gauge = struct { } }; +fn GaugeEnum(comptime label: []const u8, comptime T: type) type { + return struct { + values: std.enums.EnumArray(T, Gauge) = .initFill(.{}), + + pub const Tag = T; + pub const label_name = label; + + const Self = @This(); + + pub fn incr(self: *Self, tag: T) void { + self.values.getPtr(tag).incr(); + } + + pub fn decr(self: *Self, tag: T) void { + self.values.getPtr(tag).decr(); + } + + fn write(self: *const Self, comptime name: []const u8, comptime help_text: []const u8, writer: *std.Io.Writer) !void { + try writer.writeAll("# HELP " ++ name ++ " " ++ help_text ++ "\n" ++ "# TYPE " ++ name ++ " gauge\n"); + inline for (comptime std.enums.values(Tag)) |tag| { + const value = @atomicLoad(isize, &self.values.getPtrConst(tag).value, .monotonic); + try writer.print(name ++ "{{" ++ label ++ "=\"" ++ @tagName(tag) ++ "\"}} {d}\n", .{value}); + } + } + }; +} + fn CounterEnum(comptime label: []const u8, comptime T: type) type { return struct { counts: std.enums.EnumArray(T, Counter) = .initFill(.{}), diff --git a/src/browser/Factory.zig b/src/browser/Factory.zig index 4c0a5a4a0..3dd35c56d 100644 --- a/src/browser/Factory.zig +++ b/src/browser/Factory.zig @@ -526,12 +526,10 @@ pub fn destroy(self: *Factory, value: anytype) void { if (comptime IS_DEBUG) { // We should always destroy from the leaf down. - if (@hasDecl(S, "_prototype_root")) { - // A Event{._type == .generic} (or any other similar types) - // _should_ be destroyed directly. The _type = .generic is a pseudo - // child - if (S != Event or value._type != .generic) { - log.fatal(.bug, "factory.destroy.event", .{ .type = @typeName(S) }); + if (comptime @hasDecl(S, "_prototype_root")) { + const is_leaf = if (comptime @hasField(S, "_type")) value._type == .generic else false; + if (!is_leaf) { + log.fatal(.bug, "factory.destroy.root", .{ .type = @typeName(S) }); unreachable; } } diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 30bc1769e..77dad6cd0 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -2180,7 +2180,8 @@ pub fn loadExternalStylesheet(self: *Frame, link: *Element.Html.Link, href: []co } const element = link.asElement(); - const arena = try session.getArena(.medium, "Frame.loadExternalStylesheet"); + // HttpClient will take out a larger arena for the body, if necessary + const arena = try session.getArena(.small, "Frame.loadExternalStylesheet"); defer arena.release(); const resolved = URL.resolve(arena.allocator(), self.base(), href, .{ .encoding = self.charset }) catch |err| { @@ -2212,7 +2213,7 @@ pub fn loadExternalStylesheet(self: *Frame, link: *Element.Html.Link, href: []co sm.is_evaluating = true; defer sm.endEvaluationWindow(was_evaluating); - var response = http_client.syncRequest(arena.allocator(), .{ + var response = http_client.syncRequest(.{ .url = resolved, .method = .GET, .frame_id = self._frame_id, @@ -2227,7 +2228,7 @@ pub fn loadExternalStylesheet(self: *Frame, link: *Element.Html.Link, href: []co log.warn(.http, "external stylesheet fetch", .{ .err = err, .url = resolved }); return self.fireElementEvent(element, comptime .wrap("error")); }; - defer response.deinit(arena.allocator()); + defer response.deinit(); if (response.status < 200 or response.status >= 300) { log.info(.http, "external stylesheet status", .{ .status = response.status, .url = resolved }); diff --git a/src/browser/ScriptManager.zig b/src/browser/ScriptManager.zig index 952b82934..a0b4ae485 100644 --- a/src/browser/ScriptManager.zig +++ b/src/browser/ScriptManager.zig @@ -112,7 +112,7 @@ pub fn preloadScript(self: *ScriptManager, element: ?*Element.Html, url: []const } const frame = self.frame; - const arena = try frame.getArena(.large, "SM.preloadScript"); + const arena = try frame.getArena(.small, "SM.preloadScript"); errdefer arena.release(); const owned_url = try arena.dupeZ(u8, url); @@ -240,7 +240,7 @@ pub fn addFromElement(self: *ScriptManager, comptime from_parser: bool, script_e // released early on the adoption path — so the errdefer can't double-free. var handover = false; - const arena = try frame.getArena(.large, "SM.addFromElement"); + const arena = try frame.getArena(.small, "SM.addFromElement"); errdefer if (handover == false) { arena.release(); }; @@ -354,7 +354,7 @@ pub fn addFromElement(self: *ScriptManager, comptime from_parser: bool, script_e script.status = pre.status; script.complete = true; } else { - const response = try self.base.client.syncRequest(arena.allocator(), .{ + const response = try self.base.client.syncRequest(.{ .url = remote_url, .method = .GET, .frame_id = frame._frame_id, @@ -367,6 +367,9 @@ pub fn addFromElement(self: *ScriptManager, comptime from_parser: bool, script_e .shutdown_callback = HttpClient.noopShutdown, // syncRequest installs its own }, &frame._http_owner); + // Take the body's arena rather than releasing it: `source` + // has to outlive this call, up to script.deinit(). + script.source_arena = response.arena; script.source = .{ .remote = response.body }; script.status = response.status; script.complete = true; diff --git a/src/browser/ScriptManagerBase.zig b/src/browser/ScriptManagerBase.zig index 2069bfee7..be2e9a7a5 100644 --- a/src/browser/ScriptManagerBase.zig +++ b/src/browser/ScriptManagerBase.zig @@ -234,7 +234,7 @@ pub fn preloadImport(self: *ScriptManagerBase, url: [:0]const u8, referrer: []co } errdefer _ = self.imported_modules.remove(url); - const arena = try self.acquireArena(.large, "SM.preloadImport"); + const arena = try self.acquireArena(.small, "SM.preloadImport"); errdefer arena.release(); const script = try arena.create(Script); @@ -426,7 +426,7 @@ pub fn getAsyncImport(self: *ScriptManagerBase, url: [:0]const u8, cb: ImportAsy } } - const arena = try self.acquireArena(.large, "SM.getAsyncImport"); + const arena = try self.acquireArena(.small, "SM.getAsyncImport"); errdefer arena.release(); const script = try arena.create(Script); @@ -611,6 +611,13 @@ pub const Script = struct { source: Source, url: []const u8, arena: *lp.Arena, + + // Where `source` lives, when it isn't `arena`. The double-arena lets us + // use a .small arena for the Script itself, and then a properly sized one + // for the body, when we know its size. This avoids eager-usage of our + // limited .large arena pool + source_arena: ?*lp.Arena = null, + extra: Extra, node: std.DoublyLinkedList.Node, manager: *ScriptManagerBase, @@ -687,9 +694,18 @@ pub const Script = struct { }; pub fn deinit(self: *Script) void { + if (self.source_arena) |source_arena| { + source_arena.release(); + } self.arena.release(); } + // The allocator `source` grows from. Falls back to the control arena when + // no header callback ran to size a dedicated one. + fn sourceAllocator(self: *Script) Allocator { + return (self.source_arena orelse self.arena).allocator(); + } + pub fn startCallback(transfer: *HttpClient.Transfer) !void { log.debug(.http, "script fetch start", .{ .req = transfer }); } @@ -748,9 +764,19 @@ pub const Script = struct { } lp.assert(self.source.remote.capacity == 0, "ScriptManagerBase.Header buffer", .{ .capacity = self.source.remote.capacity }); + + const content_length = transfer.getContentLength(); + if (self.source_arena == null) { + // A redirect re-runs this callback; keep the arena we already have. + self.source_arena = if (content_length) |cl| + try self.manager.acquireArena(cl, "SM.source") + else + try self.manager.acquireArena(.large, "SM.source"); + } + var buffer: std.ArrayList(u8) = .empty; - if (transfer.getContentLength()) |cl| { - try buffer.ensureTotalCapacity(self.arena.allocator(), cl); + if (content_length) |cl| { + try buffer.ensureTotalCapacity(self.sourceAllocator(), cl); } self.source = .{ .remote = buffer }; return .proceed; @@ -765,7 +791,7 @@ pub const Script = struct { } fn _dataCallback(self: *Script, _: *HttpClient.Transfer, data: []const u8) !void { - try self.source.remote.appendSlice(self.arena.allocator(), data); + try self.source.remote.appendSlice(self.sourceAllocator(), data); } pub fn doneCallback(ctx: *anyopaque) !void { diff --git a/src/browser/tests/encoding/text_decoder.html b/src/browser/tests/encoding/text_decoder.html index 665e651ba..4f5574f40 100644 --- a/src/browser/tests/encoding/text_decoder.html +++ b/src/browser/tests/encoding/text_decoder.html @@ -49,6 +49,20 @@ testing.expectEqual('♥', d3.decode(new Uint8Array([165]), { stream: true })); + +