Add RC support to NodeList

Most importantly, this allows the Selector.List to be self-contained with
an arena from the ArenaPool. Selector.List can be both relatively large
and relatively common, so moving it off the page.arena is a nice win.

Also applied this to ChildNodes, which is much smaller but could also be
called often.

I was initially going to hook into the v8::Object's internal fields to store
the referencing v8::Object. So the v8::Object representing the Iterator
would store the v8::Object representing the NodeList inside of its internal
field - which the GC would trace/detect/respect. And that is probably the
fastest and most v8-ish solution, but I couldn't come up with an elegant
solution. The best I had was having a "afterCreate" callback which passed
the v8 object (this is similar to the old postAttach callback we had, but
used for a different purpose). However, since "acquireRef" was recently
added to events, re-using that was much simpler and worked well.
This commit is contained in:
Karl Seguin
2026-02-27 10:17:30 +08:00
parent a14ad6f700
commit 315c9a2d92
10 changed files with 131 additions and 31 deletions

View File

@@ -442,3 +442,29 @@
} }
</script> </script>
<script id=iterator_list_lifetime>
// This test is intended to ensure that a list remains alive as long as it
// must, i.e. as long as any iterator referencing the list is alive.
// This test depends on being able to force the v8 GC to cleanup, which
// we have no way of controlling. At worst, the test will pass without
// actually testing correct lifetime. But it was at least manually verified
// for me that this triggers plenty of GCs.
const expected = Array.from(document.querySelectorAll('*')).length;
{
let keys = [];
// Phase 1: Create many lists+iterators to fill up the arena pool
for (let i = 0; i < 1000; i++) {
let list = document.querySelectorAll('*');
keys.push(list.keys());
// Create an Event every iteration to compete for arenas
new Event('arena_compete');
}
for (let k of keys) {
const result = Array.from(k);
testing.expectEqual(expected, result.length);
}
}
</script>

View File

@@ -26,6 +26,7 @@ const GenericIterator = @import("iterator.zig").Entry;
// No need to go through a TreeWalker or add any filtering. // No need to go through a TreeWalker or add any filtering.
const ChildNodes = @This(); const ChildNodes = @This();
_arena: std.mem.Allocator,
_last_index: usize, _last_index: usize,
_last_length: ?u32, _last_length: ?u32,
_last_node: ?*std.DoublyLinkedList.Node, _last_node: ?*std.DoublyLinkedList.Node,
@@ -37,13 +38,23 @@ pub const ValueIterator = GenericIterator(Iterator, "1");
pub const EntryIterator = GenericIterator(Iterator, null); pub const EntryIterator = GenericIterator(Iterator, null);
pub fn init(node: *Node, page: *Page) !*ChildNodes { pub fn init(node: *Node, page: *Page) !*ChildNodes {
return page._factory.create(ChildNodes{ const arena = try page.getArena(.{ .debug = "ChildNodes" });
errdefer page.releaseArena(arena);
const self = try arena.create(ChildNodes);
self.* = .{
._node = node, ._node = node,
._arena = arena,
._last_index = 0, ._last_index = 0,
._last_node = null, ._last_node = null,
._last_length = null, ._last_length = null,
._cached_version = page.version, ._cached_version = page.version,
}); };
return self;
}
pub fn deinit(self: *const ChildNodes, page: *Page) void {
page.releaseArena(self._arena);
} }
pub fn length(self: *ChildNodes, page: *Page) !u32 { pub fn length(self: *ChildNodes, page: *Page) !u32 {
@@ -115,7 +126,7 @@ fn versionCheck(self: *ChildNodes, page: *Page) bool {
const NodeList = @import("NodeList.zig"); const NodeList = @import("NodeList.zig");
pub fn runtimeGenericWrap(self: *ChildNodes, page: *Page) !*NodeList { pub fn runtimeGenericWrap(self: *ChildNodes, page: *Page) !*NodeList {
return page._factory.create(NodeList{ .data = .{ .child_nodes = self } }); return page._factory.create(NodeList{ ._data = .{ .child_nodes = self } });
} }
const Iterator = struct { const Iterator = struct {

View File

@@ -85,7 +85,7 @@ pub fn namedItem(self: *HTMLFormControlsCollection, name: []const u8, page: *Pag
._name = try page.dupeString(name), ._name = try page.dupeString(name),
}); });
radio_node_list._proto = try page._factory.create(NodeList{ .data = .{ .radio_node_list = radio_node_list } }); radio_node_list._proto = try page._factory.create(NodeList{ ._data = .{ .radio_node_list = radio_node_list } });
return .{ .radio_node_list = radio_node_list }; return .{ .radio_node_list = radio_node_list };
} }

View File

@@ -28,24 +28,36 @@ const RadioNodeList = @import("RadioNodeList.zig");
const SelectorList = @import("../selector/List.zig"); const SelectorList = @import("../selector/List.zig");
const NodeLive = @import("node_live.zig").NodeLive; const NodeLive = @import("node_live.zig").NodeLive;
const Mode = enum {
child_nodes,
selector_list,
radio_node_list,
name,
};
const NodeList = @This(); const NodeList = @This();
data: union(Mode) { _data: union(enum) {
child_nodes: *ChildNodes, child_nodes: *ChildNodes,
selector_list: *SelectorList, selector_list: *SelectorList,
radio_node_list: *RadioNodeList, radio_node_list: *RadioNodeList,
name: NodeLive(.name), name: NodeLive(.name),
}, },
_rc: usize = 0,
pub fn deinit(self: *NodeList, _: bool, page: *Page) void {
const rc = self._rc;
if (rc > 1) {
self._rc = rc - 1;
return;
}
switch (self._data) {
.selector_list => |list| list.deinit(page),
.child_nodes => |cn| cn.deinit(page),
else => {},
}
}
pub fn acquireRef(self: *NodeList) void {
self._rc += 1;
}
pub fn length(self: *NodeList, page: *Page) !u32 { pub fn length(self: *NodeList, page: *Page) !u32 {
return switch (self.data) { return switch (self._data) {
.child_nodes => |impl| impl.length(page), .child_nodes => |impl| impl.length(page),
.selector_list => |impl| @intCast(impl.getLength()), .selector_list => |impl| @intCast(impl.getLength()),
.radio_node_list => |impl| impl.getLength(), .radio_node_list => |impl| impl.getLength(),
@@ -58,7 +70,7 @@ pub fn indexedGet(self: *NodeList, index: usize, page: *Page) !*Node {
} }
pub fn getAtIndex(self: *NodeList, index: usize, page: *Page) !?*Node { pub fn getAtIndex(self: *NodeList, index: usize, page: *Page) !?*Node {
return switch (self.data) { return switch (self._data) {
.child_nodes => |impl| impl.getAtIndex(index, page), .child_nodes => |impl| impl.getAtIndex(index, page),
.selector_list => |impl| impl.getAtIndex(index), .selector_list => |impl| impl.getAtIndex(index),
.radio_node_list => |impl| impl.getAtIndex(index, page), .radio_node_list => |impl| impl.getAtIndex(index, page),
@@ -106,6 +118,14 @@ const Iterator = struct {
const Entry = struct { u32, *Node }; const Entry = struct { u32, *Node };
pub fn deinit(self: *Iterator, shutdown: bool, page: *Page) void {
self.list.deinit(shutdown, page);
}
pub fn acquireRef(self: *Iterator) void {
self.list.acquireRef();
}
pub fn next(self: *Iterator, page: *Page) !?Entry { pub fn next(self: *Iterator, page: *Page) !?Entry {
const index = self.index; const index = self.index;
const node = try self.list.getAtIndex(index, page) orelse return null; const node = try self.list.getAtIndex(index, page) orelse return null;
@@ -122,6 +142,8 @@ pub const JsApi = struct {
pub const prototype_chain = bridge.prototypeChain(); pub const prototype_chain = bridge.prototypeChain();
pub var class_id: bridge.ClassId = undefined; pub var class_id: bridge.ClassId = undefined;
pub const enumerable = false; pub const enumerable = false;
pub const weak = true;
pub const finalizer = bridge.finalizer(NodeList.deinit);
}; };
pub const length = bridge.accessor(NodeList.length, null, .{}); pub const length = bridge.accessor(NodeList.length, null, .{});

View File

@@ -21,8 +21,7 @@ const js = @import("../../js/js.zig");
const Page = @import("../../Page.zig"); const Page = @import("../../Page.zig");
pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type { pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type {
const InnerStruct = Inner; const R = reflect(Inner, field);
const R = reflect(InnerStruct, field);
return struct { return struct {
inner: Inner, inner: Inner,
@@ -40,6 +39,18 @@ pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type {
return page._factory.create(Self{ .inner = inner }); return page._factory.create(Self{ .inner = inner });
} }
pub fn deinit(self: *Self, shutdown: bool, page: *Page) void {
if (@hasDecl(Inner, "deinit")) {
self.inner.deinit(shutdown, page);
}
}
pub fn acquireRef(self: *Self) void {
if (@hasDecl(Inner, "acquireRef")) {
self.inner.acquireRef();
}
}
pub fn next(self: *Self, page: *Page) if (R.has_error_return) anyerror!Result else Result { pub fn next(self: *Self, page: *Page) if (R.has_error_return) anyerror!Result else Result {
const entry = (if (comptime R.has_error_return) try self.inner.next(page) else self.inner.next(page)) orelse { const entry = (if (comptime R.has_error_return) try self.inner.next(page) else self.inner.next(page)) orelse {
return .{ .done = true, .value = null }; return .{ .done = true, .value = null };
@@ -61,6 +72,8 @@ pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type {
pub const Meta = struct { pub const Meta = struct {
pub const prototype_chain = bridge.prototypeChain(); pub const prototype_chain = bridge.prototypeChain();
pub var class_id: bridge.ClassId = undefined; pub var class_id: bridge.ClassId = undefined;
pub const weak = true;
pub const finalizer = bridge.finalizer(Self.deinit);
}; };
pub const next = bridge.function(Self.next, .{ .null_as_undefined = true }); pub const next = bridge.function(Self.next, .{ .null_as_undefined = true });

View File

@@ -349,7 +349,7 @@ pub fn NodeLive(comptime mode: Mode) type {
pub fn runtimeGenericWrap(self: Self, page: *Page) !if (mode == .name) *NodeList else *HTMLCollection { pub fn runtimeGenericWrap(self: Self, page: *Page) !if (mode == .name) *NodeList else *HTMLCollection {
const collection = switch (mode) { const collection = switch (mode) {
.name => return page._factory.create(NodeList{ .data = .{ .name = self } }), .name => return page._factory.create(NodeList{ ._data = .{ .name = self } }),
.tag => HTMLCollection{ ._data = .{ .tag = self } }, .tag => HTMLCollection{ ._data = .{ .tag = self } },
.tag_name => HTMLCollection{ ._data = .{ .tag_name = self } }, .tag_name => HTMLCollection{ ._data = .{ .tag_name = self } },
.tag_name_ns => HTMLCollection{ ._data = .{ .tag_name_ns = self } }, .tag_name_ns => HTMLCollection{ ._data = .{ .tag_name_ns = self } },

View File

@@ -40,6 +40,10 @@ pub const EntryIterator = GenericIterator(Iterator, null);
pub const KeyIterator = GenericIterator(Iterator, "0"); pub const KeyIterator = GenericIterator(Iterator, "0");
pub const ValueIterator = GenericIterator(Iterator, "1"); pub const ValueIterator = GenericIterator(Iterator, "1");
pub fn deinit(self: *const List, page: *Page) void {
page.releaseArena(self._arena);
}
pub fn collect( pub fn collect(
allocator: std.mem.Allocator, allocator: std.mem.Allocator,
root: *Node, root: *Node,
@@ -207,8 +211,12 @@ pub fn getAtIndex(self: *const List, index: usize) !?*Node {
} }
const NodeList = @import("../collections/NodeList.zig"); const NodeList = @import("../collections/NodeList.zig");
pub fn runtimeGenericWrap(self: *List, page: *Page) !*NodeList { pub fn runtimeGenericWrap(self: *List, _: *const Page) !*NodeList {
return page._factory.create(NodeList{ .data = .{ .selector_list = self } }); const nl = try self._arena.create(NodeList);
nl.* = .{
._data = .{ .selector_list = self },
};
return nl;
} }
const IdAnchor = struct { const IdAnchor = struct {

View File

@@ -62,7 +62,9 @@ pub fn querySelectorAll(root: *Node, input: []const u8, page: *Page) !*List {
return error.SyntaxError; return error.SyntaxError;
} }
const arena = page.arena; const arena = try page.getArena(.{ .debug = "querySelectorAll" });
errdefer page.releaseArena(arena);
var nodes: std.AutoArrayHashMapUnmanaged(*Node, void) = .empty; var nodes: std.AutoArrayHashMapUnmanaged(*Node, void) = .empty;
const selectors = try Parser.parseList(arena, input, page); const selectors = try Parser.parseList(arena, input, page);
@@ -70,10 +72,12 @@ pub fn querySelectorAll(root: *Node, input: []const u8, page: *Page) !*List {
try List.collect(arena, root, selector, &nodes, page); try List.collect(arena, root, selector, &nodes, page);
} }
return page._factory.create(List{ const list = try arena.create(List);
list.* = .{
._arena = arena, ._arena = arena,
._nodes = nodes.keys(), ._nodes = nodes.keys(),
}); };
return list;
} }
pub fn matches(el: *Node.Element, input: []const u8, page: *Page) !bool { pub fn matches(el: *Node.Element, input: []const u8, page: *Page) !bool {

View File

@@ -400,20 +400,32 @@ test "cdp Node: search list" {
defer page._session.removePage(); defer page._session.removePage();
var doc = page.window._document; var doc = page.window._document;
const s1 = try search_list.create((try doc.querySelectorAll(.wrap("a"), page))._nodes); {
const l1 = try doc.querySelectorAll(.wrap("a"), page);
defer l1.deinit(page);
const s1 = try search_list.create(l1._nodes);
try testing.expectEqual("1", s1.name); try testing.expectEqual("1", s1.name);
try testing.expectEqualSlices(u32, &.{ 1, 2 }, s1.node_ids); try testing.expectEqualSlices(u32, &.{ 1, 2 }, s1.node_ids);
}
try testing.expectEqual(2, registry.lookup_by_id.count()); try testing.expectEqual(2, registry.lookup_by_id.count());
try testing.expectEqual(2, registry.lookup_by_node.count()); try testing.expectEqual(2, registry.lookup_by_node.count());
const s2 = try search_list.create((try doc.querySelectorAll(.wrap("#a1"), page))._nodes); {
const l2 = try doc.querySelectorAll(.wrap("#a1"), page);
defer l2.deinit(page);
const s2 = try search_list.create(l2._nodes);
try testing.expectEqual("2", s2.name); try testing.expectEqual("2", s2.name);
try testing.expectEqualSlices(u32, &.{1}, s2.node_ids); try testing.expectEqualSlices(u32, &.{1}, s2.node_ids);
}
const s3 = try search_list.create((try doc.querySelectorAll(.wrap("#a2"), page))._nodes); {
const l3 = try doc.querySelectorAll(.wrap("#a2"), page);
defer l3.deinit(page);
const s3 = try search_list.create(l3._nodes);
try testing.expectEqual("3", s3.name); try testing.expectEqual("3", s3.name);
try testing.expectEqualSlices(u32, &.{2}, s3.node_ids); try testing.expectEqualSlices(u32, &.{2}, s3.node_ids);
}
try testing.expectEqual(2, registry.lookup_by_id.count()); try testing.expectEqual(2, registry.lookup_by_id.count());
try testing.expectEqual(2, registry.lookup_by_node.count()); try testing.expectEqual(2, registry.lookup_by_node.count());

View File

@@ -98,6 +98,8 @@ fn performSearch(cmd: anytype) !void {
const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
const page = bc.session.currentPage() orelse return error.PageNotLoaded; const page = bc.session.currentPage() orelse return error.PageNotLoaded;
const list = try Selector.querySelectorAll(page.window._document.asNode(), params.query, page); const list = try Selector.querySelectorAll(page.window._document.asNode(), params.query, page);
defer list.deinit(page);
const search = try bc.node_search_list.create(list._nodes); const search = try bc.node_search_list.create(list._nodes);
// dispatch setChildNodesEvents to inform the client of the subpart of node // dispatch setChildNodesEvents to inform the client of the subpart of node
@@ -247,6 +249,8 @@ fn querySelectorAll(cmd: anytype) !void {
}; };
const selected_nodes = try Selector.querySelectorAll(node.dom, params.selector, page); const selected_nodes = try Selector.querySelectorAll(node.dom, params.selector, page);
defer selected_nodes.deinit(page);
const nodes = selected_nodes._nodes; const nodes = selected_nodes._nodes;
const node_ids = try cmd.arena.alloc(Node.Id, nodes.len); const node_ids = try cmd.arena.alloc(Node.Id, nodes.len);