Merge pull request #3131 from lightpanda-io/gc-hint-heuristics

perf: Reduce memory pressure notification to v8

src/browser/Runner.zig

@@ -118,10 +118,9 @@ fn _wait(self: *Runner, comptime is_cdp: bool, timeout_ms: u32, conditions: []Wa
     const timer: std.Io.Timestamp = .now(io, .boot);
 
     // Periodic V8 GC hint during long waits. V8 is otherwise only nudged on
-    // session/page teardown (Browser.zig, Page.zig), so a page that stays
+    // session/page teardown (Session.zig, Page.zig), so a page that stays
     // alive for seconds while running heavy JS accumulates wrappers and
-    // external-ref'd Zig allocations V8 has no reason to drop. `.moderate`
-    // speeds up incremental GC without stalling the tick.
+    // external-ref'd Zig allocations V8 has no reason to drop.
     const gc_hint_period_ns: u64 = std.time.ns_per_s * 5;
     var gc_hint_timer: std.Io.Timestamp = .now(io, .boot);
 

src/browser/js/Env.zig

@@ -41,6 +41,8 @@ const Allocator = std.mem.Allocator;
 
 const MAX_CONTEXTS = if (lp.build_config.wpt_extensions) 8192 else 128;
 
+const GC_HINT_FLOOR = 1 * 1024 * 1024;
+
 fn initClassIds() void {
     inline for (JsApis, 0..) |JsApi, i| {
         JsApi.Meta.class_id = i;
@@ -512,10 +514,12 @@ pub fn runIdleTasks(self: *const Env) void {
 // a Context, it's managed by the garbage collector. We use the
 // `memoryPressureNotification` call on the isolate to encourage v8 to free
 // any contexts which have been freed.
-// The level indicates the aggressivity of the GC required:
-// moderate speeds up incremental GC
-// critical runs one full GC
+// Skips if there's little to reclaim
 pub fn memoryPressureNotification(self: *Env, level: Isolate.MemoryPressureLevel) void {
+    const stats = self.isolate.getHeapStatistics();
+    if (stats.used_heap_size + stats.external_memory < GC_HINT_FLOOR) {
+        return;
+    }
     var handle_scope: js.HandleScope = undefined;
     handle_scope.init(self.isolate);
     defer handle_scope.deinit();

src/browser/webapi/element/html/Custom.zig

@@ -265,17 +265,18 @@ pub fn checkAndAttachBuiltIn(element: *Element, frame: *Frame) !void {
     // (2) called from both V8 callbacks (Local exists) and parser (no Local).
     // Prefer either: requiring *const js.Local parameter, OR always creating
     // Local.Scope upfront.
-    var ls: ?js.Local.Scope = null;
-    var local = blk: {
+    var ls: js.Local.Scope = undefined;
+    var ls_open = false;
+    const local = blk: {
         if (frame.js.local) |l| {
             break :blk l;
         }
-        ls = undefined;
-        frame.js.localScope(&ls.?);
-        break :blk &ls.?.local;
+        frame.js.localScope(&ls);
+        ls_open = true;
+        break :blk &ls.local;
     };
-    defer if (ls) |*_ls| {
-        _ls.deinit();
+    defer if (ls_open) {
+        ls.deinit();
     };
 
     var caught: js.TryCatch.Caught = undefined;

src/cdp/domains/dom.zig

@@ -346,32 +346,33 @@ fn resolveNode(cmd: *CDP.Command) !void {
     const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
     const frame = bc.mainFrame() orelse return error.FrameNotLoaded;
 
-    var ls: ?js.Local.Scope = null;
-    defer if (ls) |*_ls| {
-        _ls.deinit();
+    var ls: js.Local.Scope = undefined;
+    var ls_open = false;
+    defer if (ls_open) {
+        ls.deinit();
     };
 
     if (params.executionContextId) |context_id| blk: {
-        ls = undefined;
-        frame.js.localScope(&ls.?);
-        if (ls.?.local.debugContextId() == context_id) {
+        frame.js.localScope(&ls);
+        ls_open = true;
+        if (ls.local.debugContextId() == context_id) {
             break :blk;
         }
         // not the default scope, check the other ones
         for (bc.isolated_worlds.items) |isolated_world| {
-            ls.?.deinit();
-            ls = null;
+            ls.deinit();
+            ls_open = false;
 
             const ctx = (isolated_world.context orelse return error.ContextNotFound);
-            ls = undefined;
-            ctx.localScope(&ls.?);
-            if (ls.?.local.debugContextId() == context_id) {
+            ctx.localScope(&ls);
+            ls_open = true;
+            if (ls.local.debugContextId() == context_id) {
                 break :blk;
             }
         } else return error.ContextNotFound;
     } else {
-        ls = undefined;
-        frame.js.localScope(&ls.?);
+        frame.js.localScope(&ls);
+        ls_open = true;
     }
 
     const input_node_id = params.nodeId orelse params.backendNodeId orelse return error.InvalidParam;
@@ -380,7 +381,7 @@ fn resolveNode(cmd: *CDP.Command) !void {
     // node._node is a *DOMNode we need this to be able to find its most derived type e.g. Node -> Element -> HTMLElement
     // So we use the Node.Union when retrieve the value from the environment
     const remote_object = try bc.inspector_session.getRemoteObject(
-        &ls.?.local,
+        &ls.local,
         params.objectGroup orelse "",
         node.dom,
     );