From 2d1b9d64bd09799548a1b61a4a23fdd610e8567e Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sat, 19 Apr 2025 14:30:50 +0800 Subject: [PATCH] Remove unnecessary cleanup when we know we have an arena Change a few old alloc+memcpy to dupe Some other smaller cleanups. --- src/browser/dom/html_collection.zig | 122 +++++++++----------------- src/browser/dom/mutation_observer.zig | 26 ++---- src/browser/url/url.zig | 1 - src/browser/xhr/xhr.zig | 70 ++++----------- src/cdp/domains/page.zig | 1 - 5 files changed, 67 insertions(+), 153 deletions(-) diff --git a/src/browser/dom/html_collection.zig b/src/browser/dom/html_collection.zig index 812b6931..9580b8f0 100644 --- a/src/browser/dom/html_collection.zig +++ b/src/browser/dom/html_collection.zig @@ -17,6 +17,7 @@ // along with this program. If not, see . const std = @import("std"); +const Allocator = std.mem.Allocator; const parser = @import("../netsurf.zig"); @@ -42,25 +43,11 @@ const Matcher = union(enum) { pub fn match(self: Matcher, node: *parser.Node) !bool { switch (self) { - inline .matchTrue => return true, - inline .matchFalse => return false, - inline .matchByTagName => |case| return case.match(node), - inline .matchByClassName => |case| return case.match(node), - inline .matchByName => |case| return case.match(node), - inline .matchByLinks => return MatchByLinks.match(node), - inline .matchByAnchors => return MatchByAnchors.match(node), - } - } - - pub fn deinit(self: Matcher, alloc: std.mem.Allocator) void { - switch (self) { - inline .matchTrue => return, - inline .matchFalse => return, - inline .matchByTagName => |case| return case.deinit(alloc), - inline .matchByClassName => |case| return case.deinit(alloc), - inline .matchByName => |case| return case.deinit(alloc), - inline .matchByLinks => return, - inline .matchByAnchors => return, + .matchTrue => return true, + .matchFalse => return false, + .matchByLinks => return MatchByLinks.match(node), + .matchByAnchors => return MatchByAnchors.match(node), + inline else => |m| return m.match(node), } } }; @@ -71,54 +58,49 @@ pub const MatchByTagName = struct { tag: []const u8, is_wildcard: bool, - fn init(alloc: std.mem.Allocator, tag_name: []const u8) !MatchByTagName { - const tag_name_alloc = try alloc.alloc(u8, tag_name.len); - @memcpy(tag_name_alloc, tag_name); - return MatchByTagName{ - .tag = tag_name_alloc, - .is_wildcard = std.mem.eql(u8, tag_name, "*"), + fn init(arena: Allocator, tag_name: []const u8) !MatchByTagName { + if (std.mem.eql(u8, tag_name, "*")) { + return .{ .tag = "*", .is_wildcard = true }; + } + + return .{ + .tag = try arena.dupe(u8, tag_name), + .is_wildcard = false, }; } pub fn match(self: MatchByTagName, node: *parser.Node) !bool { return self.is_wildcard or std.ascii.eqlIgnoreCase(self.tag, try parser.nodeName(node)); } - - fn deinit(self: MatchByTagName, alloc: std.mem.Allocator) void { - alloc.free(self.tag); - } }; pub fn HTMLCollectionByTagName( - alloc: std.mem.Allocator, + arena: Allocator, root: ?*parser.Node, tag_name: []const u8, include_root: bool, ) !HTMLCollection { return HTMLCollection{ .root = root, - .walker = Walker{ .walkerDepthFirst = .{} }, - .matcher = Matcher{ - .matchByTagName = try MatchByTagName.init(alloc, tag_name), - }, + .walker = .{ .walkerDepthFirst = .{} }, + .matcher = .{ .matchByTagName = try MatchByTagName.init(arena, tag_name) }, .include_root = include_root, }; } pub const MatchByClassName = struct { - classNames: []const u8, + class_names: []const u8, - fn init(alloc: std.mem.Allocator, classNames: []const u8) !MatchByClassName { - const class_names_alloc = try alloc.alloc(u8, classNames.len); - @memcpy(class_names_alloc, classNames); - return MatchByClassName{ - .classNames = class_names_alloc, + fn init(arena: Allocator, class_names: []const u8) !MatchByClassName { + return .{ + .class_names = try arena.dupe(u8, class_names), }; } pub fn match(self: MatchByClassName, node: *parser.Node) !bool { - var it = std.mem.splitAny(u8, self.classNames, " "); const e = parser.nodeToElement(node); + + var it = std.mem.splitScalar(u8, self.class_names, ' '); while (it.next()) |c| { if (!try parser.elementHasClass(e, c)) { return false; @@ -127,24 +109,18 @@ pub const MatchByClassName = struct { return true; } - - fn deinit(self: MatchByClassName, alloc: std.mem.Allocator) void { - alloc.free(self.classNames); - } }; pub fn HTMLCollectionByClassName( - alloc: std.mem.Allocator, + arena: Allocator, root: ?*parser.Node, classNames: []const u8, include_root: bool, ) !HTMLCollection { return HTMLCollection{ .root = root, - .walker = Walker{ .walkerDepthFirst = .{} }, - .matcher = Matcher{ - .matchByClassName = try MatchByClassName.init(alloc, classNames), - }, + .walker = .{ .walkerDepthFirst = .{} }, + .matcher = .{ .matchByClassName = try MatchByClassName.init(arena, classNames) }, .include_root = include_root, }; } @@ -152,11 +128,9 @@ pub fn HTMLCollectionByClassName( pub const MatchByName = struct { name: []const u8, - fn init(alloc: std.mem.Allocator, name: []const u8) !MatchByName { - const names_alloc = try alloc.alloc(u8, name.len); - @memcpy(names_alloc, name); - return MatchByName{ - .name = names_alloc, + fn init(arena: Allocator, name: []const u8) !MatchByName { + return .{ + .name = try arena.dupe(u8, name), }; } @@ -165,24 +139,18 @@ pub const MatchByName = struct { const nname = try parser.elementGetAttribute(e, "name") orelse return false; return std.mem.eql(u8, self.name, nname); } - - fn deinit(self: MatchByName, alloc: std.mem.Allocator) void { - alloc.free(self.name); - } }; pub fn HTMLCollectionByName( - alloc: std.mem.Allocator, + arena: Allocator, root: ?*parser.Node, name: []const u8, include_root: bool, ) !HTMLCollection { return HTMLCollection{ .root = root, - .walker = Walker{ .walkerDepthFirst = .{} }, - .matcher = Matcher{ - .matchByName = try MatchByName.init(alloc, name), - }, + .walker = .{ .walkerDepthFirst = .{} }, + .matcher = .{ .matchByName = try MatchByName.init(arena, name) }, .include_root = include_root, }; } @@ -193,8 +161,8 @@ pub fn HTMLCollectionAll( ) !HTMLCollection { return HTMLCollection{ .root = root, - .walker = Walker{ .walkerDepthFirst = .{} }, - .matcher = Matcher{ .matchTrue = .{} }, + .walker = .{ .walkerDepthFirst = .{} }, + .matcher = .{ .matchTrue = .{} }, .include_root = include_root, }; } @@ -205,8 +173,8 @@ pub fn HTMLCollectionChildren( ) !HTMLCollection { return HTMLCollection{ .root = root, - .walker = Walker{ .walkerChildren = .{} }, - .matcher = Matcher{ .matchTrue = .{} }, + .walker = .{ .walkerChildren = .{} }, + .matcher = .{ .matchTrue = .{} }, .include_root = include_root, }; } @@ -214,8 +182,8 @@ pub fn HTMLCollectionChildren( pub fn HTMLCollectionEmpty() !HTMLCollection { return HTMLCollection{ .root = null, - .walker = Walker{ .walkerNone = .{} }, - .matcher = Matcher{ .matchFalse = .{} }, + .walker = .{ .walkerNone = .{} }, + .matcher = .{ .matchFalse = .{} }, .include_root = false, }; } @@ -240,10 +208,8 @@ pub fn HTMLCollectionByLinks( ) !HTMLCollection { return HTMLCollection{ .root = root, - .walker = Walker{ .walkerDepthFirst = .{} }, - .matcher = Matcher{ - .matchByLinks = MatchByLinks{}, - }, + .walker = .{ .walkerDepthFirst = .{} }, + .matcher = .{ .matchByLinks = MatchByLinks{} }, .include_root = include_root, }; } @@ -267,10 +233,8 @@ pub fn HTMLCollectionByAnchors( ) !HTMLCollection { return HTMLCollection{ .root = root, - .walker = Walker{ .walkerDepthFirst = .{} }, - .matcher = Matcher{ - .matchByAnchors = MatchByAnchors{}, - }, + .walker = .{ .walkerDepthFirst = .{} }, + .matcher = .{ .matchByAnchors = MatchByAnchors{} }, .include_root = include_root, }; } @@ -321,7 +285,7 @@ pub const HTMLCollection = struct { cur_node: ?*parser.Node = undefined, // start returns the first node to walk on. - fn start(self: HTMLCollection) !?*parser.Node { + fn start(self: *const HTMLCollection) !?*parser.Node { if (self.root == null) return null; if (self.include_root) { diff --git a/src/browser/dom/mutation_observer.zig b/src/browser/dom/mutation_observer.zig index 38da1bd8..c6b57b8d 100644 --- a/src/browser/dom/mutation_observer.zig +++ b/src/browser/dom/mutation_observer.zig @@ -45,13 +45,6 @@ pub const MutationObserver = struct { options: MutationObserverInit, }; - const deinitFunc = struct { - fn deinit(ctx: ?*anyopaque, allocator: std.mem.Allocator) void { - const o: *Observer = @ptrCast(@alignCast(ctx)); - allocator.destroy(o); - } - }.deinit; - const Observers = std.ArrayListUnmanaged(*Observer); pub const MutationObserverInit = struct { @@ -104,7 +97,7 @@ pub const MutationObserver = struct { arena, "DOMNodeInserted", EventHandler, - .{ .cbk = self.cbk, .ctx = o, .deinitFunc = deinitFunc }, + .{ .cbk = self.cbk, .ctx = o }, false, ); try parser.eventTargetAddEventListener( @@ -112,7 +105,7 @@ pub const MutationObserver = struct { arena, "DOMNodeRemoved", EventHandler, - .{ .cbk = self.cbk, .ctx = o, .deinitFunc = deinitFunc }, + .{ .cbk = self.cbk, .ctx = o }, false, ); } @@ -122,7 +115,7 @@ pub const MutationObserver = struct { arena, "DOMAttrModified", EventHandler, - .{ .cbk = self.cbk, .ctx = o, .deinitFunc = deinitFunc }, + .{ .cbk = self.cbk, .ctx = o }, false, ); } @@ -132,7 +125,7 @@ pub const MutationObserver = struct { arena, "DOMCharacterDataModified", EventHandler, - .{ .cbk = self.cbk, .ctx = o, .deinitFunc = deinitFunc }, + .{ .cbk = self.cbk, .ctx = o }, false, ); } @@ -142,7 +135,7 @@ pub const MutationObserver = struct { arena, "DOMSubtreeModified", EventHandler, - .{ .cbk = self.cbk, .ctx = o, .deinitFunc = deinitFunc }, + .{ .cbk = self.cbk, .ctx = o }, false, ); } @@ -153,15 +146,6 @@ pub const MutationObserver = struct { // TODO unregister listeners. } - pub fn deinit(self: *MutationObserver, state: *SessionState) void { - const arena = state.arena; - // TODO unregister listeners. - for (self.observers.items) |o| { - arena.destroy(o); - } - self.observers.deinit(arena); - } - // TODO pub fn _takeRecords(_: *const MutationObserver) ?[]const u8 { return &[_]u8{}; diff --git a/src/browser/url/url.zig b/src/browser/url/url.zig index 7c7bf0a9..3bf496bd 100644 --- a/src/browser/url/url.zig +++ b/src/browser/url/url.zig @@ -51,7 +51,6 @@ pub const URL = struct { ) !URL { const arena = state.arena; const raw = try std.mem.concat(arena, u8, &[_][]const u8{ url, base orelse "" }); - errdefer arena.free(raw); const uri = std.Uri.parse(raw) catch return error.TypeError; return init(arena, uri); diff --git a/src/browser/xhr/xhr.zig b/src/browser/xhr/xhr.zig index 716cef44..b974fdad 100644 --- a/src/browser/xhr/xhr.zig +++ b/src/browser/xhr/xhr.zig @@ -48,15 +48,7 @@ pub const XMLHttpRequestUpload = struct { proto: XMLHttpRequestEventTarget = XMLHttpRequestEventTarget{}, }; -pub const XMLHttpRequestBodyInitTag = enum { - Blob, - BufferSource, - FormData, - URLSearchParams, - String, -}; - -pub const XMLHttpRequestBodyInit = union(XMLHttpRequestBodyInitTag) { +const XMLHttpRequestBodyInit = union(enum) { Blob: []const u8, BufferSource: []const u8, FormData: []const u8, @@ -173,11 +165,6 @@ pub const XMLHttpRequest = struct { }; } - fn deinit(self: *Headers) void { - self.free(); - self.list.deinit(self.arena); - } - fn append(self: *Headers, k: []const u8, v: []const u8) !void { // duplicate strings const kk = try self.arena.dupe(u8, k); @@ -185,17 +172,8 @@ pub const XMLHttpRequest = struct { try self.list.append(self.arena, .{ .name = kk, .value = vv }); } - // free all strings allocated. - fn free(self: *Headers) void { - for (self.list.items) |h| { - self.arena.free(h.name); - self.arena.free(h.value); - } - } - - fn clearAndFree(self: *Headers) void { - self.free(); - self.list.clearAndFree(self.arena); + fn reset(self: *Headers) void { + self.list.clearRetainingCapacity(); } fn has(self: Headers, k: []const u8) bool { @@ -222,9 +200,7 @@ pub const XMLHttpRequest = struct { fn set(self: *Headers, k: []const u8, v: []const u8) !void { for (self.list.items, 0..) |h, i| { if (std.ascii.eqlIgnoreCase(k, h.name)) { - const hh = self.list.swapRemove(i); - self.arena.free(hh.name); - self.arena.free(hh.value); + _ = self.list.swapRemove(i); } } self.append(k, v); @@ -247,25 +223,19 @@ pub const XMLHttpRequest = struct { JSON: JSONValue, }; - const ResponseObjTag = enum { - Document, - Failure, - JSON, - }; - const ResponseObj = union(ResponseObjTag) { + const ResponseObj = union(enum) { Document: *parser.Document, Failure: void, - JSON: std.json.Parsed(JSONValue), + JSON: JSONValue, fn deinit(self: ResponseObj) void { - return switch (self) { + switch (self) { + .JSON, .Failure => {}, .Document => |d| { const doc = @as(*parser.DocumentHTML, @ptrCast(d)); parser.documentHTMLClose(doc) catch {}; }, - .JSON => |p| p.deinit(), - .Failure => {}, - }; + } } }; @@ -291,15 +261,17 @@ pub const XMLHttpRequest = struct { pub fn reset(self: *XMLHttpRequest) void { self.url = null; - if (self.response_obj) |v| v.deinit(); + if (self.response_obj) |v| { + v.deinit(); + } self.response_obj = null; self.response_type = .Empty; self.response_mime = null; // TODO should we clearRetainingCapacity instead? - self.headers.clearAndFree(); - self.response_headers.clearAndFree(); + self.headers.reset(); + self.response_headers.reset(); self.response_status = 0; self.send_flag = false; @@ -308,13 +280,9 @@ pub const XMLHttpRequest = struct { } pub fn deinit(self: *XMLHttpRequest, alloc: Allocator) void { - self.reset(); - self.headers.deinit(); - self.response_headers.deinit(); - if (self.response_mime) |*mime| { - mime.deinit(); + if (self.response_obj) |v| { + v.deinit(); } - self.proto.deinit(alloc); } @@ -666,7 +634,7 @@ pub const XMLHttpRequest = struct { return switch (obj) { .Failure => null, .Document => |v| .{ .Document = v }, - .JSON => |v| .{ .JSON = v.value }, + .JSON => |v| .{ .JSON = v }, }; } @@ -707,7 +675,7 @@ pub const XMLHttpRequest = struct { return switch (obj) { .Failure => null, .Document => |v| .{ .Document = v }, - .JSON => |v| .{ .JSON = v.value }, + .JSON => |v| .{ .JSON = v }, }; } @@ -753,7 +721,7 @@ pub const XMLHttpRequest = struct { fn setResponseObjJSON(self: *XMLHttpRequest) void { // TODO should we use parseFromSliceLeaky if we expect the allocator is // already an arena? - const p = std.json.parseFromSlice( + const p = std.json.parseFromSliceLeaky( JSONValue, self.arena, self.response_bytes.items, diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 94c569ab..a261c0ed 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -158,7 +158,6 @@ fn navigate(cmd: anytype) !void { .loaderId = bc.loader_id, }, .{}); - std.debug.print("page: {s}\n", .{target_id}); try page.navigate(url, .{ .reason = .address_bar, });