From c63c85071a0200884cb504ce777465c3bb894901 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 21 Jan 2026 18:07:49 +0800 Subject: [PATCH] Start to eagerly reset globals. Currently, when you create a Global (Value, Object, Function, ...) it exists until the context is destroyed. This PR adds the ability to eagerly free them when they fall out of scope, which is only possible because of the new finalizer hooks. Previously, we had js.Value, js.Value.Global; js.Function, js.Function.Global, etc. This PR introduces a .Temp variant: js.Value.Temp and js.Function.Temp. This is purely a discriminatory type and it behaves (and IS) a Global. The difference is that it can be released: page.js.release(self._on_ready_state_change.?) Why a new type? There's no guarantee that a global (the existing .Global or the new .Temp) will get released before the context ends. For this reason, we always track them in order to free the on context deninit: ```zig for (self.global_functions.items) |*global| { v8.v8__Global__Reset(global); } ``` If a .Temp is eagerly released, we need to remove it from this list. The simple solution would be to switch `global_functions` from an ArrayList to a HashMap. But that adds overhead for values that we know we'll never be able to eagerly release. For this reason, .Temp are stored in a hashmap (and can be released) and .Globla are stored in an ArrayList (and cannot be released). It's a micro- optimization...eagerly releasing doesn't have to O(N) scan the list, and we only pay the memory overhead of the hashmap for values that have a change to be eagerly freed. Eager-freeing is now applied to both the callbacn and the values for window timers (setTimeout, setInterval, RAF). And to the XHR ready_state_change callback. (we'll do more as we go). --- src/browser/js/Context.zig | 67 +++++++++++++---- src/browser/js/Function.zig | 60 +++++++++++---- src/browser/js/Local.zig | 89 ++++++++--------------- src/browser/js/Value.zig | 56 +++++++++----- src/browser/webapi/Window.zig | 30 ++++---- src/browser/webapi/net/XMLHttpRequest.zig | 17 +++-- 6 files changed, 189 insertions(+), 130 deletions(-) diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 9314183c..00ebfcbc 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -99,6 +99,11 @@ global_promises: std.ArrayList(v8.Global) = .empty, global_functions: std.ArrayList(v8.Global) = .empty, global_promise_resolvers: std.ArrayList(v8.Global) = .empty, +// Temp variants stored in HashMaps for O(1) early cleanup. +// Key is global.data_ptr. +global_values_temp: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, +global_functions_temp: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, + // Our module cache: normalized module specifier => module. module_cache: std.StringHashMapUnmanaged(ModuleEntry) = .empty, @@ -181,6 +186,20 @@ pub fn deinit(self: *Context) void { v8.v8__Global__Reset(global); } + { + var it = self.global_values_temp.valueIterator(); + while (it.next()) |global| { + v8.v8__Global__Reset(global); + } + } + + { + var it = self.global_functions_temp.valueIterator(); + while (it.next()) |global| { + v8.v8__Global__Reset(global); + } + } + if (self.entered) { var ls: js.Local.Scope = undefined; self.localScope(&ls); @@ -212,24 +231,40 @@ pub fn strongRef(self: *Context, obj: anytype) void { v8.v8__Global__ClearWeak(global); } -pub fn release(self: *Context, obj: *anyopaque) void { - var global = self.identity_map.fetchRemove(@intFromPtr(obj)) orelse { - if (comptime IS_DEBUG) { - // should not be possible - std.debug.assert(false); - } - return; - }; - v8.v8__Global__Reset(&global.value); +pub fn release(self: *Context, item: anytype) void { + if (@TypeOf(item) == *anyopaque) { + // Existing *anyopaque path for identity_map. Called internally from + // finalizers + var global = self.identity_map.fetchRemove(@intFromPtr(item)) orelse { + if (comptime IS_DEBUG) { + // should not be possible + std.debug.assert(false); + } + return; + }; + v8.v8__Global__Reset(&global.value); - // The item has been fianalized, remove it for the finalizer callback so that - // we don't try to call it again on shutdown. - _ = self.finalizer_callbacks.fetchRemove(@intFromPtr(obj)) orelse { - if (comptime IS_DEBUG) { - // should not be possible - std.debug.assert(false); - } + // The item has been fianalized, remove it for the finalizer callback so that + // we don't try to call it again on shutdown. + _ = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { + if (comptime IS_DEBUG) { + // should not be possible + std.debug.assert(false); + } + }; + return; + } + + var map = switch (@TypeOf(item)) { + js.Value.Temp => &self.global_values_temp, + js.Function.Temp => &self.global_functions_temp, + else => |T| @compileError("Context.release cannot be called with a " ++ @typeName(T)), }; + + if (map.fetchRemove(item.handle.data_ptr)) |kv| { + var global = kv.value; + v8.v8__Global__Reset(&global); + } } // Any operation on the context have to be made from a local. diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index 9f83f70d..c1ed050b 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -171,33 +171,61 @@ pub fn getPropertyValue(self: *const Function, name: []const u8) !?js.Value { } pub fn persist(self: *const Function) !Global { + return self._persist(true); +} + +pub fn temp(self: *const Function) !Temp { + return self._persist(false); +} + +fn _persist(self: *const Function, comptime is_global: bool) !(if (is_global) Global else Temp) { var ctx = self.local.ctx; + var global: v8.Global = undefined; v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); - try ctx.global_functions.append(ctx.arena, global); + if (comptime is_global) { + try ctx.global_functions.append(ctx.arena, global); + } else { + try ctx.global_functions_temp.put(ctx.arena, global.data_ptr, global); + } return .{ .handle = global }; } +pub fn tempWithThis(self: *const Function, value: anytype) !Temp { + const with_this = try self.withThis(value); + return with_this.temp(); +} + pub fn persistWithThis(self: *const Function, value: anytype) !Global { const with_this = try self.withThis(value); return with_this.persist(); } -pub const Global = struct { - handle: v8.Global, +pub const Temp = G(0); +pub const Global = G(1); - pub fn deinit(self: *Global) void { - v8.v8__Global__Reset(&self.handle); - } +fn G(comptime discriminator: u8) type { + return struct { + handle: v8.Global, - pub fn local(self: *const Global, l: *const js.Local) Function { - return .{ - .local = l, - .handle = @ptrCast(v8.v8__Global__Get(&self.handle, l.isolate.handle)), - }; - } + // makes the types different (G(0) != G(1)), without taking up space + comptime _: u8 = discriminator, - pub fn isEqual(self: *const Global, other: Function) bool { - return v8.v8__Global__IsEqual(&self.handle, other.handle); - } -}; + const Self = @This(); + + pub fn deinit(self: *Self) void { + v8.v8__Global__Reset(&self.handle); + } + + pub fn local(self: *const Self, l: *const js.Local) Function { + return .{ + .local = l, + .handle = @ptrCast(v8.v8__Global__Get(&self.handle, l.isolate.handle)), + }; + } + + pub fn isEqual(self: *const Self, other: Function) bool { + return v8.v8__Global__IsEqual(&self.handle, other.handle); + } + }; +} diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index 312d7127..b74331f4 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -293,61 +293,29 @@ pub fn zigValueToJs(self: *const Local, value: anytype, comptime opts: CallOpts) } } - if (T == js.Function) { - // we're returning a callback - return .{ .local = self, .handle = @ptrCast(value.handle) }; - } + // zig fmt: off + switch (T) { + js.Value => return value, + js.Exception => return .{ .local = self, .handle = isolate.throwException(value.handle) }, - if (T == js.Function.Global) { - // Auto-convert Global to local for bridge - return .{ .local = self, .handle = @ptrCast(value.local(self).handle) }; - } + inline + js.Function, + js.Object, + js.Promise, + js.String => return .{ .local = self, .handle = @ptrCast(value.handle) }, - if (T == js.Object) { - // we're returning a v8.Object - return .{ .local = self, .handle = @ptrCast(value.handle) }; - } - - if (T == js.Object.Global) { - // Auto-convert Global to local for bridge - return .{ .local = self, .handle = @ptrCast(value.local(self).handle) }; - } - - if (T == js.Value.Global) { - // Auto-convert Global to local for bridge - return .{ .local = self, .handle = @ptrCast(value.local(self).handle) }; - } - - if (T == js.Promise.Global) { - // Auto-convert Global to local for bridge - return .{ .local = self, .handle = @ptrCast(value.local(self).handle) }; - } - - if (T == js.PromiseResolver.Global) { - // Auto-convert Global to local for bridge - return .{ .local = self, .handle = @ptrCast(value.local(self).handle) }; - } - - if (T == js.Module.Global) { - // Auto-convert Global to local for bridge - return .{ .local = self, .handle = @ptrCast(value.local(self).handle) }; - } - - if (T == js.Value) { - return value; - } - - if (T == js.Promise) { - return .{ .local = self, .handle = @ptrCast(value.handle) }; - } - - if (T == js.Exception) { - return .{ .local = self, .handle = isolate.throwException(value.handle) }; - } - - if (T == js.String) { - return .{ .local = self, .handle = @ptrCast(value.handle) }; + inline + js.Function.Global, + js.Function.Temp, + js.Value.Global, + js.Value.Temp, + js.Object.Global, + js.Promise.Global, + js.PromiseResolver.Global, + js.Module.Global => return .{ .local = self, .handle = @ptrCast(value.local(self).handle) }, + else => {} } + // zig fmt: on if (@hasDecl(T, "runtimeGenericWrap")) { const wrap = try value.runtimeGenericWrap(self.ctx.page); @@ -596,17 +564,17 @@ pub fn jsValueToZig(self: *const Local, comptime T: type, js_val: js.Value) !T { // probeJsValueToZig. Avoids having to duplicate this logic when probing. fn jsValueToStruct(self: *const Local, comptime T: type, js_val: js.Value) !?T { return switch (T) { - js.Function => { + js.Function, js.Function.Global, js.Function.Temp => { if (!js_val.isFunction()) { return null; } - return .{ .local = self, .handle = @ptrCast(js_val.handle) }; - }, - js.Function.Global => { - if (!js_val.isFunction()) { - return null; - } - return try (js.Function{ .local = self, .handle = @ptrCast(js_val.handle) }).persist(); + const js_func = js.Function{ .local = self, .handle = @ptrCast(js_val.handle) }; + return switch (T) { + js.Function => js_func, + js.Function.Temp => try js_func.temp(), + js.Function.Global => try js_func.persist(), + else => unreachable, + }; }, // zig fmt: off js.TypedArray(u8), js.TypedArray(u16), js.TypedArray(u32), js.TypedArray(u64), @@ -620,6 +588,7 @@ fn jsValueToStruct(self: *const Local, comptime T: type, js_val: js.Value) !?T { }, js.Value => js_val, js.Value.Global => return try js_val.persist(), + js.Value.Temp => return try js_val.temp(), js.Object => { if (!js_val.isObject()) { return null; diff --git a/src/browser/js/Value.zig b/src/browser/js/Value.zig index e37594e4..d76313d5 100644 --- a/src/browser/js/Value.zig +++ b/src/browser/js/Value.zig @@ -236,13 +236,23 @@ fn _toString(self: Value, comptime null_terminate: bool, opts: js.String.ToZigOp } pub fn persist(self: Value) !Global { + return self._persist(true); +} + +pub fn temp(self: Value) !Temp { + return self._persist(false); +} + +fn _persist(self: *const Value, comptime is_global: bool) !(if (is_global) Global else Temp) { var ctx = self.local.ctx; var global: v8.Global = undefined; v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); - - try ctx.global_values.append(ctx.arena, global); - + if (comptime is_global) { + try ctx.global_values.append(ctx.arena, global); + } else { + try ctx.global_values_temp.put(ctx.arena, global.data_ptr, global); + } return .{ .handle = global }; } @@ -290,21 +300,31 @@ pub fn format(self: Value, writer: *std.Io.Writer) !void { return writer.writeAll(str); } -pub const Global = struct { - handle: v8.Global, +pub const Temp = G(0); +pub const Global = G(1); - pub fn deinit(self: *Global) void { - v8.v8__Global__Reset(&self.handle); - } +fn G(comptime discriminator: u8) type { + return struct { + handle: v8.Global, - pub fn local(self: *const Global, l: *const js.Local) Value { - return .{ - .local = l, - .handle = @ptrCast(v8.v8__Global__Get(&self.handle, l.isolate.handle)), - }; - } + // makes the types different (G(0) != G(1)), without taking up space + comptime _: u8 = discriminator, - pub fn isEqual(self: *const Global, other: Value) bool { - return v8.v8__Global__IsEqual(&self.handle, other.handle); - } -}; + const Self = @This(); + + pub fn deinit(self: *Self) void { + v8.v8__Global__Reset(&self.handle); + } + + pub fn local(self: *const Self, l: *const js.Local) Value { + return .{ + .local = l, + .handle = @ptrCast(v8.v8__Global__Get(&self.handle, l.isolate.handle)), + }; + } + + pub fn isEqual(self: *const Self, other: Value) bool { + return v8.v8__Global__IsEqual(&self.handle, other.handle); + } + }; +} diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index 940b41dd..3e96cc89 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -194,7 +194,7 @@ pub fn fetch(_: *const Window, input: Fetch.Input, options: ?Fetch.InitOpts, pag return Fetch.init(input, options, page); } -pub fn setTimeout(self: *Window, cb: js.Function.Global, delay_ms: ?u32, params: []js.Value.Global, page: *Page) !u32 { +pub fn setTimeout(self: *Window, cb: js.Function.Temp, delay_ms: ?u32, params: []js.Value.Temp, page: *Page) !u32 { return self.scheduleCallback(cb, delay_ms orelse 0, .{ .repeat = false, .params = params, @@ -203,7 +203,7 @@ pub fn setTimeout(self: *Window, cb: js.Function.Global, delay_ms: ?u32, params: }, page); } -pub fn setInterval(self: *Window, cb: js.Function.Global, delay_ms: ?u32, params: []js.Value.Global, page: *Page) !u32 { +pub fn setInterval(self: *Window, cb: js.Function.Temp, delay_ms: ?u32, params: []js.Value.Temp, page: *Page) !u32 { return self.scheduleCallback(cb, delay_ms orelse 0, .{ .repeat = true, .params = params, @@ -212,7 +212,7 @@ pub fn setInterval(self: *Window, cb: js.Function.Global, delay_ms: ?u32, params }, page); } -pub fn setImmediate(self: *Window, cb: js.Function.Global, params: []js.Value.Global, page: *Page) !u32 { +pub fn setImmediate(self: *Window, cb: js.Function.Temp, params: []js.Value.Temp, page: *Page) !u32 { return self.scheduleCallback(cb, 0, .{ .repeat = false, .params = params, @@ -221,7 +221,7 @@ pub fn setImmediate(self: *Window, cb: js.Function.Global, params: []js.Value.Gl }, page); } -pub fn requestAnimationFrame(self: *Window, cb: js.Function.Global, page: *Page) !u32 { +pub fn requestAnimationFrame(self: *Window, cb: js.Function.Temp, page: *Page) !u32 { return self.scheduleCallback(cb, 5, .{ .repeat = false, .params = &.{}, @@ -258,7 +258,7 @@ pub fn cancelAnimationFrame(self: *Window, id: u32) void { const RequestIdleCallbackOpts = struct { timeout: ?u32 = null, }; -pub fn requestIdleCallback(self: *Window, cb: js.Function.Global, opts_: ?RequestIdleCallbackOpts, page: *Page) !u32 { +pub fn requestIdleCallback(self: *Window, cb: js.Function.Temp, opts_: ?RequestIdleCallbackOpts, page: *Page) !u32 { const opts = opts_ orelse RequestIdleCallbackOpts{}; return self.scheduleCallback(cb, opts.timeout orelse 50, .{ .mode = .idle, @@ -496,13 +496,13 @@ pub fn scrollTo(self: *Window, opts: ScrollToOpts, y: ?i32, page: *Page) !void { const ScheduleOpts = struct { repeat: bool, - params: []js.Value.Global, + params: []js.Value.Temp, name: []const u8, low_priority: bool = false, animation_frame: bool = false, mode: ScheduleCallback.Mode = .normal, }; -fn scheduleCallback(self: *Window, cb: js.Function.Global, delay_ms: u32, opts: ScheduleOpts, page: *Page) !u32 { +fn scheduleCallback(self: *Window, cb: js.Function.Temp, delay_ms: u32, opts: ScheduleOpts, page: *Page) !u32 { if (self._timers.count() > 512) { // these are active return error.TooManyTimeout; @@ -512,9 +512,9 @@ fn scheduleCallback(self: *Window, cb: js.Function.Global, delay_ms: u32, opts: self._timer_id = timer_id; const params = opts.params; - var persisted_params: []js.Value.Global = &.{}; + var persisted_params: []js.Value.Temp = &.{}; if (params.len > 0) { - persisted_params = try page.arena.dupe(js.Value.Global, params); + persisted_params = try page.arena.dupe(js.Value.Temp, params); } const gop = try self._timers.getOrPut(page.arena, timer_id); @@ -554,11 +554,11 @@ const ScheduleCallback = struct { // delay, in ms, to repeat. When null, will be removed after the first time repeat_ms: ?u32, - cb: js.Function.Global, + cb: js.Function.Temp, page: *Page, - params: []const js.Value.Global, + params: []const js.Value.Temp, removed: bool = false, @@ -571,6 +571,10 @@ const ScheduleCallback = struct { }; fn deinit(self: *ScheduleCallback) void { + self.page.js.release(self.cb); + for (self.params) |param| { + self.page.js.release(param); + } self.page._factory.destroy(self); } @@ -605,14 +609,12 @@ const ScheduleCallback = struct { }; }, } - + ls.local.runMicrotasks(); if (self.repeat_ms) |ms| { return ms; } defer self.deinit(); - _ = page.window._timers.remove(self.timer_id); - ls.local.runMicrotasks(); return null; } }; diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index 38ef0564..35773921 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -55,7 +55,7 @@ _response_headers: std.ArrayList([]const u8) = .empty, _response_type: ResponseType = .text, _ready_state: ReadyState = .unsent, -_on_ready_state_change: ?js.Function.Global = null, +_on_ready_state_change: ?js.Function.Temp = null, const ReadyState = enum(u8) { unsent = 0, @@ -79,7 +79,7 @@ const ResponseType = enum { }; pub fn init(page: *Page) !*XMLHttpRequest { - const arena = try page.getArena(.{.debug = "XMLHttpRequest"}); + const arena = try page.getArena(.{ .debug = "XMLHttpRequest" }); errdefer page.releaseArena(arena); return page._factory.xhrEventTarget(XMLHttpRequest{ ._page = page, @@ -98,21 +98,26 @@ pub fn deinit(self: *XMLHttpRequest, comptime shutdown: bool) void { } self._transfer = null; } - self._page.releaseArena(self._arena); - self._page._factory.destroy(self); + + const page = self._page; + if (self._on_ready_state_change) |func| { + page.js.release(func); + } + page.releaseArena(self._arena); + page._factory.destroy(self); } fn asEventTarget(self: *XMLHttpRequest) *EventTarget { return self._proto._proto; } -pub fn getOnReadyStateChange(self: *const XMLHttpRequest) ?js.Function.Global { +pub fn getOnReadyStateChange(self: *const XMLHttpRequest) ?js.Function.Temp { return self._on_ready_state_change; } pub fn setOnReadyStateChange(self: *XMLHttpRequest, cb_: ?js.Function) !void { if (cb_) |cb| { - self._on_ready_state_change = try cb.persistWithThis(self); + self._on_ready_state_change = try cb.tempWithThis(self); } else { self._on_ready_state_change = null; }