From 3bf98a33fdb93ba1aa657dad31d07930f1dde6f0 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 28 Jul 2026 10:39:49 +0800 Subject: [PATCH] mem: optimize node child list Reworks how a Node's child list is modeled. This used to be a optional pointer to the child list, along with a Node representing this node's linkedlist node in the parent's child list. The new model is the same 24 bytes, but it's fully embedded: nodes with children don't require another 16 byte allocation. Generally speaking, this is more linked-list-ish, so it does simplify the code. `linkToNode` is removed BUT, the first child's `_prev` points to the last child, which is a bit unusual but necessary so that `lastChild` remains O(1). --- src/browser/Frame.zig | 21 +--- src/browser/frame/parse.zig | 9 +- src/browser/webapi/DOMNodeIterator.zig | 4 +- src/browser/webapi/Document.zig | 5 +- src/browser/webapi/Node.zig | 108 +++++++++++++----- src/browser/webapi/collections/ChildNodes.zig | 20 ++-- 6 files changed, 103 insertions(+), 64 deletions(-) diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index cdb864359..5d40c591e 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -2458,20 +2458,13 @@ pub fn removeNode(self: *Frame, parent: *Node, child: *Node, opts: RemoveNodeOpt else null; - const children = parent._children.?; - children.remove(&child._child_link); - if (children.first == null) { - // last child removed; drop the list so a childless node holds no allocation - parent._children = null; - self._factory.destroy(children); - } + parent.unlink(child); // grab this before we null the parent const was_connected = child.isConnected(); // Capture the ID map before disconnecting, so we can remove IDs from the correct document const id_maps = if (was_connected) self.getElementIdMap(child) else null; child._parent = null; - child._child_link = .{}; // Update live ranges for removal (DOM spec remove steps 4-7) if (child_index_for_ranges) |idx| { @@ -2631,23 +2624,17 @@ pub fn _insertNodeRelative(self: *Frame, comptime from_parser: bool, parent: *No lp.assert(child._parent == null, "Frame.insertNodeRelative parent", .{}); - const children = parent._children orelse blk: { - const list = try self._factory.create(std.DoublyLinkedList{}); - parent._children = list; - break :blk list; - }; - switch (relative) { - .append => children.append(&child._child_link), + .append => parent.linkLast(child), .after => |ref_node| { // caller should have made sure this was the case lp.assert(ref_node._parent.? == parent, "Frame.insertNodeRelative after", .{ .url = self.url }); - children.insertAfter(&ref_node._child_link, &child._child_link); + parent.linkAfter(ref_node, child); }, .before => |ref_node| { // caller should have made sure this was the case lp.assert(ref_node._parent.? == parent, "Frame.insertNodeRelative before", .{ .url = self.url }); - children.insertBefore(&ref_node._child_link, &child._child_link); + parent.linkBefore(ref_node, child); }, } child._parent = parent; diff --git a/src/browser/frame/parse.zig b/src/browser/frame/parse.zig index 526fc872d..aa690755c 100644 --- a/src/browser/frame/parse.zig +++ b/src/browser/frame/parse.zig @@ -80,12 +80,15 @@ fn htmlAsChildrenInner(frame: *Frame, node: *Node, html: []const u8, opts: Fragm // Because of custom element callbacks, the structure might not be what // we expect, and nodes might be altogether removed. We deal with this in a // few different places, but always the same way: leave it as-is. - const children = node._children orelse return; - const first = Node.linkToNode(children.first.?); + const first = node.firstChild() orelse return; if (first.is(Element.Html.Html) == null) { return; } - node._children = first._children; + node._first_child = first._first_child; + first._first_child = null; + first._parent = null; + first._prev = null; + first._next = null; // No mutation records for the unwrapped children either; see the comment // about fragment parses in _insertNodeRelative. diff --git a/src/browser/webapi/DOMNodeIterator.zig b/src/browser/webapi/DOMNodeIterator.zig index b2336bb31..1686e054e 100644 --- a/src/browser/webapi/DOMNodeIterator.zig +++ b/src/browser/webapi/DOMNodeIterator.zig @@ -215,8 +215,8 @@ fn filterNode(self: *const DOMNodeIterator, node: *Node, frame: *Frame) !i32 { fn getNextInTree(self: *const DOMNodeIterator, node: *Node) ?*Node { // Depth-first traversal within the root subtree - if (node._children) |children| { - return Node.linkToNode(children.first.?); + if (node.firstChild()) |first| { + return first; } var current = node; diff --git a/src/browser/webapi/Document.zig b/src/browser/webapi/Document.zig index 6a4712408..aaef3fce4 100644 --- a/src/browser/webapi/Document.zig +++ b/src/browser/webapi/Document.zig @@ -1061,8 +1061,7 @@ fn writeInternal(self: *Document, text: []const []const u8, append_newline: bool // Extract children from wrapper HTML element (html5ever wraps fragments) // https://github.com/servo/html5ever/issues/583 - const children = fragment_node._children orelse return; - const first = Node.linkToNode(children.first.?); + const first = fragment_node.firstChild() orelse return; // Collect all children to insert (to avoid iterator invalidation) var children_to_insert: std.ArrayList(*Node) = .empty; @@ -1472,7 +1471,7 @@ pub fn injectBlank(self: *Document, frame: *Frame) error{InjectBlankError}!void fn _injectBlank(self: *Document, frame: *Frame) !void { if (comptime IS_DEBUG) { // should only be called on an empty document - std.debug.assert(self.asNode()._children == null); + std.debug.assert(self.asNode()._first_child == null); } const html = try Frame.node_factory.createElementNS(frame, .html, "html", null); diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig index 0bd37dbcc..2519df1a6 100644 --- a/src/browser/webapi/Node.zig +++ b/src/browser/webapi/Node.zig @@ -37,7 +37,6 @@ pub const ShadowRoot = @import("ShadowRoot.zig"); const String = lp.String; const Allocator = std.mem.Allocator; -const LinkedList = std.DoublyLinkedList; pub const AssignedSlotLookup = std.AutoHashMapUnmanaged(*Node, *Element.Html.Slot); @@ -46,10 +45,11 @@ const Node = @This(); _type: Type, _proto: *EventTarget, _parent: ?*Node = null, -// A node with no children leaves this null (no allocation). Otherwise it -// points to a heap-allocated intrusive list of the node's `_child_link`s. -_children: ?*LinkedList = null, -_child_link: LinkedList.Node = .{}, +// The first child's `_prev` points at the LAST child (keeping lastChild +// O(1)); the last child's `_next` is null. A detached node has null links. +_first_child: ?*Node = null, +_next: ?*Node = null, +_prev: ?*Node = null, // Lookup for nodes that have a different owner document than frame.document pub const OwnerDocumentLookup = std.AutoHashMapUnmanaged(*Node, *Document); @@ -178,21 +178,25 @@ fn adjacentParent(self: *Node, variant: AdjacentVariant) !*Node { } pub fn firstChild(self: *const Node) ?*Node { - const children = self._children orelse return null; - return linkToNodeOrNull(children.first); + return self._first_child; } pub fn lastChild(self: *const Node) ?*Node { - const children = self._children orelse return null; - return linkToNodeOrNull(children.last); + const first = self._first_child orelse return null; + return first._prev; } pub fn nextSibling(self: *const Node) ?*Node { - return linkToNodeOrNull(self._child_link.next); + return self._next; } pub fn previousSibling(self: *const Node) ?*Node { - return linkToNodeOrNull(self._child_link.prev); + const parent = self._parent orelse return null; + if (parent._first_child == self) { + // our _prev points at the parent's last child + return null; + } + return self._prev; } pub fn parentNode(self: *const Node) ?*Node { @@ -1074,13 +1078,7 @@ pub fn format(self: *Node, writer: *std.Io.Writer) !void { // Returns an iterator the can be used to iterate through the node's children // For internal use. pub fn childrenIterator(self: *Node) NodeIterator { - const children = self._children orelse { - return .{ .node = null }; - }; - - return .{ - .node = linkToNodeOrNull(children.first), - }; + return .{ .node = self._first_child }; } pub fn getChildrenCount(self: *Node) usize { @@ -1322,13 +1320,11 @@ pub fn compareDocumentPosition(self: *Node, other: *Node) u16 { return DISCONNECTED; } -// faster to compare the linked list node links directly fn isNodeBefore(node1: *const Node, node2: *const Node) bool { - var current = node1._child_link.next; - const target = &node2._child_link; - while (current) |link| { - if (link == target) return true; - current = link.next; + var current = node1._next; + while (current) |n| { + if (n == node2) return true; + current = n._next; } return false; } @@ -1587,18 +1583,70 @@ const NodeIterator = struct { node: ?*Node, pub fn next(self: *NodeIterator) ?*Node { const node = self.node orelse return null; - self.node = linkToNodeOrNull(node._child_link.next); + self.node = node._next; return node; } }; -// Turns a linked list node into a Node -pub fn linkToNode(n: *LinkedList.Node) *Node { - return @fieldParentPtr("_child_link", n); +// Low-level sibling-list splicing, used by Frame's insert/remove. These only +// maintain the link invariants (first child's `_prev` is the last child, last +// child's `_next` is null); the caller handles `_parent` and all DOM +// bookkeeping. +pub fn linkLast(self: *Node, child: *Node) void { + child._next = null; + if (self._first_child) |first| { + const last = first._prev.?; + last._next = child; + child._prev = last; + first._prev = child; + } else { + self._first_child = child; + child._prev = child; + } } -pub fn linkToNodeOrNull(n_: ?*LinkedList.Node) ?*Node { - return if (n_) |n| linkToNode(n) else null; +pub fn linkBefore(self: *Node, ref: *Node, child: *Node) void { + child._next = ref; + child._prev = ref._prev; + if (self._first_child == ref) { + self._first_child = child; + } else { + ref._prev.?._next = child; + } + ref._prev = child; +} + +pub fn linkAfter(self: *Node, ref: *Node, child: *Node) void { + child._prev = ref; + child._next = ref._next; + if (ref._next) |next| { + next._prev = child; + } else { + // ref was the last child + self._first_child.?._prev = child; + } + ref._next = child; +} + +pub fn unlink(self: *Node, child: *Node) void { + const first = self._first_child.?; + if (child == first) { + if (child._next) |next| { + next._prev = child._prev; + self._first_child = next; + } else { + self._first_child = null; + } + } else { + child._prev.?._next = child._next; + if (child._next) |next| { + next._prev = child._prev; + } else { + first._prev = child._prev; + } + } + child._next = null; + child._prev = null; } pub const JsApi = struct { diff --git a/src/browser/webapi/collections/ChildNodes.zig b/src/browser/webapi/collections/ChildNodes.zig index 08e4096ae..176497593 100644 --- a/src/browser/webapi/collections/ChildNodes.zig +++ b/src/browser/webapi/collections/ChildNodes.zig @@ -32,7 +32,7 @@ const ChildNodes = @This(); _arena: std.mem.Allocator, _last_index: usize, _last_length: ?u32, -_last_node: ?*std.DoublyLinkedList.Node, +_last_node: ?*Node, _cached_version: usize, _node: *Node, @@ -66,10 +66,12 @@ pub fn length(self: *ChildNodes, frame: *const Frame) !u32 { return cached_length; } } - const children = self._node._children orelse return 0; - // O(N) - const len: u32 = @intCast(children.len()); + var len: u32 = 0; + var it = self._node.childrenIterator(); + while (it.next()) |_| { + len += 1; + } self._last_length = len; return len; } @@ -78,7 +80,7 @@ pub fn getAtIndex(self: *ChildNodes, index: usize, frame: *const Frame) !?*Node _ = self.versionCheck(frame); var current = self._last_index; - var node: ?*std.DoublyLinkedList.Node = null; + var node: ?*Node = null; if (index < current or self._last_node == null) { current = 0; node = self.first() orelse return null; @@ -90,17 +92,17 @@ pub fn getAtIndex(self: *ChildNodes, index: usize, frame: *const Frame) !?*Node while (node) |n| { if (index == current) { self._last_node = n; - return Node.linkToNode(n); + return n; } current += 1; - node = n.next; + node = n.nextSibling(); } self._last_node = null; return null; } -pub fn first(self: *const ChildNodes) ?*std.DoublyLinkedList.Node { - return (self._node._children orelse return null).first; +pub fn first(self: *const ChildNodes) ?*Node { + return self._node.firstChild(); } pub fn keys(self: *ChildNodes, frame: *Frame) !*KeyIterator {