From f32606a27be29274b085b54f1049bf60c90623f9 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 8 Jul 2026 18:09:29 +0800 Subject: [PATCH 1/2] mem: Store Element's attributes as an array, not a linked list An element's attributes are currently stored as a ?*Attribute.List which is an intrusive doubly linkedlist of Attribute.Entry. This has two small benefits: 1 - Elements with no attributes only grow by 8 bytes 2 - Attribute list mutation (additions/deletion) are linkedlist cheap This commit embeds an ArrayList-like structure directly in Element. The impact being: 1 - Elements with no attributes now grow by 16 bytes (+8) 2 - Elements with 1+ attribute shrink from 32 -> 16 bytes (-16) 3 - Mutations are more expensive 4 - Fewer indirections (and better cache locality) 5 - Much fewer allocations (4 attributes go from 5 allocations to 1) 6 - Every Attribute shrinks by 16 bytes (no need for next/prev link) While looking at a few popular sites (amazon product, redit post, ...), the majority of elements have 1+ element and most attributes are never accessed in JS and, when they are, reads are more frequent than writes (in fact, even internally to support other WebAPIs, reads far outweigh writes). There's virtually no real world site where this shouldn't reduce memory usage by hundreds of KB and also improve performance (in a way that isn't significant to the overall page loading though). --- src/SemanticTree.zig | 9 +- src/browser/Frame.zig | 9 +- src/browser/frame/node_factory.zig | 51 ++-- src/browser/parser/Parser.zig | 2 +- src/browser/webapi/CustomElementRegistry.zig | 7 +- src/browser/webapi/Element.zig | 114 +++----- src/browser/webapi/element/Attribute.zig | 287 ++++++++++--------- src/browser/webapi/element/html/Audio.zig | 5 +- src/browser/xpath/Evaluator.zig | 13 +- src/cdp/Node.zig | 7 +- src/string.zig | 73 +++++ 11 files changed, 309 insertions(+), 268 deletions(-) diff --git a/src/SemanticTree.zig b/src/SemanticTree.zig index 7e815acf2..ac556be00 100644 --- a/src/SemanticTree.zig +++ b/src/SemanticTree.zig @@ -380,13 +380,12 @@ const JsonVisitor = struct { try self.jw.write(value); } - if (el._attributes) |attrs| { + if (!el._attributes.isEmpty()) { try self.jw.objectField("attributes"); try self.jw.beginObject(); - var iter = attrs.iterator(); - while (iter.next()) |attr| { - try self.jw.objectField(attr._name.str()); - try self.jw.write(attr._value.str()); + for (el.attributeEntries()) |*attr| { + try self.jw.objectField(attr._name); + try self.jw.write(attr._value); } try self.jw.endObject(); } diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index aee5bcfe7..7538fab6c 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -107,10 +107,11 @@ _fragment_scripts_runnable: bool = false, // See Attribute.List for what this is. TL;DR: proper DOM Attribute Nodes are // fat yet rarely needed. We only create them on-demand, but still need proper // identity (a given attribute should return the same *Attribute), so we do -// a look here. We don't store this in the Element or Attribute.List.Entry -// because that would require additional space per element / Attribute.List.Entry -// even though we'll create very few (if any) actual *Attributes. -_attribute_lookup: std.AutoHashMapUnmanaged(usize, *Element.Attribute) = .empty, +// a look here, keyed by (list, name). We don't store this in the Element or +// Attribute.List.Entry because that would require additional space per +// element / Attribute.List.Entry even though we'll create very few (if any) +// actual *Attributes. +_attribute_lookup: Element.Attribute.List.Lookup = .empty, // Same as _atlribute_lookup, but instead of individual attributes, this is for // the return of elements.attributes. diff --git a/src/browser/frame/node_factory.zig b/src/browser/frame/node_factory.zig index 5665efcff..56be67650 100644 --- a/src/browser/frame/node_factory.zig +++ b/src/browser/frame/node_factory.zig @@ -862,18 +862,15 @@ pub fn createElementNS(frame: *Frame, namespace: Element.Namespace, name: []cons // After constructor runs, invoke attributeChangedCallback for initial attributes const element = node.as(Element); - if (element._attributes) |attributes| { - var it = attributes.iterator(); - while (it.next()) |attr| { - Element.Html.Custom.enqueueAttributeChangedCallbackOnElement( - element, - attr._name, - null, // old_value is null for initial attributes - attr._value, - null, - frame, - ); - } + for (element.attributeEntries()) |*attr| { + Element.Html.Custom.enqueueAttributeChangedCallbackOnElement( + element, + .wrap(attr._name), + null, // old_value is null for initial attributes + .wrap(attr._value), + null, + frame, + ); } return node; @@ -907,6 +904,7 @@ fn createHtmlElementT(frame: *Frame, comptime E: type, namespace: Element.Namesp const html_element_ptr = try frame._factory.htmlElement(html_element); const element = html_element_ptr.asElement(); element._namespace = namespace; + element._attributes.normalize = namespace == .html; try populateElementAttributes(frame, element, attribute_iterator); // Check for customized built-in element via "is" attribute @@ -934,34 +932,27 @@ fn createSvgElementT(frame: *Frame, comptime E: type, tag_name: []const u8, attr const svg_element_ptr = try frame._factory.svgElement(tag_name, svg_element); var element = svg_element_ptr.asElement(); element._namespace = .svg; + element._attributes.normalize = false; try populateElementAttributes(frame, element, attribute_iterator); return element.asNode(); } fn populateElementAttributes(frame: *Frame, element: *Element, list: anytype) !void { - if (@TypeOf(list) == ?*Element.Attribute.List) { + if (@TypeOf(list) == *Element.Attribute.List or @TypeOf(list) == *const Element.Attribute.List) { // from cloneNode - - var existing = list orelse return; - - var attributes = try frame.arena.create(Element.Attribute.List); - attributes.* = .{ - .normalize = existing.normalize, - }; - - var it = existing.iterator(); - while (it.next()) |attr| { - try attributes.putNew(attr._name.str(), attr._value.str(), frame); - } - element._attributes = attributes; - return; + return element._attributes.cloneFrom(list, frame); } // from the parser - if (@TypeOf(list) == @TypeOf(null) or list.count() == 0) { + if (@TypeOf(list) == @TypeOf(null)) { return; } - var attributes = try element.createAttributeList(frame); + const count = list.count(); + if (count == 0) { + return; + } + var attributes = &element._attributes; + try attributes.ensureTotalCapacity(count, frame); while (list.next()) |attr| { try attributes.putNew(attr.name.local.slice(), attr.value.slice(), frame); } @@ -988,7 +979,7 @@ pub fn constructCustomElement(frame: *Frame, new_target: JS.Function) !*Element } const tag_name = try String.init(frame.arena, definition.name, .{}); - const node = try createHtmlElementT(frame, Element.Html.Custom, .html, @as(?*Element.Attribute.List, null), .{ + const node = try createHtmlElementT(frame, Element.Html.Custom, .html, null, .{ ._proto = undefined, ._tag_name = tag_name, ._definition = definition, diff --git a/src/browser/parser/Parser.zig b/src/browser/parser/Parser.zig index c3fc4b6c0..700dc5377 100644 --- a/src/browser/parser/Parser.zig +++ b/src/browser/parser/Parser.zig @@ -539,7 +539,7 @@ fn _addAttrsIfMissingCallback(self: *Parser, node: *Node, attributes: h5e.Attrib const element = node.as(Element); const frame = self.frame; - const attr_list = try element.getOrCreateAttributeList(frame); + const attr_list = &element._attributes; while (attributes.next()) |attr| { const name = attr.name.local.slice(); const value = attr.value.slice(); diff --git a/src/browser/webapi/CustomElementRegistry.zig b/src/browser/webapi/CustomElementRegistry.zig index ccab7ee30..4a8e204c7 100644 --- a/src/browser/webapi/CustomElementRegistry.zig +++ b/src/browser/webapi/CustomElementRegistry.zig @@ -213,11 +213,10 @@ pub fn upgradeCustomElement(custom: *Custom, definition: *CustomElementDefinitio // Enqueue attributeChangedCallback for existing observed attributes const element = custom.asElement(); - var attr_it = element.attributeIterator(); - while (attr_it.next()) |attr| { - const name = attr._name; + for (element.attributeEntries()) |*attr| { + const name = lp.String.wrap(attr._name); if (definition.isAttributeObserved(name)) { - Custom.enqueueAttributeChangedCallbackOnElement(element, name, null, attr._value, null, frame); + Custom.enqueueAttributeChangedCallbackOnElement(element, name, null, .wrap(attr._value), null, frame); } } diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index cc4e54979..806840308 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -104,7 +104,7 @@ pub const Namespace = enum(u8) { _type: Type, _proto: *Node, _namespace: Namespace = .html, -_attributes: ?*Attribute.List = null, +_attributes: Attribute.List = .{}, pub const Type = union(enum) { html: *Html, @@ -150,15 +150,6 @@ pub fn asConstNode(self: *const Element) *const Node { return self._proto; } -pub fn attributesEql(self: *const Element, other: *Element) bool { - if (self._attributes) |attr_list| { - const other_list = other._attributes orelse return false; - return attr_list.eql(other_list); - } - // Make sure no attrs in both sides. - return other._attributes == null; -} - /// TODO: localName and prefix comparison. pub fn isEqualNode(self: *Element, other: *Element) bool { const self_tag = self.getTagNameDump(); @@ -169,8 +160,7 @@ pub fn isEqualNode(self: *Element, other: *Element) bool { return false; } - // Compare attributes. - if (!self.attributesEql(other)) { + if (self._attributes.eql(&other._attributes) == false) { return false; } @@ -382,22 +372,19 @@ pub fn lookupNamespaceURIForElement(self: *Element, prefix: ?[]const u8, frame: } // Step 2: search xmlns attributes - if (self._attributes) |attrs| { - var iter = attrs.iterator(); - while (iter.next()) |entry| { - if (prefix == null) { - if (entry._name.eql(comptime .wrap("xmlns"))) { - const val = entry._value.str(); + for (self._attributes.entries()) |*entry| { + if (prefix == null) { + if (std.mem.eql(u8, entry._name, "xmlns")) { + const val = entry._value; + return if (val.len == 0) null else val; + } + } else { + const name = entry._name; + if (std.mem.startsWith(u8, name, "xmlns:")) { + if (std.mem.eql(u8, name["xmlns:".len..], prefix.?)) { + const val = entry._value; return if (val.len == 0) null else val; } - } else { - const name = entry._name.str(); - if (std.mem.startsWith(u8, name, "xmlns:")) { - if (std.mem.eql(u8, name["xmlns:".len..], prefix.?)) { - const val = entry._value.str(); - return if (val.len == 0) null else val; - } - } } } } @@ -420,13 +407,10 @@ pub fn lookupPrefixForElement(self: *Element, namespace: []const u8, frame: *Fra } // Step 2: search xmlns: attribute declarations for one whose value is the namespace - if (self._attributes) |attrs| { - var iter = attrs.iterator(); - while (iter.next()) |entry| { - const name = entry._name.str(); - if (std.mem.startsWith(u8, name, "xmlns:") and std.mem.eql(u8, entry._value.str(), namespace)) { - return name["xmlns:".len..]; - } + for (self._attributes.entries()) |*entry| { + const name = entry._name; + if (std.mem.startsWith(u8, name, "xmlns:") and std.mem.eql(u8, entry._value, namespace)) { + return name["xmlns:".len..]; } } @@ -565,14 +549,13 @@ pub fn setClassName(self: *Element, value: []const u8, frame: *Frame) !void { return self.setAttributeSafe(comptime .wrap("class"), .wrap(value), frame); } -pub fn attributeIterator(self: *Element) Attribute.InnerIterator { - const attributes = self._attributes orelse return .{}; - return attributes.iterator(); +/// DANGER: Invalidated by mutation of the attribute list. +pub fn attributeEntries(self: *const Element) []const Attribute.List.Entry { + return self._attributes.entries(); } pub fn getAttribute(self: *const Element, name: String, frame: *Frame) !?String { - const attributes = self._attributes orelse return null; - return attributes.get(name, frame); + return self._attributes.get(name, frame); } /// For simplicity, the namespace is currently ignored and only the local name is used. @@ -592,19 +575,16 @@ pub fn getAttributeNS( } pub fn getAttributeSafe(self: *const Element, name: String) ?[]const u8 { - const attributes = self._attributes orelse return null; - return attributes.getSafe(name); + return self._attributes.getSafe(name); } pub fn hasAttribute(self: *const Element, name: String, frame: *Frame) !bool { - const attributes = self._attributes orelse return false; - const value = try attributes.get(name, frame); + const value = try self._attributes.get(name, frame); return value != null; } pub fn hasAttributeSafe(self: *const Element, name: String) bool { - const attributes = self._attributes orelse return false; - return attributes.hasSafe(name); + return self._attributes.hasSafe(name); } // Per HTML "concept-fe-disabled", only listed elements participate in the @@ -665,19 +645,16 @@ pub fn isDisabled(self: *Element) bool { } pub fn hasAttributes(self: *const Element) bool { - const attributes = self._attributes orelse return false; - return attributes.isEmpty() == false; + return self._attributes.isEmpty() == false; } pub fn getAttributeNode(self: *Element, name: String, frame: *Frame) !?*Attribute { - const attributes = self._attributes orelse return null; - return attributes.getAttribute(name, self, frame); + return self._attributes.getAttribute(name, self, frame); } pub fn setAttribute(self: *Element, name: String, value: String, frame: *Frame) !void { try Attribute.validateAttributeName(name); - const attributes = try self.getOrCreateAttributeList(frame); - _ = try attributes.put(name, value, self, frame); + _ = try self._attributes.put(name, value, self, frame); } pub fn setAttributeNS( @@ -710,20 +687,7 @@ pub fn setAttributeNS( } pub fn setAttributeSafe(self: *Element, name: String, value: String, frame: *Frame) !void { - const attributes = try self.getOrCreateAttributeList(frame); - _ = try attributes.putSafe(name, value, self, frame); -} - -pub fn getOrCreateAttributeList(self: *Element, frame: *Frame) !*Attribute.List { - return self._attributes orelse return self.createAttributeList(frame); -} - -pub fn createAttributeList(self: *Element, frame: *Frame) !*Attribute.List { - lp.assert(self._attributes == null, "Element.createAttributeList non-null _attributes", .{}); - const a = try frame.arena.create(Attribute.List); - a.* = .{ .normalize = self._namespace == .html }; - self._attributes = a; - return a; + _ = try self._attributes.putSafe(name, value, self, frame); } pub fn getShadowRoot(self: *Element, frame: *Frame) ?*ShadowRoot { @@ -811,13 +775,11 @@ pub fn setAttributeNode(self: *Element, attr: *Attribute, frame: *Frame) !?*Attr _ = try el.removeAttributeNode(attr, frame); } - const attributes = try self.getOrCreateAttributeList(frame); - return attributes.putAttribute(attr, self, frame); + return self._attributes.putAttribute(attr, self, frame); } pub fn removeAttribute(self: *Element, name: String, frame: *Frame) !void { - const attributes = self._attributes orelse return; - return attributes.delete(name, self, frame); + return self._attributes.delete(name, self, frame); } pub fn toggleAttribute(self: *Element, name: String, force: ?bool, frame: *Frame) !bool { @@ -847,16 +809,13 @@ pub fn removeAttributeNode(self: *Element, attr: *Attribute, frame: *Frame) !*At } pub fn getAttributeNames(self: *const Element, frame: *Frame) ![][]const u8 { - const attributes = self._attributes orelse return &.{}; - return attributes.getNames(frame); + return self._attributes.getNames(frame.local_arena); } pub fn getAttributeNamedNodeMap(self: *Element, frame: *Frame) !*Attribute.NamedNodeMap { const gop = try frame._attribute_named_node_map_lookup.getOrPut(frame.arena, @intFromPtr(self)); if (!gop.found_existing) { - const attributes = try self.getOrCreateAttributeList(frame); - const named_node_map = try frame._factory.create(Attribute.NamedNodeMap{ ._list = attributes, ._element = self }); - gop.value_ptr.* = named_node_map; + gop.value_ptr.* = try frame._factory.create(Attribute.NamedNodeMap{ ._element = self }); } return gop.value_ptr.*; } @@ -1506,7 +1465,7 @@ pub fn getElementsByClassName(self: *Element, class_name: []const u8, frame: *Fr pub fn clone(self: *Element, deep: bool, frame: *Frame) !*Node { const tag_name = self.getTagNameDump(); - const node = try Frame.node_factory.createElementNS(frame, self._namespace, tag_name, self._attributes); + const node = try Frame.node_factory.createElementNS(frame, self._namespace, tag_name, &self._attributes); // Allow element-specific types to copy their runtime state _ = Element.Build.call(node.as(Element), "cloned", .{ self, node.as(Element), deep, frame }) catch |err| { @@ -1623,11 +1582,8 @@ pub fn format(self: *Element, writer: *std.Io.Writer) !void { try writer.writeByte('<'); try writer.writeAll(self.getTagNameDump()); - if (self._attributes) |attributes| { - var it = attributes.iterator(); - while (it.next()) |attr| { - try writer.print(" {f}", .{attr}); - } + for (self._attributes.entries()) |*attr| { + try writer.print(" {f}", .{attr}); } try writer.writeByte('>'); } diff --git a/src/browser/webapi/element/Attribute.zig b/src/browser/webapi/element/Attribute.zig index a1fc9683e..cae57785a 100644 --- a/src/browser/webapi/element/Attribute.zig +++ b/src/browser/webapi/element/Attribute.zig @@ -47,7 +47,7 @@ _value: String, _element: ?*Element, pub fn format(self: *const Attribute, writer: *std.Io.Writer) !void { - return formatAttribute(self._name, self._value, writer); + return formatAttribute(self._name.str(), self._value.str(), writer); } pub fn getName(self: *const Attribute) String { @@ -128,41 +128,62 @@ pub const JsApi = struct { // in our Entry? Because that would require an extra 8 bytes for every single // attribute in the DOM, and, again, we expect that to almost always be null. pub const List = struct { - normalize: bool, - /// Length of items in `_list`. Not usize to increase memory usage. - /// Honestly, this is more than enough. - _len: u32 = 0, - _list: std.DoublyLinkedList = .{}, + normalize: bool = true, + _len: u16 = 0, + _cap: u16 = 0, + _entries: [*]Entry = @constCast(&empty_entries), + + const empty_entries: [0]Entry = .{}; + + pub const Lookup = std.HashMapUnmanaged(LookupKey, *Attribute, LookupContext, std.hash_map.default_max_load_percentage); + + // for Frame._attribute_lookup which is our identity map for attributes + pub const LookupKey = struct { + list: *const List, + // Owned by the frame arena (an entry's name) + name: []const u8, + }; + + const LookupContext = struct { + pub fn hash(_: LookupContext, key: LookupKey) u64 { + var h = std.hash.Wyhash.init(@intFromPtr(key.list)); + h.update(key.name); + return h.final(); + } + pub fn eql(_: LookupContext, a: LookupKey, b: LookupKey) bool { + return a.list == b.list and std.mem.eql(u8, a.name, b.name); + } + }; + + pub fn entries(self: *const List) []const Entry { + return self._entries[0..self._len]; + } pub fn isEmpty(self: *const List) bool { - return self._list.first == null; + return self._len == 0; + } + + pub fn length(self: *const List) usize { + return self._len; } pub fn get(self: *const List, name: String, frame: *Frame) !?String { const entry = (try self.getEntry(name, frame)) orelse return null; - return entry._value; + return .wrap(entry._value); } - pub inline fn length(self: *const List) usize { - return self._len; - } - - /// Compares 2 attribute lists for equality. - pub fn eql(self: *List, other: *List) bool { - if (self.length() != other.length()) { + pub fn eql(self: *const List, other: *const List) bool { + if (self._len != other._len) { return false; } - var iter = self.iterator(); - search: while (iter.next()) |attr| { - // Iterate over all `other` attributes. - var other_iter = other.iterator(); - while (other_iter.next()) |other_attr| { + search: for (self.entries()) |*attr| { + for (other.entries()) |*other_attr| { if (attr.eql(other_attr)) { continue :search; // Found match. } } - // Iterated over all `other` and not match. + // Iterated over all `other` and no match. return false; } return true; @@ -171,7 +192,7 @@ pub const List = struct { // meant for internal usage, where the name is known to be properly cased pub fn getSafe(self: *const List, name: String) ?[]const u8 { const entry = self.getEntryWithNormalizedName(name) orelse return null; - return entry._value.str(); + return entry._value; } // meant for internal usage, where the name is known to be properly cased @@ -181,13 +202,17 @@ pub const List = struct { pub fn getAttribute(self: *const List, name: String, element: ?*Element, frame: *Frame) !?*Attribute { const entry = (try self.getEntry(name, frame)) orelse return null; - const gop = try frame._attribute_lookup.getOrPut(frame.arena, @intFromPtr(entry)); - if (gop.found_existing) { - return gop.value_ptr.*; + return self.getOrCreateAttribute(entry, element, frame); + } + + // Identity map access: a given (list, name) always yields the same + // *Attribute until the attribute is removed. + pub fn getOrCreateAttribute(self: *const List, entry: *const Entry, element: ?*Element, frame: *Frame) !*Attribute { + const gop = try frame._attribute_lookup.getOrPut(frame.arena, .{ .list = self, .name = entry._name }); + if (!gop.found_existing) { + gop.value_ptr.* = try entry.toAttribute(element, frame); } - const attribute = try entry.toAttribute(element, frame); - gop.value_ptr.* = attribute; - return attribute; + return gop.value_ptr.*; } pub fn put(self: *List, name: String, value: String, element: *Element, frame: *Frame) !*Entry { @@ -200,25 +225,27 @@ pub const List = struct { return self._put(.{ .entry = entry, .normalized = name }, value, element, frame); } + // The returned *Entry is only valid until the next mutation of the list. fn _put(self: *List, result: NormalizeAndEntry, value: String, element: *Element, frame: *Frame) !*Entry { const is_id = shouldAddToIdMap(result.normalized, element); var entry: *Entry = undefined; var old_value: ?String = null; if (result.entry) |e| { - old_value = try e._value.dupe(frame.call_arena); + // the old bytes are arena-owned or static; they outlive this update + old_value = String.wrap(e._value); if (is_id) { - frame.removeElementId(element, e._value.str()); + frame.removeElementId(element, e._value); } - e._value = try value.dupe(frame.arena); + e._value = try frame.dupeString(value.str()); entry = e; } else { - entry = try frame._factory.create(Entry{ - ._node = .{}, - ._name = try result.normalized.dupe(frame.arena), - ._value = try value.dupe(frame.arena), - }); - self._list.append(&entry._node); + try self.ensureUnusedCapacity(1, frame); + entry = &self._entries[self._len]; + entry.* = .{ + ._name = try frame.dupeString(result.normalized.str()), + ._value = try frame.dupeString(value.str()), + }; self._len += 1; } @@ -226,23 +253,26 @@ pub const List = struct { const parent = element.asNode()._parent orelse { return entry; }; - try frame.addElementId(parent, element, entry._value.str()); + try frame.addElementId(parent, element, entry._value); } frame.domChanged(); - frame.attributeChange(element, result.normalized, entry._value, old_value); + frame.attributeChange(element, result.normalized, .wrap(entry._value), old_value); return entry; } - // Optimized for cloning. We know `name` is already normalized. We know there isn't duplicates. - // We know the Element is detached (and thus, don't need to check for `id`). - pub fn putForCloned(self: *List, name: []const u8, value: []const u8, frame: *Frame) !void { - const entry = try frame._factory.create(Entry{ - ._node = .{}, - ._name = try String.init(frame.arena, name, .{}), - ._value = try String.init(frame.arena, value, .{}), - }); - self._list.append(&entry._node); - self._len += 1; + // Optimized for cloning. We know the names are already normalized and + // unique. We know the Element is detached (and thus, don't need to check + // for `id`). + pub fn cloneFrom(self: *List, other: *const List, frame: *Frame) !void { + try self.ensureTotalCapacity(other._len, frame); + for (other.entries()) |*e| { + const len = self._len; + self._entries[len] = .{ + ._name = try frame.dupeString(e._name), + ._value = try frame.dupeString(e._value), + }; + self._len = len + 1; + } } // not efficient, won't be called often (if ever!) @@ -263,7 +293,7 @@ pub const List = struct { const entry = try self.put(attribute._name, attribute._value, element, frame); attribute._element = element; - try frame._attribute_lookup.put(frame.arena, @intFromPtr(entry), attribute); + try frame._attribute_lookup.put(frame.arena, .{ .list = self, .name = entry._name }, attribute); return existing_attribute; } @@ -274,14 +304,18 @@ pub const List = struct { // the first is kept return; } + const len = self._len; + if (len == std.math.maxInt(u16)) { + // Bad input. Drop rather than fail the parse + return; + } - const entry = try frame._factory.create(Entry{ - ._node = .{}, - ._name = try String.init(frame.arena, name, .{}), - ._value = try String.init(frame.arena, value, .{}), - }); - self._list.append(&entry._node); - self._len += 1; + try self.ensureUnusedCapacity(1, frame); + self._entries[len] = .{ + ._name = try frame.dupeString(name), + ._value = try frame.dupeString(value), + }; + self._len = len + 1; } pub fn delete(self: *List, name: String, element: *Element, frame: *Frame) !void { @@ -292,32 +326,58 @@ pub const List = struct { const old_value = entry._value; if (is_id) { - frame.removeElementId(element, entry._value.str()); + frame.removeElementId(element, entry._value); } // remove this BEFORE triggering anything, incase that re-enters delete // or some other callback. - _ = frame._attribute_lookup.remove(@intFromPtr(entry)); - self._list.remove(&entry._node); + _ = frame._attribute_lookup.remove(.{ .list = self, .name = entry._name }); + const index = (@intFromPtr(entry) - @intFromPtr(self._entries)) / @sizeOf(Entry); + const list_entries = self._entries[0..self._len]; + std.mem.copyForwards(Entry, list_entries[index .. list_entries.len - 1], list_entries[index + 1 ..]); self._len -= 1; frame.domChanged(); - frame.attributeRemove(element, result.normalized, old_value); - frame._factory.destroy(entry); + frame.attributeRemove(element, result.normalized, .wrap(old_value)); } - pub fn getNames(self: *const List, frame: *Frame) ![][]const u8 { + pub fn getNames(self: *const List, allocator: Allocator) ![][]const u8 { var arr: std.ArrayList([]const u8) = .empty; - var node = self._list.first; - while (node) |n| { - try arr.append(frame.call_arena, Entry.fromNode(n)._name.str()); - node = n.next; + try arr.ensureTotalCapacity(allocator, self._len); + for (self.entries()) |*e| { + arr.appendAssumeCapacity(e._name); } return arr.items; } - pub fn iterator(self: *List) InnerIterator { - return .{ ._node = self._list.first }; + pub fn ensureTotalCapacity(self: *List, count: usize, frame: *Frame) !void { + const cap: u16 = @intCast(@min(count, std.math.maxInt(u16))); + return self.setCapacity(cap, frame); + } + + fn ensureUnusedCapacity(self: *List, extra: u16, frame: *Frame) !void { + const needed = @as(u32, self._len) + extra; + if (needed <= self._cap) { + return; + } + if (needed > std.math.maxInt(u16)) { + return error.OutOfMemory; + } + // Lists are sized exactly at creation; dynamic additions are rare and + // small, so grow slowly. + return self.setCapacity(@intCast(@max(needed, @as(u32, self._cap) + 2)), frame); + } + + fn setCapacity(self: *List, new_cap: u16, frame: *Frame) !void { + if (new_cap <= self._cap) { + return; + } + if (self._cap == 0) { + self._entries = (try frame.arena.alloc(Entry, new_cap)).ptr; + } else { + self._entries = (try frame.arena.realloc(self._entries[0..self._cap], new_cap)).ptr; + } + self._cap = new_cap; } fn getEntry(self: *const List, name: String, frame: *Frame) !?*Entry { @@ -342,30 +402,24 @@ pub const List = struct { } fn getEntryWithNormalizedName(self: *const List, name: String) ?*Entry { - var node = self._list.first; - while (node) |n| { - var e = Entry.fromNode(n); - if (e._name.eql(name)) { + const name_str = name.str(); + for (self._entries[0..self._len]) |*e| { + if (std.mem.eql(u8, e._name, name_str)) { return e; } - node = n.next; } return null; } pub const Entry = struct { - _name: String, - _value: String, - _node: std.DoublyLinkedList.Node, - - fn fromNode(n: *std.DoublyLinkedList.Node) *Entry { - return @alignCast(@fieldParentPtr("_node", n)); - } + // Arena-owned or interned static bytes. Stable for the frame's + // lifetime, unlike the entry itself. + _name: []const u8, + _value: []const u8, /// Returns true if 2 entries are equal. - /// This doesn't compare `_node` fields. pub fn eql(self: *const Entry, other: *const Entry) bool { - return self._name.eql(other._name) and self._value.eql(other._value); + return std.mem.eql(u8, self._name, other._name) and std.mem.eql(u8, self._value, other._value); } pub fn format(self: *const Entry, writer: *std.Io.Writer) !void { @@ -376,11 +430,10 @@ pub const List = struct { return frame._factory.node(Attribute{ ._proto = undefined, ._element = element, - // Cannot directly reference self._name.str() and self._value.str() - // This attribute can outlive the list entry (the node can be - // removed from the element's attribute, but still exist in the DOM) - ._name = try self._name.dupe(frame.arena), - ._value = try self._value.dupe(frame.arena), + // The entry's arena-owned bytes outlive the entry itself, so + // the Attribute can wrap them without duping. + ._name = .wrap(self._name), + ._value = .wrap(self._value), }); } }; @@ -433,7 +486,7 @@ fn normalizeNameForLookup(name: String, frame: *Frame) !String { const normalized = if (name.len < frame.buf.len) std.ascii.lowerString(&frame.buf, name.str()) else - try std.ascii.allocLowerString(frame.call_arena, name.str()); + try std.ascii.allocLowerString(frame.local_arena, name.str()); return .wrap(normalized); } @@ -467,50 +520,40 @@ fn needsLowerCasing(name: []const u8) bool { } pub const NamedNodeMap = struct { - _list: *List, - // Whenever the NamedNodeMap creates an Attribute, it needs to provide the - // "ownerElement". + // "ownerElement". The attribute list is the element's. _element: *Element, + fn list(self: *const NamedNodeMap) *List { + return &self._element._attributes; + } + pub fn length(self: *const NamedNodeMap) u32 { - return @intCast(self._list._list.len()); + return self.list()._len; } pub fn getAtIndex(self: *const NamedNodeMap, index: usize, frame: *Frame) !?*Attribute { - var i: usize = 0; - var node = self._list._list.first; - while (node) |n| { - if (i == index) { - var entry = List.Entry.fromNode(n); - const gop = try frame._attribute_lookup.getOrPut(frame.arena, @intFromPtr(entry)); - if (gop.found_existing) { - return gop.value_ptr.*; - } - const attribute = try entry.toAttribute(self._element, frame); - gop.value_ptr.* = attribute; - return attribute; - } - node = n.next; - i += 1; + const l = self.list(); + if (index >= l._len) { + return null; } - return null; + return l.getOrCreateAttribute(&l._entries[index], self._element, frame); } pub fn getByName(self: *const NamedNodeMap, name: String, frame: *Frame) !?*Attribute { - return self._list.getAttribute(name, self._element, frame); + return self.list().getAttribute(name, self._element, frame); } pub fn set(self: *const NamedNodeMap, attribute: *Attribute, frame: *Frame) !?*Attribute { attribute._element = null; // just a requirement of list.putAttribute, it'll re-set it. - return self._list.putAttribute(attribute, self._element, frame); + return self.list().putAttribute(attribute, self._element, frame); } pub fn removeByName(self: *const NamedNodeMap, name: String, frame: *Frame) !?*Attribute { // this 2-step process (get then delete) isn't efficient. But we don't // expect this to be called often, and this lets us keep delete straightforward. const attr = (try self.getByName(name, frame)) orelse return null; - try self._list.delete(name, self._element, frame); + try self.list().delete(name, self._element, frame); return attr; } @@ -559,26 +602,12 @@ pub const NamedNodeMap = struct { }; }; -// Not meant to be exposed. The "public" iterator is a NamedNodeMap, and it's a -// bit awkward. Having this for more straightforward key=>value is useful for -// the few internal places we need to iterate through the attributes (e.g. dump) -pub const InnerIterator = struct { - _node: ?*std.DoublyLinkedList.Node = null, - - pub fn next(self: *InnerIterator) ?*List.Entry { - const node = self._node orelse return null; - self._node = node.next; - return List.Entry.fromNode(node); - } -}; - -fn formatAttribute(name: String, value_: String, writer: *std.Io.Writer) !void { - try writer.writeAll(name.str()); +fn formatAttribute(name: []const u8, value: []const u8, writer: *std.Io.Writer) !void { + try writer.writeAll(name); // Boolean attributes with empty values are serialized without a value - const value = value_.str(); - if (value.len == 0 and boolean_attributes_lookup.has(name.str())) { + if (value.len == 0 and boolean_attributes_lookup.has(name)) { return; } diff --git a/src/browser/webapi/element/html/Audio.zig b/src/browser/webapi/element/html/Audio.zig index 8d6663396..88a41e384 100644 --- a/src/browser/webapi/element/html/Audio.zig +++ b/src/browser/webapi/element/html/Audio.zig @@ -35,12 +35,11 @@ pub fn constructor(maybe_url: ?String, frame: *Frame) !*Media { const node = try Frame.node_factory.createElementNS(frame, .html, "audio", null); const el = node.as(Element); - const list = try el.getOrCreateAttributeList(frame); // Always set to "auto" initially. - _ = try list.putSafe(comptime .wrap("preload"), comptime .wrap("auto"), el, frame); + try el.setAttributeSafe(comptime .wrap("preload"), comptime .wrap("auto"), frame); // Set URL if provided. if (maybe_url) |url| { - _ = try list.putSafe(comptime .wrap("src"), url, el, frame); + try el.setAttributeSafe(comptime .wrap("src"), url, frame); } return node.as(Media); diff --git a/src/browser/xpath/Evaluator.zig b/src/browser/xpath/Evaluator.zig index 5260c9aee..c10af1439 100644 --- a/src/browser/xpath/Evaluator.zig +++ b/src/browser/xpath/Evaluator.zig @@ -529,17 +529,12 @@ fn appendPreceding(self: *Evaluator, start: *Node, out: *std.ArrayList(*Node)) E fn appendAttributes(self: *Evaluator, node: *Node, out: *std.ArrayList(*Node)) Error!void { const el = node.is(Element) orelse return; - var it = el.attributeIterator(); - while (it.next()) |entry| { - // Memoize via frame._attribute_lookup so repeated XPath queries + for (el.attributeEntries()) |*entry| { + // Memoized via frame._attribute_lookup so repeated XPath queries // (Capybara/Selenium polling) reuse the same *Attribute instead // of leaking fresh ones into page-lifetime storage on every call. - // Same pattern as Attribute.List.getAttribute / NamedNodeMap.getAtIndex. - const gop = try self.frame._attribute_lookup.getOrPut(self.frame.arena, @intFromPtr(entry)); - if (!gop.found_existing) { - gop.value_ptr.* = try entry.toAttribute(el, self.frame); - } - try out.append(self.arena, gop.value_ptr.*._proto); + const attribute = try el._attributes.getOrCreateAttribute(entry, el, self.frame); + try out.append(self.arena, attribute._proto); } } diff --git a/src/cdp/Node.zig b/src/cdp/Node.zig index b5ccbe6ff..6f521afdf 100644 --- a/src/cdp/Node.zig +++ b/src/cdp/Node.zig @@ -306,10 +306,9 @@ pub const Writer = struct { if (element.hasAttributes()) { try w.objectField("attributes"); try w.beginArray(); - var it = element.attributeIterator(); - while (it.next()) |attr| { - try w.write(attr._name.str()); - try w.write(attr._value.str()); + for (element.attributeEntries()) |*attr| { + try w.write(attr._name); + try w.write(attr._value); } try w.endArray(); } diff --git a/src/string.zig b/src/string.zig index 5c93c5c3c..181371114 100644 --- a/src/string.zig +++ b/src/string.zig @@ -212,6 +212,7 @@ pub const String = packed struct { // This can be used outside of the small string optimization pub fn intern(input: []const u8) ?[]const u8 { switch (input.len) { + 0 => return "", 1 => switch (input[0]) { '\n' => return "\n", '\r' => return "\r", @@ -248,36 +249,102 @@ pub const String = packed struct { 3 => switch (@as(u24, @bitCast(input[0..3].*))) { asUint(" ") => return " ", asUint("•") => return "•", + asUint("src") => return "src", + asUint("rel") => return "rel", + asUint("alt") => return "alt", + asUint("dir") => return "dir", + asUint("for") => return "for", else => {}, }, 4 => switch (@as(u32, @bitCast(input[0..4].*))) { asUint(" ") => return " ", asUint(" to ") => return " to ", + asUint("type") => return "type", + asUint("name") => return "name", + asUint("href") => return "href", + asUint("role") => return "role", + asUint("lang") => return "lang", + asUint("true") => return "true", + asUint("text") => return "text", + asUint("none") => return "none", + asUint("lazy") => return "lazy", else => {}, }, 5 => switch (@as(u40, @bitCast(input[0..5].*))) { asUint(" ") => return " ", asUint(" › ") => return " › ", + asUint("class") => return "class", + asUint("style") => return "style", + asUint("title") => return "title", + asUint("value") => return "value", + asUint("width") => return "width", + asUint("media") => return "media", + asUint("async") => return "async", + asUint("defer") => return "defer", + asUint("false") => return "false", else => {}, }, 6 => switch (@as(u48, @bitCast(input[0..6].*))) { asUint(" ") => return " ", + asUint("height") => return "height", + asUint("action") => return "action", + asUint("method") => return "method", + asUint("target") => return "target", + asUint("srcset") => return "srcset", + asUint("hidden") => return "hidden", + asUint("button") => return "button", + asUint("submit") => return "submit", + asUint("_blank") => return "_blank", else => {}, }, 7 => switch (@as(u56, @bitCast(input[0..7].*))) { asUint(" ") => return " ", + asUint("content") => return "content", + asUint("charset") => return "charset", + asUint("checked") => return "checked", + asUint("loading") => return "loading", else => {}, }, 8 => switch (@as(u64, @bitCast(input[0..8].*))) { asUint(" ") => return " ", + asUint("disabled") => return "disabled", + asUint("multiple") => return "multiple", + asUint("readonly") => return "readonly", + asUint("required") => return "required", + asUint("selected") => return "selected", + asUint("tabindex") => return "tabindex", + asUint("checkbox") => return "checkbox", + asUint("noopener") => return "noopener", + asUint("download") => return "download", else => {}, }, 9 => switch (@as(u72, @bitCast(input[0..9].*))) { asUint(" ") => return " ", + asUint("autofocus") => return "autofocus", + asUint("maxlength") => return "maxlength", + asUint("draggable") => return "draggable", + asUint("anonymous") => return "anonymous", else => {}, }, 10 => switch (@as(u80, @bitCast(input[0..10].*))) { asUint(" ") => return " ", + asUint("aria-label") => return "aria-label", + asUint("novalidate") => return "novalidate", + asUint("spellcheck") => return "spellcheck", + asUint("stylesheet") => return "stylesheet", + asUint("noreferrer") => return "noreferrer", + else => {}, + }, + 11 => switch (@as(u88, @bitCast(input[0..11].*))) { + asUint("aria-hidden") => return "aria-hidden", + asUint("crossorigin") => return "crossorigin", + asUint("placeholder") => return "placeholder", + else => {}, + }, + 12 => switch (@as(u96, @bitCast(input[0..12].*))) { + asUint("autocomplete") => return "autocomplete", + asUint("aria-current") => return "aria-current", + asUint("presentation") => return "presentation", else => {}, }, 13 => switch (@as(u104, @bitCast(input[0..13].*))) { @@ -285,6 +352,9 @@ pub const String = packed struct { asUint("padding-right") => return "padding-right", asUint("margin-bottom") => return "margin-bottom", asUint("space-between") => return "space-between", + asUint("aria-expanded") => return "aria-expanded", + asUint("aria-disabled") => return "aria-disabled", + asUint("aria-controls") => return "aria-controls", else => {}, }, 14 => switch (@as(u112, @bitCast(input[0..14].*))) { @@ -292,15 +362,18 @@ pub const String = packed struct { asUint("text-transform") => return "text-transform", asUint("letter-spacing") => return "letter-spacing", asUint("vertical-align") => return "vertical-align", + asUint("referrerpolicy") => return "referrerpolicy", else => {}, }, 15 => switch (@as(u120, @bitCast(input[0..15].*))) { asUint("text-decoration") => return "text-decoration", asUint("justify-content") => return "justify-content", + asUint("aria-labelledby") => return "aria-labelledby", else => {}, }, 16 => switch (@as(u128, @bitCast(input[0..16].*))) { asUint("background-color") => return "background-color", + asUint("aria-describedby") => return "aria-describedby", else => {}, }, else => {}, From 4a57e3c40200a7f174523605db38e327051977e7 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 8 Jul 2026 19:14:41 +0800 Subject: [PATCH 2/2] perf, mem: canonicalize attribute names Adds deduping to attribute names. This has two benefits. First, it results in fewer dupes/allocations. Second, it allows finding an attribute name by a single pointer comparison. Say we need to create an attribute attr1=val1 (from the parser or JS, doesn't matter). We: 1 - Lookup "attr1" in String.intern, it doesn't exist 2 - We lookup or create "attr1" in Frame._attribute_names Whether the name was found in String.intern, found in Frame._attribute_names or created in Frame._attribute_names, any attribute name "attr1" always points to the same value. Now say we want to lookup the value "attr1". We _could_ iterate the attribute list and do a string comparison on each attribute. OR, we could apply the same canonicalization to that input as we did when building the list. We can do this via a read-don't-create API. If it doesn't find it, than that attribute was never canonicalized and thus, cannot exist (early exit!). If it IS found, then we now have the pointer for "attr1" that all attributes with "attr1" use, so we can just do a pointer comparison. We also replace the name: []const u8, value: []const u8 with a packed representation (where len: u32, instead of u64) meaning every attribute entry saves an extra 8 bytes (on top of the 16 bytes saved by the previous commit) --- src/SemanticTree.zig | 6 +- src/browser/Frame.zig | 6 + src/browser/frame/node_factory.zig | 4 +- src/browser/webapi/CustomElementRegistry.zig | 4 +- src/browser/webapi/Element.zig | 12 +- src/browser/webapi/element/Attribute.zig | 175 +++++++++++++------ src/cdp/Node.zig | 4 +- 7 files changed, 140 insertions(+), 71 deletions(-) diff --git a/src/SemanticTree.zig b/src/SemanticTree.zig index ac556be00..4adb987f8 100644 --- a/src/SemanticTree.zig +++ b/src/SemanticTree.zig @@ -194,7 +194,7 @@ fn walk( var name = try axn.getName(self.frame, self.arena); const has_explicit_label = if (node.is(Element)) |el| - el.getAttributeSafe(.wrap("aria-label")) != null or el.getAttributeSafe(.wrap("title")) != null + el.getAttributeSafe(comptime .wrap("aria-label")) != null or el.getAttributeSafe(comptime .wrap("title")) != null else false; @@ -384,8 +384,8 @@ const JsonVisitor = struct { try self.jw.objectField("attributes"); try self.jw.beginObject(); for (el.attributeEntries()) |*attr| { - try self.jw.objectField(attr._name); - try self.jw.write(attr._value); + try self.jw.objectField(attr.name()); + try self.jw.write(attr.value()); } try self.jw.endObject(); } diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 7538fab6c..23c2047c8 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -113,6 +113,12 @@ _fragment_scripts_runnable: bool = false, // actual *Attributes. _attribute_lookup: Element.Attribute.List.Lookup = .empty, +// Canonical pool for attribute names that aren't in String.intern's. +// Every Attribute's entry's name is either a String intern or held here. +// This is both a memory optimization (deduping attribute names) and a performance +// optimization (since we can compare strings by just their pointer) +_attribute_names: std.StringHashMapUnmanaged(void) = .empty, + // Same as _atlribute_lookup, but instead of individual attributes, this is for // the return of elements.attributes. _attribute_named_node_map_lookup: std.AutoHashMapUnmanaged(usize, *Element.Attribute.NamedNodeMap) = .empty, diff --git a/src/browser/frame/node_factory.zig b/src/browser/frame/node_factory.zig index 56be67650..b647e439c 100644 --- a/src/browser/frame/node_factory.zig +++ b/src/browser/frame/node_factory.zig @@ -865,9 +865,9 @@ pub fn createElementNS(frame: *Frame, namespace: Element.Namespace, name: []cons for (element.attributeEntries()) |*attr| { Element.Html.Custom.enqueueAttributeChangedCallbackOnElement( element, - .wrap(attr._name), + .wrap(attr.name()), null, // old_value is null for initial attributes - .wrap(attr._value), + .wrap(attr.value()), null, frame, ); diff --git a/src/browser/webapi/CustomElementRegistry.zig b/src/browser/webapi/CustomElementRegistry.zig index 4a8e204c7..a6f19830f 100644 --- a/src/browser/webapi/CustomElementRegistry.zig +++ b/src/browser/webapi/CustomElementRegistry.zig @@ -214,9 +214,9 @@ pub fn upgradeCustomElement(custom: *Custom, definition: *CustomElementDefinitio // Enqueue attributeChangedCallback for existing observed attributes const element = custom.asElement(); for (element.attributeEntries()) |*attr| { - const name = lp.String.wrap(attr._name); + const name = lp.String.wrap(attr.name()); if (definition.isAttributeObserved(name)) { - Custom.enqueueAttributeChangedCallbackOnElement(element, name, null, .wrap(attr._value), null, frame); + Custom.enqueueAttributeChangedCallbackOnElement(element, name, null, .wrap(attr.value()), null, frame); } } diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index 806840308..2ed88adc4 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -374,15 +374,15 @@ pub fn lookupNamespaceURIForElement(self: *Element, prefix: ?[]const u8, frame: // Step 2: search xmlns attributes for (self._attributes.entries()) |*entry| { if (prefix == null) { - if (std.mem.eql(u8, entry._name, "xmlns")) { - const val = entry._value; + if (std.mem.eql(u8, entry.name(), "xmlns")) { + const val = entry.value(); return if (val.len == 0) null else val; } } else { - const name = entry._name; + const name = entry.name(); if (std.mem.startsWith(u8, name, "xmlns:")) { if (std.mem.eql(u8, name["xmlns:".len..], prefix.?)) { - const val = entry._value; + const val = entry.value(); return if (val.len == 0) null else val; } } @@ -408,8 +408,8 @@ pub fn lookupPrefixForElement(self: *Element, namespace: []const u8, frame: *Fra // Step 2: search xmlns: attribute declarations for one whose value is the namespace for (self._attributes.entries()) |*entry| { - const name = entry._name; - if (std.mem.startsWith(u8, name, "xmlns:") and std.mem.eql(u8, entry._value, namespace)) { + const name = entry.name(); + if (std.mem.startsWith(u8, name, "xmlns:") and std.mem.eql(u8, entry.value(), namespace)) { return name["xmlns:".len..]; } } diff --git a/src/browser/webapi/element/Attribute.zig b/src/browser/webapi/element/Attribute.zig index cae57785a..33c6297bc 100644 --- a/src/browser/webapi/element/Attribute.zig +++ b/src/browser/webapi/element/Attribute.zig @@ -135,24 +135,13 @@ pub const List = struct { const empty_entries: [0]Entry = .{}; - pub const Lookup = std.HashMapUnmanaged(LookupKey, *Attribute, LookupContext, std.hash_map.default_max_load_percentage); + pub const Lookup = std.AutoHashMapUnmanaged(LookupKey, *Attribute); // for Frame._attribute_lookup which is our identity map for attributes pub const LookupKey = struct { list: *const List, - // Owned by the frame arena (an entry's name) - name: []const u8, - }; - - const LookupContext = struct { - pub fn hash(_: LookupContext, key: LookupKey) u64 { - var h = std.hash.Wyhash.init(@intFromPtr(key.list)); - h.update(key.name); - return h.final(); - } - pub fn eql(_: LookupContext, a: LookupKey, b: LookupKey) bool { - return a.list == b.list and std.mem.eql(u8, a.name, b.name); - } + // canonical (see canonicalizeName), so identity is the address + name: [*]const u8, }; pub fn entries(self: *const List) []const Entry { @@ -169,7 +158,7 @@ pub const List = struct { pub fn get(self: *const List, name: String, frame: *Frame) !?String { const entry = (try self.getEntry(name, frame)) orelse return null; - return .wrap(entry._value); + return .wrap(entry.value()); } pub fn eql(self: *const List, other: *const List) bool { @@ -192,7 +181,7 @@ pub const List = struct { // meant for internal usage, where the name is known to be properly cased pub fn getSafe(self: *const List, name: String) ?[]const u8 { const entry = self.getEntryWithNormalizedName(name) orelse return null; - return entry._value; + return entry.value(); } // meant for internal usage, where the name is known to be properly cased @@ -208,7 +197,7 @@ pub const List = struct { // Identity map access: a given (list, name) always yields the same // *Attribute until the attribute is removed. pub fn getOrCreateAttribute(self: *const List, entry: *const Entry, element: ?*Element, frame: *Frame) !*Attribute { - const gop = try frame._attribute_lookup.getOrPut(frame.arena, .{ .list = self, .name = entry._name }); + const gop = try frame._attribute_lookup.getOrPut(frame.arena, .{ .list = self, .name = entry._name_ptr }); if (!gop.found_existing) { gop.value_ptr.* = try entry.toAttribute(element, frame); } @@ -233,19 +222,19 @@ pub const List = struct { var old_value: ?String = null; if (result.entry) |e| { // the old bytes are arena-owned or static; they outlive this update - old_value = String.wrap(e._value); + old_value = String.wrap(e.value()); if (is_id) { - frame.removeElementId(element, e._value); + frame.removeElementId(element, e.value()); } - e._value = try frame.dupeString(value.str()); + e.setValue(try frame.dupeString(value.str())); entry = e; } else { try self.ensureUnusedCapacity(1, frame); entry = &self._entries[self._len]; - entry.* = .{ - ._name = try frame.dupeString(result.normalized.str()), - ._value = try frame.dupeString(value.str()), - }; + entry.* = .init( + try canonicalizeName(result.normalized.str(), frame), + try frame.dupeString(value.str()), + ); self._len += 1; } @@ -253,10 +242,10 @@ pub const List = struct { const parent = element.asNode()._parent orelse { return entry; }; - try frame.addElementId(parent, element, entry._value); + try frame.addElementId(parent, element, entry.value()); } frame.domChanged(); - frame.attributeChange(element, result.normalized, .wrap(entry._value), old_value); + frame.attributeChange(element, result.normalized, .wrap(entry.value()), old_value); return entry; } @@ -267,10 +256,11 @@ pub const List = struct { try self.ensureTotalCapacity(other._len, frame); for (other.entries()) |*e| { const len = self._len; - self._entries[len] = .{ - ._name = try frame.dupeString(e._name), - ._value = try frame.dupeString(e._value), - }; + // re-canonicalize: `other` can belong to a different frame + self._entries[len] = .init( + try canonicalizeName(e.name(), frame), + try frame.dupeString(e.value()), + ); self._len = len + 1; } } @@ -293,7 +283,7 @@ pub const List = struct { const entry = try self.put(attribute._name, attribute._value, element, frame); attribute._element = element; - try frame._attribute_lookup.put(frame.arena, .{ .list = self, .name = entry._name }, attribute); + try frame._attribute_lookup.put(frame.arena, .{ .list = self, .name = entry._name_ptr }, attribute); return existing_attribute; } @@ -305,16 +295,16 @@ pub const List = struct { return; } const len = self._len; - if (len == std.math.maxInt(u16)) { + if (len == std.math.maxInt(u16) or name.len > std.math.maxInt(u32) or value.len > std.math.maxInt(u32)) { // Bad input. Drop rather than fail the parse return; } try self.ensureUnusedCapacity(1, frame); - self._entries[len] = .{ - ._name = try frame.dupeString(name), - ._value = try frame.dupeString(value), - }; + self._entries[len] = .init( + try canonicalizeName(name, frame), + try frame.dupeString(value), + ); self._len = len + 1; } @@ -323,15 +313,15 @@ pub const List = struct { const entry = result.entry orelse return; const is_id = shouldAddToIdMap(result.normalized, element); - const old_value = entry._value; + const old_value = entry.value(); if (is_id) { - frame.removeElementId(element, entry._value); + frame.removeElementId(element, old_value); } // remove this BEFORE triggering anything, incase that re-enters delete // or some other callback. - _ = frame._attribute_lookup.remove(.{ .list = self, .name = entry._name }); + _ = frame._attribute_lookup.remove(.{ .list = self, .name = entry._name_ptr }); const index = (@intFromPtr(entry) - @intFromPtr(self._entries)) / @sizeOf(Entry); const list_entries = self._entries[0..self._len]; std.mem.copyForwards(Entry, list_entries[index .. list_entries.len - 1], list_entries[index + 1 ..]); @@ -345,7 +335,7 @@ pub const List = struct { var arr: std.ArrayList([]const u8) = .empty; try arr.ensureTotalCapacity(allocator, self._len); for (self.entries()) |*e| { - arr.appendAssumeCapacity(e._name); + arr.appendAssumeCapacity(e.name()); } return arr.items; } @@ -395,45 +385,89 @@ pub const List = struct { const normalized = if (self.normalize) try normalizeNameForLookup(name, frame) else name; + // A name that was never canonicalized can't be in any list. + const canonical = lookupCanonicalName(normalized.str(), frame) orelse { + return .{ .normalized = normalized, .entry = null }; + }; return .{ .normalized = normalized, - .entry = self.getEntryWithNormalizedName(normalized), + .entry = self.getEntryWithCanonicalName(canonical.ptr), }; } - fn getEntryWithNormalizedName(self: *const List, name: String) ?*Entry { - const name_str = name.str(); + // This is one of the wins of the canonical names...we can compare strings + // as a single pointer comparison. By applying the same canonicalization to + // the lists attributes name and to an input, we get the same pointer for + // a given string. + fn getEntryWithCanonicalName(self: *const List, name_ptr: [*]const u8) ?*Entry { for (self._entries[0..self._len]) |*e| { - if (std.mem.eql(u8, e._name, name_str)) { + if (e._name_ptr == name_ptr) { return e; } } return null; } - pub const Entry = struct { - // Arena-owned or interned static bytes. Stable for the frame's - // lifetime, unlike the entry itself. - _name: []const u8, - _value: []const u8, + fn getEntryWithNormalizedName(self: *const List, name: String) ?*Entry { + const name_str = name.str(); + for (self._entries[0..self._len]) |*e| { + if (std.mem.eql(u8, e.name(), name_str)) { + return e; + } + } + return null; + } - /// Returns true if 2 entries are equal. + // Two []const u8 would be 32 bytes. Packed like this, it's 24. + pub const Entry = struct { + _name_ptr: [*]const u8, + _value_ptr: [*]const u8, + _name_len: u32, + _value_len: u32, + + fn init(canonical_name: []const u8, value_: []const u8) Entry { + return .{ + ._name_ptr = canonical_name.ptr, + ._value_ptr = value_.ptr, + ._name_len = @intCast(canonical_name.len), + ._value_len = @intCast(value_.len), + }; + } + + pub fn name(self: *const Entry) []const u8 { + return self._name_ptr[0..self._name_len]; + } + + pub fn value(self: *const Entry) []const u8 { + return self._value_ptr[0..self._value_len]; + } + + fn setValue(self: *Entry, value_: []const u8) void { + self._value_ptr = value_.ptr; + self._value_len = @intCast(value_.len); + } + + /// Returns true if 2 entries are equal. Names can't be compared by + /// pointer alone: the entries can belong to different frames. pub fn eql(self: *const Entry, other: *const Entry) bool { - return std.mem.eql(u8, self._name, other._name) and std.mem.eql(u8, self._value, other._value); + if (self._name_ptr != other._name_ptr and !std.mem.eql(u8, self.name(), other.name())) { + return false; + } + return std.mem.eql(u8, self.value(), other.value()); } pub fn format(self: *const Entry, writer: *std.Io.Writer) !void { - return formatAttribute(self._name, self._value, writer); + return formatAttribute(self.name(), self.value(), writer); } pub fn toAttribute(self: *const Entry, element: ?*Element, frame: *Frame) !*Attribute { return frame._factory.node(Attribute{ ._proto = undefined, ._element = element, - // The entry's arena-owned bytes outlive the entry itself, so - // the Attribute can wrap them without duping. - ._name = .wrap(self._name), - ._value = .wrap(self._value), + // The entry's bytes outlive the entry itself, so the + // Attribute can wrap them without duping. + ._name = .wrap(self.name()), + ._value = .wrap(self.value()), }); } }; @@ -479,6 +513,35 @@ pub fn validateAttributeName(name: String) !void { } } +// Every stored entry name either comes from the static String.intern or from +// the frame._attribute_names. This doesn't just avoid extra dupes/allocations, +// it gives us comparable pointer. +// For the lifetime of a frame: +// canonicalizeName("attrx").ptr == canonicalizeName("attrx").ptr +fn canonicalizeName(name: []const u8, frame: *Frame) ![]const u8 { + if (String.intern(name)) |static| { + return static; + } + const gop = try frame._attribute_names.getOrPut(frame.arena, name); + if (!gop.found_existing) { + gop.key_ptr.* = try frame.arena.dupe(u8, name); + } + return gop.key_ptr.*; +} + +// Let's say you want to find an attribute name "attrx". We could iterate an +// attribute list to do the string comparison. OR, we could run "attrx" through +// the same canonicalization as we applied to the attribute's names. If we don't +// find a canonical value, then it's then this attribute was never canonicalized +// before (and thus, we can shortcircuit a search). If we DO canonicalize it +// the we get do a pointer comparison rather than a full string comparison. +fn lookupCanonicalName(name: []const u8, frame: *const Frame) ?[]const u8 { + if (String.intern(name)) |static| { + return static; + } + return frame._attribute_names.getKey(name); +} + fn normalizeNameForLookup(name: String, frame: *Frame) !String { if (!needsLowerCasing(name.str())) { return name; diff --git a/src/cdp/Node.zig b/src/cdp/Node.zig index 6f521afdf..5c11bcff6 100644 --- a/src/cdp/Node.zig +++ b/src/cdp/Node.zig @@ -307,8 +307,8 @@ pub const Writer = struct { try w.objectField("attributes"); try w.beginArray(); for (element.attributeEntries()) |*attr| { - try w.write(attr._name); - try w.write(attr._value); + try w.write(attr.name()); + try w.write(attr.value()); } try w.endArray(); }