From ba8a0179d57ab567c61e5e9f22ed69b239de8472 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 19 Mar 2025 10:53:26 +0800 Subject: [PATCH 1/2] Share the HTTP client globally --- src/app.zig | 40 +++++++++++++++++++++++------------- src/browser/browser.zig | 7 +++---- src/cdp/testing.zig | 4 ++-- src/main.zig | 4 ++-- src/telemetry/lightpanda.zig | 22 ++++++++++---------- src/telemetry/telemetry.zig | 39 +++++++++++++++++++---------------- src/testing.zig | 23 ++++++++++++++------- src/unit_tests.zig | 3 +-- 8 files changed, 82 insertions(+), 60 deletions(-) diff --git a/src/app.zig b/src/app.zig index f64c88e4..94883b65 100644 --- a/src/app.zig +++ b/src/app.zig @@ -2,24 +2,29 @@ const std = @import("std"); const Loop = @import("jsruntime").Loop; const Allocator = std.mem.Allocator; +const HttpClient = @import("http/Client.zig"); const Telemetry = @import("telemetry/telemetry.zig").Telemetry; const log = std.log.scoped(.app); -pub const RunMode = enum { - serve, - fetch, -}; - // Container for global state / objects that various parts of the system // might need. pub const App = struct { loop: *Loop, - app_dir_path: ?[]const u8, allocator: Allocator, telemetry: Telemetry, + http_client: HttpClient, + app_dir_path: ?[]const u8, + + pub const RunMode = enum { + serve, + fetch, + }; + + pub fn init(allocator: Allocator, run_mode: RunMode) !*App { + const app = try allocator.create(App); + errdefer allocator.destroy(app); - pub fn init(allocator: Allocator, run_mode: RunMode) !App { const loop = try allocator.create(Loop); errdefer allocator.destroy(loop); @@ -27,29 +32,36 @@ pub const App = struct { errdefer loop.deinit(); const app_dir_path = getAndMakeAppDir(allocator); - const telemetry = Telemetry.init(allocator, run_mode, app_dir_path); - errdefer telemetry.deinit(); - return .{ + app.* = .{ .loop = loop, .allocator = allocator, - .telemetry = telemetry, + .telemetry = undefined, .app_dir_path = app_dir_path, + .http_client = .{ .allocator = allocator }, }; + app.telemetry = Telemetry.init(app, run_mode); + + return app; } pub fn deinit(self: *App) void { + const allocator = self.allocator; if (self.app_dir_path) |app_dir_path| { - self.allocator.free(app_dir_path); + allocator.free(app_dir_path); } - self.telemetry.deinit(); self.loop.deinit(); - self.allocator.destroy(self.loop); + allocator.destroy(self.loop); + self.http_client.deinit(); + allocator.destroy(self); } }; fn getAndMakeAppDir(allocator: Allocator) ?[]const u8 { + if (@import("builtin").is_test) { + return allocator.dupe(u8, "/tmp") catch unreachable; + } const app_dir_path = std.fs.getAppDataDir(allocator, "lightpanda") catch |err| { log.warn("failed to get lightpanda data dir: {}", .{err}); return null; diff --git a/src/browser/browser.zig b/src/browser/browser.zig index 2469c94f..776845b8 100644 --- a/src/browser/browser.zig +++ b/src/browser/browser.zig @@ -63,7 +63,7 @@ pub const Browser = struct { app: *App, session: ?*Session, allocator: Allocator, - http_client: HttpClient, + http_client: *HttpClient, session_pool: SessionPool, page_arena: std.heap.ArenaAllocator, @@ -75,7 +75,7 @@ pub const Browser = struct { .app = app, .session = null, .allocator = allocator, - .http_client = .{ .allocator = allocator }, + .http_client = @ptrCast(&app.http_client), .session_pool = SessionPool.init(allocator), .page_arena = std.heap.ArenaAllocator.init(allocator), }; @@ -83,7 +83,6 @@ pub const Browser = struct { pub fn deinit(self: *Browser) void { self.closeSession(); - self.http_client.deinit(); self.session_pool.deinit(); self.page_arena.deinit(); } @@ -454,7 +453,7 @@ pub const Page = struct { // replace the user context document with the new one. try session.env.setUserContext(.{ .document = html_doc, - .httpClient = &self.session.browser.http_client, + .httpClient = self.session.browser.http_client, }); // browse the DOM tree to retrieve scripts diff --git a/src/cdp/testing.zig b/src/cdp/testing.zig index 5bc5f029..9002f754 100644 --- a/src/cdp/testing.zig +++ b/src/cdp/testing.zig @@ -118,7 +118,7 @@ const TestCDP = main.CDPT(struct { }); const TestContext = struct { - app: App, + app: *App, client: ?Client = null, cdp_: ?TestCDP = null, arena: std.heap.ArenaAllocator, @@ -136,7 +136,7 @@ const TestContext = struct { self.client = Client.init(self.arena.allocator()); // Don't use the arena here. We want to detect leaks in CDP. // The arena is only for test-specific stuff - self.cdp_ = TestCDP.init(&self.app, &self.client.?); + self.cdp_ = TestCDP.init(self.app, &self.client.?); } return &self.cdp_.?; } diff --git a/src/main.zig b/src/main.zig index a7d27f6f..d606dca1 100644 --- a/src/main.zig +++ b/src/main.zig @@ -75,7 +75,7 @@ pub fn main() !void { app.telemetry.record(.{ .run = {} }); const timeout = std.time.ns_per_s * @as(u64, opts.timeout); - server.run(&app, address, timeout) catch |err| { + server.run(app, address, timeout) catch |err| { log.err("Server error", .{}); return err; }; @@ -92,7 +92,7 @@ pub fn main() !void { defer vm.deinit(); // browser - var browser = Browser.init(&app); + var browser = Browser.init(app); defer browser.deinit(); var session = try browser.newSession({}); diff --git a/src/telemetry/lightpanda.zig b/src/telemetry/lightpanda.zig index 6dba26d0..4a54e34d 100644 --- a/src/telemetry/lightpanda.zig +++ b/src/telemetry/lightpanda.zig @@ -5,8 +5,8 @@ const build_info = @import("build_info"); const Thread = std.Thread; const Allocator = std.mem.Allocator; +const App = @import("../app.zig").App; const telemetry = @import("telemetry.zig"); -const RunMode = @import("../app.zig").RunMode; const log = std.log.scoped(.telemetry); const URL = "https://telemetry.lightpanda.io"; @@ -19,11 +19,13 @@ pub const LightPanda = struct { allocator: Allocator, mutex: std.Thread.Mutex, cond: Thread.Condition, + client: *std.http.Client, node_pool: std.heap.MemoryPool(List.Node), const List = std.DoublyLinkedList(LightPandaEvent); - pub fn init(allocator: Allocator) !LightPanda { + pub fn init(app: *App) !LightPanda { + const allocator = app.allocator; return .{ .cond = .{}, .mutex = .{}, @@ -31,6 +33,7 @@ pub const LightPanda = struct { .thread = null, .running = true, .allocator = allocator, + .client = @ptrCast(&app.http_client), .uri = std.Uri.parse(URL) catch unreachable, .node_pool = std.heap.MemoryPool(List.Node).init(allocator), }; @@ -47,7 +50,7 @@ pub const LightPanda = struct { self.node_pool.deinit(); } - pub fn send(self: *LightPanda, iid: ?[]const u8, run_mode: RunMode, raw_event: telemetry.Event) !void { + pub fn send(self: *LightPanda, iid: ?[]const u8, run_mode: App.RunMode, raw_event: telemetry.Event) !void { const event = LightPandaEvent{ .iid = iid, .mode = run_mode, @@ -57,7 +60,7 @@ pub const LightPanda = struct { self.mutex.lock(); defer self.mutex.unlock(); if (self.thread == null) { - self.thread = try std.Thread.spawn(.{}, run, .{self}); + self.thread = try std.Thread.spawn(.{ .stack_size = 1024 * 1024 * 4 }, run, .{self}); } const node = try self.node_pool.create(); @@ -68,19 +71,16 @@ pub const LightPanda = struct { } fn run(self: *LightPanda) void { + const client = self.client; var arr: std.ArrayListUnmanaged(u8) = .{}; - var client = std.http.Client{ .allocator = self.allocator }; - defer { - arr.deinit(self.allocator); - client.deinit(); - } + defer arr.deinit(self.allocator); self.mutex.lock(); while (true) { while (self.pending.popFirst()) |node| { self.mutex.unlock(); - self.postEvent(&node.data, &client, &arr) catch |err| { + self.postEvent(&node.data, client, &arr) catch |err| { log.warn("Telementry reporting error: {}", .{err}); }; self.mutex.lock(); @@ -113,7 +113,7 @@ pub const LightPanda = struct { const LightPandaEvent = struct { iid: ?[]const u8, - mode: RunMode, + mode: App.RunMode, event: telemetry.Event, pub fn jsonStringify(self: *const LightPandaEvent, writer: anytype) !void { diff --git a/src/telemetry/telemetry.zig b/src/telemetry/telemetry.zig index fd6d0447..126f7ab0 100644 --- a/src/telemetry/telemetry.zig +++ b/src/telemetry/telemetry.zig @@ -3,9 +3,9 @@ const builtin = @import("builtin"); const Allocator = std.mem.Allocator; +const App = @import("../app.zig").App; const Loop = @import("jsruntime").Loop; const uuidv4 = @import("../id.zig").uuidv4; -const RunMode = @import("../app.zig").RunMode; const log = std.log.scoped(.telemetry); const IID_FILE = "iid"; @@ -25,11 +25,11 @@ fn TelemetryT(comptime P: type) type { disabled: bool, - run_mode: RunMode, + run_mode: App.RunMode, const Self = @This(); - pub fn init(allocator: Allocator, run_mode: RunMode, app_dir_path: ?[]const u8) Self { + pub fn init(app: *App, run_mode: App.RunMode) Self { const disabled = std.process.hasEnvVarConstant("LIGHTPANDA_DISABLE_TELEMETRY"); if (builtin.mode != .Debug and builtin.is_test == false) { log.info("telemetry {s}", .{if (disabled) "disabled" else "enabled"}); @@ -38,8 +38,8 @@ fn TelemetryT(comptime P: type) type { return .{ .disabled = disabled, .run_mode = run_mode, - .provider = try P.init(allocator), - .iid = if (disabled) null else getOrCreateId(app_dir_path), + .provider = try P.init(app), + .iid = if (disabled) null else getOrCreateId(app.app_dir_path), }; } @@ -104,32 +104,32 @@ pub const Event = union(enum) { }; const NoopProvider = struct { - fn init(_: Allocator) !NoopProvider { + fn init(_: *App) !NoopProvider { return .{}; } fn deinit(_: NoopProvider) void {} - pub fn send(_: NoopProvider, _: ?[]const u8, _: RunMode, _: Event) !void {} + pub fn send(_: NoopProvider, _: ?[]const u8, _: App.RunMode, _: Event) !void {} }; extern fn setenv(name: [*:0]u8, value: [*:0]u8, override: c_int) c_int; extern fn unsetenv(name: [*:0]u8) c_int; -const testing = std.testing; +const testing = @import("../testing.zig"); test "telemetry: disabled by environment" { _ = setenv(@constCast("LIGHTPANDA_DISABLE_TELEMETRY"), @constCast(""), 0); defer _ = unsetenv(@constCast("LIGHTPANDA_DISABLE_TELEMETRY")); const FailingProvider = struct { - fn init(_: Allocator) !@This() { + fn init(_: *App) !@This() { return .{}; } fn deinit(_: @This()) void {} - pub fn send(_: @This(), _: ?[]const u8, _: RunMode, _: Event) !void { + pub fn send(_: @This(), _: ?[]const u8, _: App.RunMode, _: Event) !void { unreachable; } }; - var telemetry = TelemetryT(FailingProvider).init(testing.allocator, .serve, null); + var telemetry = TelemetryT(FailingProvider).init(undefined, .serve); defer telemetry.deinit(); telemetry.record(.{ .run = {} }); } @@ -141,7 +141,7 @@ test "telemetry: getOrCreateId" { const id1 = getOrCreateId("/tmp/").?; const id2 = getOrCreateId("/tmp/").?; - try testing.expectEqualStrings(&id1, &id2); + try testing.expectEqual(&id1, &id2); std.fs.cwd().deleteFile("/tmp/" ++ IID_FILE) catch {}; const id3 = getOrCreateId("/tmp/").?; @@ -149,7 +149,10 @@ test "telemetry: getOrCreateId" { } test "telemetry: sends event to provider" { - var telemetry = TelemetryT(MockProvider).init(testing.allocator, .serve, "/tmp/"); + var app = testing.app(.{}); + defer app.deinit(); + + var telemetry = TelemetryT(MockProvider).init(app, .serve); defer telemetry.deinit(); const mock = &telemetry.provider; @@ -165,28 +168,28 @@ test "telemetry: sends event to provider" { const MockProvider = struct { iid: ?[]const u8, - run_mode: ?RunMode, + run_mode: ?App.RunMode, allocator: Allocator, events: std.ArrayListUnmanaged(Event), - fn init(allocator: Allocator) !@This() { + fn init(app: *App) !@This() { return .{ .iid = null, .run_mode = null, .events = .{}, - .allocator = allocator, + .allocator = app.allocator, }; } fn deinit(self: *MockProvider) void { self.events.deinit(self.allocator); } - pub fn send(self: *MockProvider, iid: ?[]const u8, run_mode: RunMode, events: Event) !void { + pub fn send(self: *MockProvider, iid: ?[]const u8, run_mode: App.RunMode, events: Event) !void { if (self.iid == null) { try testing.expectEqual(null, self.run_mode); self.iid = iid.?; self.run_mode = run_mode; } else { - try testing.expectEqualStrings(self.iid.?, iid.?); + try testing.expectEqual(self.iid.?, iid.?); try testing.expectEqual(self.run_mode.?, run_mode); } try self.events.append(self.allocator, events); diff --git a/src/testing.zig b/src/testing.zig index f0b3f6e0..2a2c61e0 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -4,6 +4,8 @@ pub const allocator = std.testing.allocator; pub const expectError = std.testing.expectError; pub const expectString = std.testing.expectEqualStrings; +const App = @import("app.zig").App; + // Merged std.testing.expectEqual and std.testing.expectString // can be useful when testing fields of an anytype an you don't know // exactly how to assert equality @@ -12,12 +14,14 @@ pub fn expectEqual(expected: anytype, actual: anytype) !void { .Array => |arr| if (arr.child == u8) { return std.testing.expectEqualStrings(expected, &actual); }, - .Pointer => |ptr| if (ptr.child == u8) { - return std.testing.expectEqualStrings(expected, actual); - } else if (comptime isStringArray(ptr.child)) { - return std.testing.expectEqualStrings(expected, actual); - } else if (ptr.child == []u8 or ptr.child == []const u8) { - return expectString(expected, actual); + .Pointer => |ptr| { + if (ptr.child == u8) { + return std.testing.expectEqualStrings(expected, actual); + } else if (comptime isStringArray(ptr.child)) { + return std.testing.expectEqualStrings(expected, actual); + } else if (ptr.child == []u8 or ptr.child == []const u8) { + return expectString(expected, actual); + } }, .Struct => |structType| { inline for (structType.fields) |field| { @@ -92,7 +96,7 @@ pub fn expectDelta(expected: anytype, actual: anytype, delta: anytype) !void { } fn isStringArray(comptime T: type) bool { - if (!is(.array)(T) and !isPtrTo(.array)(T)) { + if (!is(.Array)(T) and !isPtrTo(.Array)(T)) { return false; } return std.meta.Elem(T) == u8; @@ -132,3 +136,8 @@ pub fn print(comptime fmt: []const u8, args: anytype) void { std.debug.print(fmt, args); } } + +// dummy opts incase we want to add something, and not have to break all the callers +pub fn app(_: anytype) *App { + return App.init(allocator, .serve) catch unreachable; +} diff --git a/src/unit_tests.zig b/src/unit_tests.zig index 1cc9975f..56fdacad 100644 --- a/src/unit_tests.zig +++ b/src/unit_tests.zig @@ -26,7 +26,6 @@ const App = @import("app.zig").App; const jsruntime = @import("jsruntime"); pub const Types = jsruntime.reflect(@import("generate.zig").Tuple(.{}){}); pub const UserContext = @import("user_context.zig").UserContext; -// pub const IO = @import("asyncio").Wrapper(jsruntime.Loop); pub const std_options = std.Options{ .log_level = .err, @@ -69,7 +68,7 @@ pub fn main() !void { const cdp_thread = blk: { const address = try std.net.Address.parseIp("127.0.0.1", 9583); const thread = try std.Thread.spawn(.{}, serveCDP, .{ - &app, + app, address, }); break :blk thread; From 705603a0880d9b974532c0925a6c7c684ed24ebf Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 19 Mar 2025 16:17:41 +0800 Subject: [PATCH 2/2] remove explicit thread stack size. The real win is having a global http_client, so the thread only needs a pointer. --- src/telemetry/lightpanda.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/telemetry/lightpanda.zig b/src/telemetry/lightpanda.zig index 4a54e34d..49fcc1b4 100644 --- a/src/telemetry/lightpanda.zig +++ b/src/telemetry/lightpanda.zig @@ -60,7 +60,7 @@ pub const LightPanda = struct { self.mutex.lock(); defer self.mutex.unlock(); if (self.thread == null) { - self.thread = try std.Thread.spawn(.{ .stack_size = 1024 * 1024 * 4 }, run, .{self}); + self.thread = try std.Thread.spawn(.{}, run, .{self}); } const node = try self.node_pool.create();