From 11ac11496d72cbecc50147291283ab6fb1ce46ce Mon Sep 17 00:00:00 2001 From: Halil Durak Date: Fri, 30 Jan 2026 14:32:48 +0300 Subject: [PATCH] rework `_element_attr_listeners` Merges caching and lazy evaluation logic in `_element_attr_listeners`; this introduce allocations for inline function values, which is a price we pay for future compat w/ modern browsers. Spec also require us to keep a list of **internal raw uncompiled handler**, which is a fancy way to say store-as-bytes-evaluate-later. --- src/browser/Page.zig | 55 ++++++++++++++++++++++++----- src/browser/webapi/Element.zig | 9 ++++- src/browser/webapi/element/Html.zig | 25 +------------ 3 files changed, 55 insertions(+), 34 deletions(-) diff --git a/src/browser/Page.zig b/src/browser/Page.zig index d5dd28f3..54b8d479 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -276,7 +276,6 @@ fn reset(self: *Page, comptime initializing: bool) !void { self._arena_pool_leak_track.clearRetainingCapacity(); } - // We force a garbage collection between page navigations to keep v8 // memory usage as low as possible. self._session.browser.env.memoryPressureNotification(.moderate); @@ -1173,7 +1172,10 @@ pub fn setAttrListener( self: *Page, element: *Element, listener_type: Element.KnownListener, - listener_callback: JS.Function.Global, + /// This can be; + /// []const u8 or []u8 (memory will be duped anyway) or, + /// JS function (persisted). + raw_or_function: anytype, ) !void { if (comptime IS_DEBUG) { log.debug(.event, "Page.setAttrListener", .{ @@ -1184,16 +1186,53 @@ pub fn setAttrListener( const key = element.calcAttrListenerKey(listener_type); const gop = try self._element_attr_listeners.getOrPut(self.arena, key); - gop.value_ptr.* = listener_callback; + switch (@TypeOf(raw_or_function)) { + []const u8, []u8 => gop.value_ptr.* = .{ .raw = try self.arena.dupe(u8, raw_or_function) }, + JS.Function.Global => gop.value_ptr.* = .{ .function = raw_or_function }, + else => |T| @panic("setAttrListener: unknown type " ++ @typeName(T)), + } } -/// Returns the inline event listener by an element and listener type. +/// Returns the inline event listener function by an element and listener type. pub fn getAttrListener( - self: *const Page, + self: *Page, element: *Element, listener_type: Element.KnownListener, ) ?JS.Function.Global { - return self._element_attr_listeners.get(element.calcAttrListenerKey(listener_type)); + const listeners = &self._element_attr_listeners; + // Check if there's such attr listener. + const key = element.calcAttrListenerKey(listener_type); + const listener = listeners.getPtr(key) orelse return null; + + return switch (listener.*) { + // Fast path. + .function => |function| function, + // Lazy evaluation. + .raw => |untrusted| { + // First time access to this getter; try to compile as function and cache result. + const function = self.js.stringToPersistedFunction(untrusted) catch |err| { + // Not a valid expression; log this to find out if its something + // that we should be supporting. + log.warn(.unknown_prop, "Page.getAttrListener", .{ + .expression = untrusted, + .err = err, + }); + // We can remove this safely. + const result = listeners.remove(key); + lp.assert(result == true, "Page.getAttrListener: unexpected result", .{}); + // Remove invalid bytes. + self.arena.free(untrusted); + + return null; + }; + + // Now that we obtained a function; this has no use. + self.arena.free(untrusted); + // Cache the resulting function. + listener.* = .{ .function = function }; + return function; + }, + }; } pub fn registerPerformanceObserver(self: *Page, observer: *PerformanceObserver) !void { @@ -2242,14 +2281,12 @@ fn populateElementAttributes(self: *Page, element: *Element, list: anytype) !voi const has_on_prefix = @as(u16, @bitCast([2]u8{ name.ptr[0], name.ptr[1 % name.len] })) == asUint("on"); // We may have found an event handler. if (has_on_prefix) { - // Must be usable as function. - const func = self.js.stringToPersistedFunction(attr.value.slice()) catch continue; - // Longest known listener kind is 32 bytes long. const remaining: u6 = @truncate(name.len -| 2); const unsafe = name.ptr + 2; const Vec16x8 = @Vector(16, u8); const Vec32x8 = @Vector(32, u8); + const func = attr.value.slice(); switch (remaining) { 3 => if (@as(u24, @bitCast(unsafe[0..3].*)) == asUint("cut")) { diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index f4cebc15..4e3a5833 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -53,8 +53,15 @@ pub const AssignedSlotLookup = std.AutoHashMapUnmanaged(*Element, *Html.Slot); /// /// See `calcAttrListenerKey` to obtain one. const AttrListenerKey = u64; +/// We lazily evaluate functions in order to not to create garbage. +const AttrListener = union(enum(u1)) { + /// Raw bytes; this may or may not be a valid JS expression. + raw: []const u8, + /// A valid JS function; can be executed directly. + function: js.Function.Global, +}; /// Use `getAttrListenerKey` to create a key. -pub const AttrListenerLookup = std.AutoHashMapUnmanaged(AttrListenerKey, js.Function.Global); +pub const AttrListenerLookup = std.AutoHashMapUnmanaged(AttrListenerKey, AttrListener); /// Enum of known event listeners; increasing the size of it (u7) /// can cause `AttrListenerKey` to behave incorrectly. diff --git a/src/browser/webapi/element/Html.zig b/src/browser/webapi/element/Html.zig index ac8b5c59..79ecc4b2 100644 --- a/src/browser/webapi/element/Html.zig +++ b/src/browser/webapi/element/Html.zig @@ -339,30 +339,7 @@ fn getAttributeFunction( listener_type: Element.KnownListener, page: *Page, ) ?js.Function.Global { - const element = self.asElement(); - // Check if we've already cached this. - if (page.getAttrListener(element, listener_type)) |cached_func| { - return cached_func; - } - - // Not found in cache; parse from attribute list. - const js_expression = element.getAttributeSafe(.wrap(@tagName(listener_type))) orelse { - return null; - }; - - const callback = page.js.stringToPersistedFunction(js_expression) catch { - // Not a valid expression; log this to find out if its something - // that we should be supporting. - log.warn(.unknown_prop, "Html.getAttributeFunction", .{ .expression = js_expression }); - return null; - }; - - // Cache the function for future calls. - page.setAttrListener(element, listener_type, callback) catch { - // This is fine :tm: we likely hit out of memory. - }; - - return callback; + return page.getAttrListener(self.asElement(), listener_type); } pub fn setOnAbort(self: *HtmlElement, callback: js.Function.Global, page: *Page) !void {