From 193e012aa6a7da63bafc1cf779a31b4d42d7eb59 Mon Sep 17 00:00:00 2001 From: sjorsdonkers <72333389+sjorsdonkers@users.noreply.github.com> Date: Wed, 21 May 2025 13:58:42 +0200 Subject: [PATCH 1/7] Rename to ExecutionWorlds --- src/browser/session.zig | 4 ++-- src/cdp/cdp.zig | 14 +++++--------- src/cdp/domains/dom.zig | 4 ++-- src/cdp/domains/page.zig | 6 +++--- src/runtime/js.zig | 35 +++++++++++++++++++---------------- src/runtime/testing.zig | 4 ++-- src/testing.zig | 4 ++-- 7 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/browser/session.zig b/src/browser/session.zig index ddcc138e..972bff81 100644 --- a/src/browser/session.zig +++ b/src/browser/session.zig @@ -49,14 +49,14 @@ pub const Session = struct { // page and start another. transfer_arena: Allocator, - executor: Env.Executor, + executor: Env.ExecutionWorld, storage_shed: storage.Shed, cookie_jar: storage.CookieJar, page: ?Page = null, pub fn init(self: *Session, browser: *Browser) !void { - var executor = try browser.env.newExecutor(); + var executor = try browser.env.newExecutionWorld(); errdefer executor.deinit(); const allocator = browser.app.allocator; diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 66980723..ef8377f8 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -372,12 +372,11 @@ pub fn BrowserContext(comptime CDP_T: type) type { return error.CurrentlyOnly1IsolatedWorldSupported; } - var executor = try self.cdp.browser.env.newExecutor(); + var executor = try self.cdp.browser.env.newExecutionWorld(); errdefer executor.deinit(); self.isolated_world = .{ .name = try self.arena.dupe(u8, world_name), - .scope = null, .executor = executor, .grant_universal_access = grant_universal_access, }; @@ -511,18 +510,15 @@ pub fn BrowserContext(comptime CDP_T: type) type { /// An object id is unique across all contexts, different object ids can refer to the same Node in different contexts. const IsolatedWorld = struct { name: []const u8, - scope: ?*Env.Scope, - executor: Env.Executor, + executor: Env.ExecutionWorld, grant_universal_access: bool, pub fn deinit(self: *IsolatedWorld) void { self.executor.deinit(); - self.scope = null; } pub fn removeContext(self: *IsolatedWorld) !void { - if (self.scope == null) return error.NoIsolatedContextToRemove; + if (self.executor.scope == null) return error.NoIsolatedContextToRemove; self.executor.endScope(); - self.scope = null; } // The isolate world must share at least some of the state with the related page, specifically the DocumentHTML @@ -531,8 +527,8 @@ const IsolatedWorld = struct { // This also means this pointer becomes invalid after removePage untill a new page is created. // Currently we have only 1 page/frame and thus also only 1 state in the isolate world. pub fn createContext(self: *IsolatedWorld, page: *Page) !void { - if (self.scope != null) return error.Only1IsolatedContextSupported; - self.scope = try self.executor.startScope(&page.window, &page.state, {}, false); + if (self.executor.scope != null) return error.Only1IsolatedContextSupported; + _ = try self.executor.startScope(&page.window, &page.state, {}, false); } }; diff --git a/src/cdp/domains/dom.zig b/src/cdp/domains/dom.zig index 14b89343..53d8e19a 100644 --- a/src/cdp/domains/dom.zig +++ b/src/cdp/domains/dom.zig @@ -262,8 +262,8 @@ fn resolveNode(cmd: anytype) !void { var scope = page.scope; if (params.executionContextId) |context_id| { if (scope.context.debugContextId() != context_id) { - const isolated_world = bc.isolated_world orelse return error.ContextNotFound; - scope = isolated_world.scope orelse return error.ContextNotFound; + var isolated_world = bc.isolated_world orelse return error.ContextNotFound; + scope = &(isolated_world.executor.scope orelse return error.ContextNotFound); if (scope.context.debugContextId() != context_id) return error.ContextNotFound; } diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 75e10695..6ec1a1b1 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -115,7 +115,7 @@ fn createIsolatedWorld(cmd: anytype) !void { const world = try bc.createIsolatedWorld(params.worldName, params.grantUniveralAccess); const page = bc.session.currentPage() orelse return error.PageNotLoaded; try pageCreated(bc, page); - const scope = world.scope.?; + const scope = &world.executor.scope.?; // Create the auxdata json for the contextCreated event // Calling contextCreated will assign a Id to the context and send the contextCreated event @@ -236,7 +236,7 @@ pub fn pageNavigate(bc: anytype, event: *const Notification.PageNavigate) !void const aux_json = try std.fmt.bufPrint(&buffer, "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\"}}", .{target_id}); // Calling contextCreated will assign a new Id to the context and send the contextCreated event bc.inspector.contextCreated( - isolated_world.scope.?, + &isolated_world.executor.scope.?, isolated_world.name, "://", aux_json, @@ -258,7 +258,7 @@ pub fn pageCreated(bc: anytype, page: *Page) !void { try isolated_world.createContext(page); const polyfill = @import("../../browser/polyfill/polyfill.zig"); - try polyfill.load(bc.arena, isolated_world.scope.?); + try polyfill.load(bc.arena, &isolated_world.executor.scope.?); } } diff --git a/src/runtime/js.zig b/src/runtime/js.zig index 51960b10..37fe113e 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -53,8 +53,8 @@ pub const Platform = struct { // The Env maps to a V8 isolate, which represents a isolated sandbox for // executing JavaScript. The Env is where we'll define our V8 <-> Zig bindings, -// and it's where we'll start Executors, which actually execute JavaScript. -// The `S` parameter is arbitrary state. When we start an Executor, an instance +// and it's where we'll start ExecutionWorlds, which actually execute JavaScript. +// The `S` parameter is arbitrary state. When we start an ExecutionWorld, an instance // of S must be given. This instance is available to any Zig binding. // The `types` parameter is a tuple of Zig structures we want to bind to V8. pub fn Env(comptime State: type, comptime WebApis: type) type { @@ -259,7 +259,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { self.isolate.performMicrotasksCheckpoint(); } - pub fn newExecutor(self: *Self) !Executor { + pub fn newExecutionWorld(self: *Self) !ExecutionWorld { return .{ .env = self, .scope = null, @@ -280,32 +280,35 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { self.isolate.lowMemoryNotification(); } - pub const Executor = struct { + // ExecutionWorld closely models a JS World. + // https://chromium.googlesource.com/chromium/src/+/master/third_party/blink/renderer/bindings/core/v8/V8BindingDesign.md#World + // https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/scripting/ExecutionWorld + pub const ExecutionWorld = struct { env: *Self, // Arena whose lifetime is for a single getter/setter/function/etc. // Largely used to get strings out of V8, like a stack trace from // a TryCatch. The allocator will be owned by the Scope, but the - // arena itself is owned by the Executor so that we can re-use it + // arena itself is owned by the ExecutionWorld so that we can re-use it // from scope to scope. call_arena: ArenaAllocator, // Arena whose lifetime is for a single page load, aka a Scope. Where // the call_arena lives for a single function call, the scope_arena // lives for the lifetime of the entire page. The allocator will be - // owned by the Scope, but the arena itself is owned by the Executor + // owned by the Scope, but the arena itself is owned by the ExecutionWorld // so that we can re-use it from scope to scope. scope_arena: ArenaAllocator, // A Scope maps to a Browser's Page. Here though, it's only a - // mechanism to organization page-specific memory. The Executor + // mechanism to organization page-specific memory. The ExecutionWorld // does all the work, but having all page-specific data structures // grouped together helps keep things clean. scope: ?Scope = null, - // no init, must be initialized via env.newExecutor() + // no init, must be initialized via env.newExecutionWorld() - pub fn deinit(self: *Executor) void { + pub fn deinit(self: *ExecutionWorld) void { if (self.scope != null) { self.endScope(); } @@ -320,7 +323,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // when the handle_scope is freed. // We also maintain our own "scope_arena" which allows us to have // all page related memory easily managed. - pub fn startScope(self: *Executor, global: anytype, state: State, module_loader: anytype, enter: bool) !*Scope { + pub fn startScope(self: *ExecutionWorld, global: anytype, state: State, module_loader: anytype, enter: bool) !*Scope { std.debug.assert(self.scope == null); const ModuleLoader = switch (@typeInfo(@TypeOf(module_loader))) { @@ -338,9 +341,9 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { const Global = @TypeOf(global.*); var context: v8.Context = blk: { - var handle_scope: v8.HandleScope = undefined; - v8.HandleScope.init(&handle_scope, isolate); - defer handle_scope.deinit(); + var temp_scope: v8.HandleScope = undefined; + v8.HandleScope.init(&temp_scope, isolate); + defer temp_scope.deinit(); const js_global = v8.FunctionTemplate.initDefault(isolate); attachClass(Global, isolate, js_global); @@ -466,7 +469,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { return scope; } - pub fn endScope(self: *Executor) void { + pub fn endScope(self: *ExecutionWorld) void { self.scope.?.deinit(); self.scope = null; _ = self.scope_arena.reset(.{ .retain_with_limit = SCOPE_ARENA_RETAIN }); @@ -1517,7 +1520,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { } // Retrieves the RemoteObject for a given value. - // The value is loaded through the Executor's mapZigInstanceToJs function, + // The value is loaded through the ExecutionWorld's mapZigInstanceToJs function, // just like a method return value. Therefore, if we've mapped this // value before, we'll get the existing JS PersistedObject and if not // we'll create it and track it for cleanup when the scope ends. @@ -2198,7 +2201,7 @@ fn isEmpty(comptime T: type) bool { } // Responsible for calling Zig functions from JS invokations. This could -// probably just contained in Executor, but having this specific logic, which +// probably just contained in ExecutionWorld, but having this specific logic, which // is somewhat repetitive between constructors, functions, getters, etc contained // here does feel like it makes it clenaer. fn Caller(comptime E: type, comptime State: type) type { diff --git a/src/runtime/testing.zig b/src/runtime/testing.zig index abd5768a..c51298c0 100644 --- a/src/runtime/testing.zig +++ b/src/runtime/testing.zig @@ -30,7 +30,7 @@ pub fn Runner(comptime State: type, comptime Global: type, comptime types: anyty return struct { env: *Env, scope: *Env.Scope, - executor: Env.Executor, + executor: Env.ExecutionWorld, pub const Env = js.Env(State, struct { pub const Interfaces = AdjustedTypes; @@ -45,7 +45,7 @@ pub fn Runner(comptime State: type, comptime Global: type, comptime types: anyty self.env = try Env.init(allocator, .{}); errdefer self.env.deinit(); - self.executor = try self.env.newExecutor(); + self.executor = try self.env.newExecutionWorld(); errdefer self.executor.deinit(); self.scope = try self.executor.startScope( diff --git a/src/testing.zig b/src/testing.zig index 7db16d65..72692489 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -383,7 +383,7 @@ pub const JsRunner = struct { renderer: Renderer, http_client: HttpClient, scope: *Env.Scope, - executor: Env.Executor, + executor: Env.ExecutionWorld, storage_shelf: storage.Shelf, cookie_jar: storage.CookieJar, @@ -435,7 +435,7 @@ pub const JsRunner = struct { .tls_verify_host = false, }); - self.executor = try self.env.newExecutor(); + self.executor = try self.env.newExecutionWorld(); errdefer self.executor.deinit(); self.scope = try self.executor.startScope(&self.window, &self.state, {}, true); From a720333c0f1e32a164abc73e73f1825266f79390 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 22 May 2025 15:28:07 +0800 Subject: [PATCH 2/7] Add debug log level to make build-dev and add new make run-debug Update WPT submodule, now includes xhr/formdata tests. --- Makefile | 6 +++++- tests/wpt | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 4fb54cda..4f8ab60b 100644 --- a/Makefile +++ b/Makefile @@ -69,7 +69,7 @@ build: ## Build in debug mode build-dev: @printf "\e[36mBuilding (debug)...\e[0m\n" - @$(ZIG) build -Dgit_commit=$$(git rev-parse --short HEAD) || (printf "\e[33mBuild ERROR\e[0m\n"; exit 1;) + @$(ZIG) build -Dgit_commit=$$(git rev-parse --short HEAD) -Dlog_level=debug || (printf "\e[33mBuild ERROR\e[0m\n"; exit 1;) @printf "\e[33mBuild OK\e[0m\n" ## Run the server in debug mode @@ -77,6 +77,10 @@ run: build @printf "\e[36mRunning...\e[0m\n" @./zig-out/bin/lightpanda || (printf "\e[33mRun ERROR\e[0m\n"; exit 1;) +run-debug: build-dev + @printf "\e[36mRunning...\e[0m\n" + @./zig-out/bin/lightpanda || (printf "\e[33mRun ERROR\e[0m\n"; exit 1;) + ## Run a JS shell in debug mode shell: @printf "\e[36mBuilding shell...\e[0m\n" diff --git a/tests/wpt b/tests/wpt index ea16a1e3..3a362d3f 160000 --- a/tests/wpt +++ b/tests/wpt @@ -1 +1 @@ -Subproject commit ea16a1e361daf4fa68a18b50cd006e90651bf03a +Subproject commit 3a362d3f23ee7809e2a0f6ddfe2648e462998403 From 72a983f6d8401a777562d4d249010aa99eba5f57 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 22 May 2025 15:36:55 +0800 Subject: [PATCH 3/7] Apply suggestions from code review Co-authored-by: Pierre Tachoire --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 4f8ab60b..e4b36707 100644 --- a/Makefile +++ b/Makefile @@ -72,11 +72,12 @@ build-dev: @$(ZIG) build -Dgit_commit=$$(git rev-parse --short HEAD) -Dlog_level=debug || (printf "\e[33mBuild ERROR\e[0m\n"; exit 1;) @printf "\e[33mBuild OK\e[0m\n" -## Run the server in debug mode +## Run the server in release mode run: build @printf "\e[36mRunning...\e[0m\n" @./zig-out/bin/lightpanda || (printf "\e[33mRun ERROR\e[0m\n"; exit 1;) +## Run the server in debug mode run-debug: build-dev @printf "\e[36mRunning...\e[0m\n" @./zig-out/bin/lightpanda || (printf "\e[33mRun ERROR\e[0m\n"; exit 1;) From ca0f407b7bacba0b66f15be54902953568cda677 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 22 May 2025 16:45:06 +0800 Subject: [PATCH 4/7] include OS in libc_v8 lib path --- .github/actions/install/action.yml | 10 +++++----- Dockerfile | 6 +++--- build.zig | 29 ++++++++++++++++++++++------- build.zig.zon | 4 ++-- 4 files changed, 32 insertions(+), 17 deletions(-) diff --git a/.github/actions/install/action.yml b/.github/actions/install/action.yml index 58ff9b11..38019982 100644 --- a/.github/actions/install/action.yml +++ b/.github/actions/install/action.yml @@ -17,7 +17,7 @@ inputs: zig-v8: description: 'zig v8 version to install' required: false - default: 'v0.1.23' + default: 'v0.1.24' v8: description: 'v8 version to install' required: false @@ -59,11 +59,11 @@ runs: - name: install v8 shell: bash run: | - mkdir -p v8/out/debug/obj/zig/ - ln -s ${{ inputs.cache-dir }}/v8/libc_v8.a v8/out/debug/obj/zig/libc_v8.a + mkdir -p v8/out/${{ inputs.os }}/debug/obj/zig/ + ln -s ${{ inputs.cache-dir }}/v8/libc_v8.a v8/out/${{ inputs.os }}/debug/obj/zig/libc_v8.a - mkdir -p v8/out/release/obj/zig/ - ln -s ${{ inputs.cache-dir }}/v8/libc_v8.a v8/out/release/obj/zig/libc_v8.a + mkdir -p v8/out/${{ inputs.os }}/release/obj/zig/ + ln -s ${{ inputs.cache-dir }}/v8/libc_v8.a v8/out/${{ inputs.os }}/release/obj/zig/libc_v8.a - name: libiconv shell: bash diff --git a/Dockerfile b/Dockerfile index 70d609d1..98cb8709 100644 --- a/Dockerfile +++ b/Dockerfile @@ -5,7 +5,7 @@ ARG ZIG=0.14.0 ARG ZIG_MINISIG=RWSGOq2NVecA2UPNdBUZykf1CCb147pkmdtYxgb3Ti+JO/wCYvhbAb/U ARG ARCH=x86_64 ARG V8=11.1.134 -ARG ZIG_V8=v0.1.23 +ARG ZIG_V8=v0.1.24 RUN apt-get update -yq && \ apt-get install -yq xz-utils \ @@ -57,8 +57,8 @@ RUN make install-libiconv && \ # download and install v8 RUN curl --fail -L -o libc_v8.a https://github.com/lightpanda-io/zig-v8-fork/releases/download/${ZIG_V8}/libc_v8_${V8}_linux_${ARCH}.a && \ - mkdir -p v8/build/${ARCH}-linux/release/ninja/obj/zig/ && \ - mv libc_v8.a v8/build/${ARCH}-linux/release/ninja/obj/zig/libc_v8.a + mkdir -p v8/out/linux/release/obj/zig/ && \ + mv libc_v8.a v8/out/linux/release/obj/zig/libc_v8.a # build release RUN make build diff --git a/build.zig b/build.zig index 50b01afe..1def42dc 100644 --- a/build.zig +++ b/build.zig @@ -170,13 +170,29 @@ fn common(b: *std.Build, opts: *std.Build.Step.Options, step: *std.Build.Step.Co mod.addImport("v8", v8_mod); } - const lib_path = try std.fmt.allocPrint( - mod.owner.allocator, - "v8/out/{s}/obj/zig/libc_v8.a", - .{if (mod.optimize.? == .Debug) "debug" else "release"}, - ); mod.link_libcpp = true; - mod.addObjectFile(mod.owner.path(lib_path)); + + { + const release_dir = if (mod.optimize.? == .Debug) "debug" else "release"; + const os = switch (target.result.os.tag) { + .linux => "linux", + .macos => "macos", + else => return error.UnsupportedPlatform, + }; + var lib_path = try std.fmt.allocPrint(mod.owner.allocator, + "v8/out/{s}/{s}/obj/zig/libc_v8.a", + .{os, release_dir}, + ); + std.fs.cwd().access(lib_path, .{}) catch { + // legacy path + lib_path = try std.fmt.allocPrint(mod.owner.allocator, + "v8/out/{s}/obj/zig/libc_v8.a", + .{release_dir}, + ); + }; + mod.addObjectFile(mod.owner.path(lib_path)); + } + switch (target.result.os.tag) { .macos => { @@ -188,7 +204,6 @@ fn common(b: *std.Build, opts: *std.Build.Step.Options, step: *std.Build.Step.Co } mod.addImport("build_config", opts.createModule()); - mod.addObjectFile(mod.owner.path(lib_path)); } fn moduleNetSurf(b: *std.Build, step: *std.Build.Step.Compile, target: std.Build.ResolvedTarget) !void { diff --git a/build.zig.zon b/build.zig.zon index b3701d90..bed23173 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -13,8 +13,8 @@ .hash = "tigerbeetle_io-0.0.0-ViLgxpyRBAB5BMfIcj3KMXfbJzwARs9uSl8aRy2OXULd", }, .v8 = .{ - .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/240140e5b3e5a8e5e51cbdd36bc120bf28ae5c31.tar.gz", - .hash = "v8-0.0.0-xddH64eyAwBcX7e2x5tv9MhT0MgQbshP2rb19blo06Db", + .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/e38cb27ddb044c6afbf8a938b293721b9804405e.tar.gz", + .hash = "v8-0.0.0-xddH6_GzAwCaz83JWuw3sepOGq0I7C_CmfOwA1Gb9q3y", }, //.v8 = .{ .path = "../zig-v8-fork" }, //.tigerbeetle_io = .{ .path = "../tigerbeetle-io" }, From f4a27af37e4c3c92cf5a3bd065d35587d5677575 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 22 May 2025 16:58:29 +0800 Subject: [PATCH 5/7] zig fmt build.zig --- build.zig | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/build.zig b/build.zig index 1def42dc..de15ab9a 100644 --- a/build.zig +++ b/build.zig @@ -179,13 +179,15 @@ fn common(b: *std.Build, opts: *std.Build.Step.Options, step: *std.Build.Step.Co .macos => "macos", else => return error.UnsupportedPlatform, }; - var lib_path = try std.fmt.allocPrint(mod.owner.allocator, + var lib_path = try std.fmt.allocPrint( + mod.owner.allocator, "v8/out/{s}/{s}/obj/zig/libc_v8.a", - .{os, release_dir}, + .{ os, release_dir }, ); std.fs.cwd().access(lib_path, .{}) catch { // legacy path - lib_path = try std.fmt.allocPrint(mod.owner.allocator, + lib_path = try std.fmt.allocPrint( + mod.owner.allocator, "v8/out/{s}/obj/zig/libc_v8.a", .{release_dir}, ); @@ -193,7 +195,6 @@ fn common(b: *std.Build, opts: *std.Build.Step.Options, step: *std.Build.Step.Co mod.addObjectFile(mod.owner.path(lib_path)); } - switch (target.result.os.tag) { .macos => { // v8 has a dependency, abseil-cpp, which, on Mac, uses CoreFoundation From 589fa4c9dec429c9f0a567733cb29a06f3448304 Mon Sep 17 00:00:00 2001 From: Pierre Tachoire Date: Thu, 22 May 2025 14:44:57 +0200 Subject: [PATCH 6/7] README: rsync is used to get v8 sources --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e912887d..08a93327 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Fast web automation for AI agents, LLM training, scraping and testing: _Puppeteer requesting 100 pages from a local website on a AWS EC2 m5.large instance. See [benchmark details](https://github.com/lightpanda-io/demo)._ -[^1]: **Playwright support disclaimer:** +[^1]: **Playwright support disclaimer:** Due to the nature of Playwright, a script that works with the current version of the browser may not function correctly with a future version. Playwright uses an intermediate JavaScript layer that selects an execution strategy based on the browser's available features. If Lightpanda adds a new [Web API](https://developer.mozilla.org/en-US/docs/Web/API), Playwright may choose to execute different code for the same script. This new code path could attempt to use features that are not yet implemented. Lightpanda makes an effort to add compatibility tests, but we can't cover all scenarios. If you encounter an issue, please create a [GitHub issue](https://github.com/lightpanda-io/browser/issues) and include the last known working version of the script. ## Quick start @@ -164,7 +164,7 @@ For Debian/Ubuntu based Linux: sudo apt install xz-utils \ python3 ca-certificates git \ pkg-config libglib2.0-dev \ - gperf libexpat1-dev unzip \ + gperf libexpat1-dev unzip rsync \ cmake clang ``` From b1d0368479f3d3ce6f1e2c2800095b2d0ea6c099 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 23 May 2025 14:15:55 +0800 Subject: [PATCH 7/7] Remove --gc_hints option, apply the --gc_hints behavior by default --- .github/workflows/e2e-test.yml | 2 +- src/app.zig | 1 - src/browser/browser.zig | 4 +--- src/main.zig | 23 ----------------------- src/runtime/js.zig | 7 +++---- 5 files changed, 5 insertions(+), 32 deletions(-) diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index 4c1c0dfe..3be3ddbe 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -88,7 +88,7 @@ jobs: - name: run puppeteer run: | python3 -m http.server 1234 -d ./public & echo $! > PYTHON.pid - ./lightpanda serve --gc_hints & echo $! > LPD.pid + ./lightpanda serve & echo $! > LPD.pid RUNS=100 npm run bench-puppeteer-cdp > puppeteer.out || exit 1 cat /proc/`cat LPD.pid`/status |grep VmHWM|grep -oP '\d+' > LPD.VmHWM kill `cat LPD.pid` `cat PYTHON.pid` diff --git a/src/app.zig b/src/app.zig index 260d8e52..d4e05a5c 100644 --- a/src/app.zig +++ b/src/app.zig @@ -28,7 +28,6 @@ pub const App = struct { pub const Config = struct { run_mode: RunMode, - gc_hints: bool = false, tls_verify_host: bool = true, http_proxy: ?std.Uri = null, }; diff --git a/src/browser/browser.zig b/src/browser/browser.zig index 9c57b76b..0a4db620 100644 --- a/src/browser/browser.zig +++ b/src/browser/browser.zig @@ -86,9 +86,7 @@ pub const Browser = struct { session.deinit(); self.session = null; _ = self.session_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 }); - if (self.app.config.gc_hints) { - self.env.lowMemoryNotification(); - } + self.env.lowMemoryNotification(); } } diff --git a/src/main.zig b/src/main.zig index f5eb4086..8cc5e538 100644 --- a/src/main.zig +++ b/src/main.zig @@ -70,7 +70,6 @@ pub fn main() !void { var app = try App.init(alloc, .{ .run_mode = args.mode, - .gc_hints = args.gcHints(), .http_proxy = args.httpProxy(), .tls_verify_host = args.tlsVerifyHost(), }); @@ -129,13 +128,6 @@ const Command = struct { mode: Mode, exec_name: []const u8, - fn gcHints(self: *const Command) bool { - return switch (self.mode) { - .serve => |opts| opts.gc_hints, - else => false, - }; - } - fn tlsVerifyHost(self: *const Command) bool { return switch (self.mode) { inline .serve, .fetch => |opts| opts.tls_verify_host, @@ -161,7 +153,6 @@ const Command = struct { host: []const u8, port: u16, timeout: u16, - gc_hints: bool, tls_verify_host: bool, http_proxy: ?std.Uri, }; @@ -210,9 +201,6 @@ const Command = struct { \\--timeout Inactivity timeout in seconds before disconnecting clients \\ Defaults to 3 (seconds) \\ - \\--gc_hints Encourage V8 to cleanup garbage for each new browser context. - \\ Defaults to false - \\ \\--insecure_disable_tls_host_verification \\ Disables host verification on all HTTP requests. \\ This is an advanced option which should only be @@ -296,10 +284,6 @@ fn inferMode(opt: []const u8) ?App.RunMode { return .serve; } - if (std.mem.eql(u8, opt, "--gc_hints")) { - return .serve; - } - return null; } @@ -310,7 +294,6 @@ fn parseServeArgs( var host: []const u8 = "127.0.0.1"; var port: u16 = 9222; var timeout: u16 = 3; - var gc_hints = false; var tls_verify_host = true; var http_proxy: ?std.Uri = null; @@ -355,11 +338,6 @@ fn parseServeArgs( continue; } - if (std.mem.eql(u8, "--gc_hints", opt)) { - gc_hints = true; - continue; - } - if (std.mem.eql(u8, "--http_proxy", opt)) { const str = args.next() orelse { log.err("--http_proxy argument requires an value", .{}); @@ -377,7 +355,6 @@ fn parseServeArgs( .host = host, .port = port, .timeout = timeout, - .gc_hints = gc_hints, .http_proxy = http_proxy, .tls_verify_host = tls_verify_host, }; diff --git a/src/runtime/js.zig b/src/runtime/js.zig index 37fe113e..0c855393 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -269,10 +269,9 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { } // V8 doesn't immediately free memory associated with - // a Context, it's managed by the garbage collector. So, when the - // `gc_hints` option is enabled, we'll use the `lowMemoryNotification` - // call on the isolate to encourage v8 to free any contexts which - // have been freed. + // a Context, it's managed by the garbage collector. We use the + // `lowMemoryNotification` call on the isolate to encourage v8 to free + // any contexts which have been freed. pub fn lowMemoryNotification(self: *Self) void { var handle_scope: v8.HandleScope = undefined; v8.HandleScope.init(&handle_scope, self.isolate);