Merge pull request #1396 from lightpanda-io/eager_global_reset

Start to eagerly reset globals.

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,8 +231,11 @@ pub fn strongRef(self: *Context, obj: anytype) void {
     v8.v8__Global__ClearWeak(global);
 }
 
-pub fn release(self: *Context, obj: *anyopaque) void {
+pub fn release(self: *Context, item: anytype) void {
-    var global = self.identity_map.fetchRemove(@intFromPtr(obj)) orelse {
+    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);
@@ -224,12 +246,25 @@ pub fn release(self: *Context, obj: *anyopaque) void {
 
         // 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 {
+        _ = 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.

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);
+    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 {
+pub const Temp = G(0);
+pub const Global = G(1);
+
+fn G(comptime discriminator: u8) type {
+    return struct {
         handle: v8.Global,
 
-    pub fn deinit(self: *Global) void {
+        // makes the types different (G(0) != G(1)), without taking up space
+        comptime _: u8 = discriminator,
+
+        const Self = @This();
+
+        pub fn deinit(self: *Self) void {
             v8.v8__Global__Reset(&self.handle);
         }
 
-    pub fn local(self: *const Global, l: *const js.Local) Function {
+        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 Global, other: Function) bool {
+        pub fn isEqual(self: *const Self, other: Function) bool {
             return v8.v8__Global__IsEqual(&self.handle, other.handle);
         }
     };
+}

src/browser/js/Local.zig

@@ -207,7 +207,10 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object,
                 }
 
                 try ctx.finalizer_callbacks.put(ctx.arena, @intFromPtr(resolved.ptr), .init(value));
-                if (@hasDecl(JsApi.Meta, "finalizer")) {
+                if (@hasDecl(JsApi.Meta, "weak")) {
+                    if (comptime IS_DEBUG) {
+                        std.debug.assert(JsApi.Meta.weak == true);
+                    }
                     v8.v8__Global__SetWeakFinalizer(gop.value_ptr, resolved.ptr, JsApi.Meta.finalizer.from_v8, v8.kParameter);
                 }
             }
@@ -290,61 +293,29 @@ pub fn zigValueToJs(self: *const Local, value: anytype, comptime opts: CallOpts)
                 }
             }
 
-            if (T == js.Function) {
+            // zig fmt: off
-                // we're returning a callback
+            switch (T) {
-                return .{ .local = self, .handle = @ptrCast(value.handle) };
+                js.Value => return value,
-            }
+                js.Exception => return .{ .local = self, .handle = isolate.throwException(value.handle) },
 
-            if (T == js.Function.Global) {
+                inline
-                // Auto-convert Global to local for bridge
+                js.Function,
-                return .{ .local = self, .handle = @ptrCast(value.local(self).handle) };
+                js.Object,
-            }
+                js.Promise,
+                js.String => return .{ .local = self, .handle = @ptrCast(value.handle) },
 
-            if (T == js.Object) {
+                inline
-                // we're returning a v8.Object
+                js.Function.Global,
-                return .{ .local = self, .handle = @ptrCast(value.handle) };
+                js.Function.Temp,
-            }
+                js.Value.Global,
-
+                js.Value.Temp,
-            if (T == js.Object.Global) {
+                js.Object.Global,
-                // Auto-convert Global to local for bridge
+                js.Promise.Global,
-                return .{ .local = self, .handle = @ptrCast(value.local(self).handle) };
+                js.PromiseResolver.Global,
-            }
+                js.Module.Global => return .{ .local = self, .handle = @ptrCast(value.local(self).handle) },
-
+                else => {}
-            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) };
             }
+            // zig fmt: on
 
             if (@hasDecl(T, "runtimeGenericWrap")) {
                 const wrap = try value.runtimeGenericWrap(self.ctx.page);
@@ -593,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) };
+            const js_func = js.Function{ .local = self, .handle = @ptrCast(js_val.handle) };
-        },
+            return switch (T) {
-        js.Function.Global => {
+                js.Function => js_func,
-            if (!js_val.isFunction()) {
+                js.Function.Temp => try js_func.temp(),
-                return null;
+                js.Function.Global => try js_func.persist(),
-            }
+                else => unreachable,
-            return try (js.Function{ .local = self, .handle = @ptrCast(js_val.handle) }).persist();
+            };
         },
         // zig fmt: off
         js.TypedArray(u8), js.TypedArray(u16), js.TypedArray(u32), js.TypedArray(u64),
@@ -617,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;

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);
-
+    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 {
+pub const Temp = G(0);
+pub const Global = G(1);
+
+fn G(comptime discriminator: u8) type {
+    return struct {
         handle: v8.Global,
 
-    pub fn deinit(self: *Global) void {
+        // makes the types different (G(0) != G(1)), without taking up space
+        comptime _: u8 = discriminator,
+
+        const Self = @This();
+
+        pub fn deinit(self: *Self) void {
             v8.v8__Global__Reset(&self.handle);
         }
 
-    pub fn local(self: *const Global, l: *const js.Local) Value {
+        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 Global, other: Value) bool {
+        pub fn isEqual(self: *const Self, other: Value) bool {
             return v8.v8__Global__IsEqual(&self.handle, other.handle);
         }
     };
+}

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;
     }
 };

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,
@@ -81,8 +81,7 @@ const ResponseType = enum {
 pub fn init(page: *Page) !*XMLHttpRequest {
     const arena = try page.getArena(.{ .debug = "XMLHttpRequest" });
     errdefer page.releaseArena(arena);
-
+    return page._factory.xhrEventTarget(XMLHttpRequest{
-    return try page._factory.xhrEventTarget(XMLHttpRequest{
         ._page = page,
         ._arena = arena,
         ._proto = undefined,
@@ -99,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;
     }
@@ -157,6 +161,7 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void {
     if (self._ready_state != .opened) {
         return error.InvalidStateError;
     }
+    self._page.js.strongRef(self);
 
     if (body_) |b| {
         if (self._method != .GET and self._method != .HEAD) {
@@ -394,6 +399,8 @@ fn httpDoneCallback(ctx: *anyopaque) !void {
         .total = loaded,
         .loaded = loaded,
     }, local, page);
+
+    page.js.weakRef(self);
 }
 
 fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void {
@@ -401,6 +408,7 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void {
     // http client will close it after an error, it isn't safe to keep around
     self._transfer = null;
     self.handleError(err);
+    self._page.js.weakRef(self);
 }
 
 pub fn abort(self: *XMLHttpRequest) void {
@@ -409,6 +417,7 @@ pub fn abort(self: *XMLHttpRequest) void {
         transfer.abort(error.Abort);
         self._transfer = null;
     }
+    self._page.js.weakRef(self);
 }
 
 fn handleError(self: *XMLHttpRequest, err: anyerror) void {
@@ -486,6 +495,7 @@ pub const JsApi = struct {
         pub const name = "XMLHttpRequest";
         pub const prototype_chain = bridge.prototypeChain();
         pub var class_id: bridge.ClassId = undefined;
+        pub const weak = true;
         pub const finalizer = bridge.finalizer(XMLHttpRequest.deinit);
     };