Skip to content

Commit 0959eea

Browse files
committed
Remove the loop
Previously, the IO loop was doing three things: 1 - Managing timeouts (either from scripts or for our own needs) 2 - Handling browser IO events (page/script/xhr) 3 - Handling CDP events (accept, read, write, timeout) With the libcurl merge, 1 was moved to an in-process scheduler and 2 was moved to libcurl's own event loop. That means the entire loop code, including the dependency on tigerbeetle-io existed for handling a single TCP client. Not only is that a lot of code, there was also friction between the two loops (the libcurl one and our IO loop), which would result in latency - while one loop is waiting for the events, any events on the other loop go un-processed. This PR removes our IO loop. To accomplish this: 1 - The main accept loop is blocking. This is simpler and works perfectly well, given we only allow 1 active connection. 2 - The client socket is passed to libcurl - yes, libcurl's loop can take arbitrary FDs and poll them along with its own. In addition to having one less dependency, the CDP code is quite a bit simpler, especially around shutdowns and writes. This also removes _some_ of the latency caused by the friction between page process and CDP processing. Specifically, when CDP now blocks for input, http page events (script loading, xhr, ...) will still be processed. There's still friction. For one, the reverse isn't true: when the page is waiting for events, CDP events aren't going to be processed. But the page.wait already have some sensitivity to this (e.g. the page.request_intercepted flag). Also, when CDP waits, while we will process network events, page timeouts are still not processed. Because of both these remaining issues, we still need to jump between the two loops - but being able to block on CDP (even for a short time) WITHOUT stopping the page's network I/O, should reduce some latency.
1 parent 390a21e commit 0959eea

File tree

15 files changed

+266
-828
lines changed

15 files changed

+266
-828
lines changed

build.zig

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,6 @@ fn addDependencies(b: *Build, mod: *Build.Module, opts: *Build.Step.Options) !vo
153153
.optimize = mod.optimize.?,
154154
};
155155

156-
mod.addImport("tigerbeetle-io", b.dependency("tigerbeetle_io", .{}).module("tigerbeetle_io"));
157-
158156
mod.addIncludePath(b.path("vendor/lightpanda"));
159157

160158
{

build.zig.zon

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,10 @@
44
.version = "0.0.0",
55
.fingerprint = 0xda130f3af836cea0,
66
.dependencies = .{
7-
.tigerbeetle_io = .{
8-
.url = "https://github.com/lightpanda-io/tigerbeetle-io/archive/19ae89eb3814d48c202ac9e0495fc5cadb29dfe7.tar.gz",
9-
.hash = "tigerbeetle_io-0.0.0-ViLgxjqSBADhuHO_RZm4yNzuoKDXWP39hDn60Kht40OC",
10-
},
117
.v8 = .{
128
.url = "https://github.com/lightpanda-io/zig-v8-fork/archive/cf412d5b3d9d608582571d821e0d552337ef690d.tar.gz",
139
.hash = "v8-0.0.0-xddH69zDAwA4fp1dBo_jEDjS5bhXycPwRlZHp6_X890t",
1410
},
1511
//.v8 = .{ .path = "../zig-v8-fork" },
16-
//.tigerbeetle_io = .{ .path = "../tigerbeetle-io" },
1712
},
1813
}

src/app.zig

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const Allocator = std.mem.Allocator;
44

55
const log = @import("log.zig");
66
const Http = @import("http/Http.zig");
7-
const Loop = @import("runtime/loop.zig").Loop;
87
const Platform = @import("runtime/js.zig").Platform;
98

109
const Telemetry = @import("telemetry/telemetry.zig").Telemetry;
@@ -14,7 +13,6 @@ const Notification = @import("notification.zig").Notification;
1413
// might need.
1514
pub const App = struct {
1615
http: Http,
17-
loop: *Loop,
1816
config: Config,
1917
platform: ?*const Platform,
2018
allocator: Allocator,
@@ -45,12 +43,6 @@ pub const App = struct {
4543
const app = try allocator.create(App);
4644
errdefer allocator.destroy(app);
4745

48-
const loop = try allocator.create(Loop);
49-
errdefer allocator.destroy(loop);
50-
51-
loop.* = try Loop.init(allocator);
52-
errdefer loop.deinit();
53-
5446
const notification = try Notification.init(allocator, null);
5547
errdefer notification.deinit();
5648

@@ -68,7 +60,6 @@ pub const App = struct {
6860
const app_dir_path = getAndMakeAppDir(allocator);
6961

7062
app.* = .{
71-
.loop = loop,
7263
.http = http,
7364
.allocator = allocator,
7465
.telemetry = undefined,
@@ -92,8 +83,6 @@ pub const App = struct {
9283
allocator.free(app_dir_path);
9384
}
9485
self.telemetry.deinit();
95-
self.loop.deinit();
96-
allocator.destroy(self.loop);
9786
self.notification.deinit();
9887
self.http.deinit();
9988
allocator.destroy(self);

src/browser/Scheduler.zig

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,15 +59,15 @@ pub fn add(self: *Scheduler, ctx: *anyopaque, func: Task.Func, ms: u32, opts: Ad
5959
});
6060
}
6161

62-
pub fn runHighPriority(self: *Scheduler) !?u32 {
62+
pub fn runHighPriority(self: *Scheduler) !?i32 {
6363
return self.runQueue(&self.primary);
6464
}
6565

66-
pub fn runLowPriority(self: *Scheduler) !?u32 {
66+
pub fn runLowPriority(self: *Scheduler) !?i32 {
6767
return self.runQueue(&self.secondary);
6868
}
6969

70-
fn runQueue(self: *Scheduler, queue: *Queue) !?u32 {
70+
fn runQueue(self: *Scheduler, queue: *Queue) !?i32 {
7171
// this is O(1)
7272
if (queue.count() == 0) {
7373
return null;

src/browser/ScriptManager.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ pub fn blockingGet(self: *ScriptManager, url: [:0]const u8) !BlockingResult {
317317

318318
// rely on http's timeout settings to avoid an endless/long loop.
319319
while (true) {
320-
try client.tick(200);
320+
_ = try client.tick(.{ .timeout_ms = 200 });
321321
switch (blocking.state) {
322322
.running => {},
323323
.done => |result| return result,

src/browser/page.zig

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,6 @@ const polyfill = @import("polyfill/polyfill.zig");
5151
pub const Page = struct {
5252
cookie_jar: *storage.CookieJar,
5353

54-
// Pre-configured http/cilent.zig used to make HTTP requests.
55-
// @newhttp
56-
// request_factory: RequestFactory,
57-
5854
session: *Session,
5955

6056
// An arena with a lifetime for the entire duration of the page
@@ -146,12 +142,9 @@ pub const Page = struct {
146142
.scheduler = Scheduler.init(arena),
147143
.keydown_event_node = .{ .func = keydownCallback },
148144
.window_clicked_event_node = .{ .func = windowClicked },
149-
// @newhttp
150-
// .request_factory = browser.http_client.requestFactory(.{
151-
// .notification = browser.notification,
152-
// }),
153145
.main_context = undefined,
154146
};
147+
155148
self.main_context = try session.executor.createJsContext(&self.window, self, self, true, Env.GlobalMissingCallback.init(&self.polyfill_loader));
156149
try polyfill.preload(self.arena, self.main_context);
157150

@@ -269,7 +262,7 @@ pub const Page = struct {
269262
return self.script_manager.blockingGet(src);
270263
}
271264

272-
pub fn wait(self: *Page, wait_sec: usize) void {
265+
pub fn wait(self: *Page, wait_sec: u16) void {
273266
self._wait(wait_sec) catch |err| switch (err) {
274267
error.JsError => {}, // already logged (with hopefully more context)
275268
else => {
@@ -283,9 +276,9 @@ pub const Page = struct {
283276
};
284277
}
285278

286-
fn _wait(self: *Page, wait_sec: usize) !void {
287-
var ms_remaining = wait_sec * 1000;
279+
fn _wait(self: *Page, wait_sec: u16) !void {
288280
var timer = try std.time.Timer.start();
281+
var ms_remaining: i32 = @intCast(wait_sec * 1000);
289282

290283
var try_catch: Env.TryCatch = undefined;
291284
try_catch.init(self.main_context);
@@ -320,7 +313,7 @@ pub const Page = struct {
320313
}
321314

322315
// There should only be 1 active http transfer, the main page
323-
try http_client.tick(ms_remaining);
316+
_ = try http_client.tick(.{ .timeout_ms = ms_remaining });
324317
},
325318
.html, .parsed => {
326319
// The HTML page was parsed. We now either have JS scripts to
@@ -381,7 +374,7 @@ pub const Page = struct {
381374
// inflight requests
382375
else @min(ms_remaining, ms_to_next_task orelse 1000);
383376

384-
try http_client.tick(ms_to_wait);
377+
_ = try http_client.tick(.{ .timeout_ms = ms_to_wait });
385378

386379
if (request_intercepted) {
387380
// Again, proritizing intercepted requests. Exit this
@@ -401,7 +394,7 @@ pub const Page = struct {
401394
if (ms_elapsed >= ms_remaining) {
402395
return;
403396
}
404-
ms_remaining -= ms_elapsed;
397+
ms_remaining -= @intCast(ms_elapsed);
405398
}
406399
}
407400

src/browser/session.zig

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ pub const Session = struct {
137137
return &(self.page orelse return null);
138138
}
139139

140-
pub fn wait(self: *Session, wait_sec: usize) void {
140+
pub fn wait(self: *Session, wait_sec: u16) void {
141141
if (self.queued_navigation) |qn| {
142142
// This was already aborted on the page, but it would be pretty
143143
// bad if old requests went to the new page, so let's make double sure

src/cdp/cdp.zig

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,9 @@ pub fn CDPT(comptime TypeProvider: type) type {
114114
}
115115

116116
// @newhttp
117-
// A bit hacky right now. The main server loop blocks only for CDP
118-
// messages. It no longer blocks for page timeouts of page HTTP
119-
// transfers. So we need to call this more ourselves.
120-
// This is called after every message and [very hackily] from the server
121-
// loop.
122-
// This is hopefully temporary.
117+
// A bit hacky right now. The main server loop doesn't unblock for
118+
// scheduled task. So we run this directly in order to process any
119+
// timeouts (or http events) which are ready to be processed.
123120
pub fn pageWait(self: *Self) void {
124121
const session = &(self.browser.session orelse return);
125122
// exits early if there's nothing to do, so a large value like
@@ -592,8 +589,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
592589
};
593590

594591
const cdp = self.cdp;
595-
var arena = std.heap.ArenaAllocator.init(cdp.allocator);
596-
errdefer arena.deinit();
592+
const allocator = cdp.client.send_arena.allocator();
597593

598594
const field = ",\"sessionId\":\"";
599595

@@ -602,7 +598,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
602598
const message_len = msg.len + session_id.len + 1 + field.len + 10;
603599

604600
var buf: std.ArrayListUnmanaged(u8) = .{};
605-
buf.ensureTotalCapacity(arena.allocator(), message_len) catch |err| {
601+
buf.ensureTotalCapacity(allocator, message_len) catch |err| {
606602
log.err(.cdp, "inspector buffer", .{ .err = err });
607603
return;
608604
};
@@ -617,7 +613,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
617613
buf.appendSliceAssumeCapacity("\"}");
618614
std.debug.assert(buf.items.len == message_len);
619615

620-
try cdp.client.sendJSONRaw(arena, buf);
616+
try cdp.client.sendJSONRaw(buf);
621617
}
622618
};
623619
}

src/cdp/testing.zig

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,14 @@ pub const Document = @import("../testing.zig").Document;
3939

4040
const Client = struct {
4141
allocator: Allocator,
42+
send_arena: ArenaAllocator,
4243
sent: std.ArrayListUnmanaged(json.Value) = .{},
4344
serialized: std.ArrayListUnmanaged([]const u8) = .{},
4445

4546
fn init(alloc: Allocator) Client {
4647
return .{
4748
.allocator = alloc,
49+
.send_arena = ArenaAllocator.init(alloc),
4850
};
4951
}
5052

@@ -58,7 +60,7 @@ const Client = struct {
5860
try self.sent.append(self.allocator, value);
5961
}
6062

61-
pub fn sendJSONRaw(self: *Client, _: ArenaAllocator, buf: std.ArrayListUnmanaged(u8)) !void {
63+
pub fn sendJSONRaw(self: *Client, buf: std.ArrayListUnmanaged(u8)) !void {
6264
const value = try json.parseFromSliceLeaky(json.Value, self.allocator, buf.items, .{});
6365
try self.sent.append(self.allocator, value);
6466
}

src/http/Client.zig

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const CookieJar = @import("../browser/storage/storage.zig").CookieJar;
2727
const urlStitch = @import("../url.zig").stitch;
2828

2929
const c = Http.c;
30+
const posix = std.posix;
3031

3132
const Allocator = std.mem.Allocator;
3233
const ArenaAllocator = std.heap.ArenaAllocator;
@@ -167,22 +168,24 @@ pub fn abort(self: *Client) void {
167168
}
168169
}
169170

170-
pub fn tick(self: *Client, timeout_ms: usize) !void {
171-
var handles = &self.handles;
171+
const TickOpts = struct {
172+
timeout_ms: i32 = 0,
173+
poll_socket: ?posix.socket_t = null,
174+
};
175+
pub fn tick(self: *Client, opts: TickOpts) !bool {
172176
while (true) {
173-
if (handles.hasAvailable() == false) {
177+
if (self.handles.hasAvailable() == false) {
174178
break;
175179
}
176180
const queue_node = self.queue.popFirst() orelse break;
177181
const req = queue_node.data;
178182
self.queue_node_pool.destroy(queue_node);
179183

180184
// we know this exists, because we checked isEmpty() above
181-
const handle = handles.getFreeHandle().?;
185+
const handle = self.handles.getFreeHandle().?;
182186
try self.makeRequest(handle, req);
183187
}
184-
185-
try self.perform(@intCast(timeout_ms));
188+
return self.perform(opts.timeout_ms, opts.poll_socket);
186189
}
187190

188191
pub fn request(self: *Client, req: Request) !void {
@@ -343,16 +346,26 @@ fn makeRequest(self: *Client, handle: *Handle, transfer: *Transfer) !void {
343346
}
344347

345348
self.active += 1;
346-
return self.perform(0);
349+
_ = try self.perform(0, null);
347350
}
348351

349-
fn perform(self: *Client, timeout_ms: c_int) !void {
352+
fn perform(self: *Client, timeout_ms: c_int, socket: ?posix.socket_t) !bool {
350353
const multi = self.multi;
351-
352354
var running: c_int = undefined;
353355
try errorMCheck(c.curl_multi_perform(multi, &running));
354356

355-
if (running > 0 and timeout_ms > 0) {
357+
if (socket) |s| {
358+
var wait_fd = c.curl_waitfd{
359+
.fd = s,
360+
.events = c.CURL_WAIT_POLLIN,
361+
.revents = 0,
362+
};
363+
try errorMCheck(c.curl_multi_poll(multi, &wait_fd, 1, timeout_ms, null));
364+
if (wait_fd.revents != 0) {
365+
// the extra socket we passed in is ready, let's signal our caller
366+
return true;
367+
}
368+
} else if (running > 0 and timeout_ms > 0) {
356369
try errorMCheck(c.curl_multi_poll(multi, null, 0, timeout_ms, null));
357370
}
358371

@@ -388,6 +401,8 @@ fn perform(self: *Client, timeout_ms: c_int) !void {
388401
self.requestFailed(transfer, err);
389402
}
390403
}
404+
405+
return false;
391406
}
392407

393408
fn endTransfer(self: *Client, transfer: *Transfer) void {

0 commit comments

Comments
 (0)