Skip to content

Commit 96b10f4

Browse files
karlseguinkarlseguin
committed
Optimize Network.responseReceived
Add a header iterator to the transfer. This removes the need for NetworkState, duping header name/values, and the http_header_received event.
1 parent 5100e06 commit 96b10f4

File tree

5 files changed

+81
-80
lines changed

5 files changed

+81
-80
lines changed

‎src/cdp/cdp.zig‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ pub fn CDPT(comptime TypeProvider: type) type {
7777
// Extra headers to add to all requests. TBD under which conditions this should be reset.
7878
extra_headers: std.ArrayListUnmanaged([*c]const u8) = .empty,
7979

80-
network_state: NetworkState,
8180
intercept_state: InterceptState,
8281

8382
const Self = @This();
@@ -94,7 +93,6 @@ pub fn CDPT(comptime TypeProvider: type) type {
9493
.browser_context = null,
9594
.message_arena = std.heap.ArenaAllocator.init(allocator),
9695
.notification_arena = std.heap.ArenaAllocator.init(allocator),
97-
.network_state = try NetworkState.init(allocator),
9896
.intercept_state = try InterceptState.init(allocator), // TBD or browser session arena?
9997
};
10098
}
@@ -104,7 +102,6 @@ pub fn CDPT(comptime TypeProvider: type) type {
104102
bc.deinit();
105103
}
106104
self.intercept_state.deinit(); // TBD Should this live in BC?
107-
self.network_state.deinit();
108105
self.browser.deinit();
109106
self.message_arena.deinit();
110107
self.notification_arena.deinit();
@@ -451,15 +448,13 @@ pub fn BrowserContext(comptime CDP_T: type) type {
451448
pub fn networkEnable(self: *Self) !void {
452449
try self.cdp.browser.notification.register(.http_request_fail, self, onHttpRequestFail);
453450
try self.cdp.browser.notification.register(.http_request_start, self, onHttpRequestStart);
454-
try self.cdp.browser.notification.register(.http_header_received, self, onHttpHeaderReceived);
455-
try self.cdp.browser.notification.register(.http_headers_done_receiving, self, onHttpHeadersDoneReceiving);
451+
try self.cdp.browser.notification.register(.http_headers_done, self, onHttpHeadersDone);
456452
}
457453

458454
pub fn networkDisable(self: *Self) void {
459455
self.cdp.browser.notification.unregister(.http_request_fail, self);
460456
self.cdp.browser.notification.unregister(.http_request_start, self);
461-
self.cdp.browser.notification.unregister(.http_header_received, self);
462-
self.cdp.browser.notification.unregister(.http_headers_done_receiving, self);
457+
self.cdp.browser.notification.unregister(.http_headers_done, self);
463458
}
464459

465460
pub fn fetchEnable(self: *Self) !void {
@@ -473,7 +468,6 @@ pub fn BrowserContext(comptime CDP_T: type) type {
473468
pub fn onPageRemove(ctx: *anyopaque, _: Notification.PageRemove) !void {
474469
const self: *Self = @alignCast(@ptrCast(ctx));
475470
try @import("domains/page.zig").pageRemove(self);
476-
try @import("domains/network.zig").pageRemove(self);
477471
}
478472

479473
pub fn onPageCreated(ctx: *anyopaque, page: *Page) !void {
@@ -504,22 +498,16 @@ pub fn BrowserContext(comptime CDP_T: type) type {
504498
try @import("domains/fetch.zig").requestPaused(self.notification_arena, self, data);
505499
}
506500

507-
pub fn onHttpHeaderReceived(ctx: *anyopaque, data: *const Notification.ResponseHeader) !void {
508-
const self: *Self = @alignCast(@ptrCast(ctx));
509-
defer self.resetNotificationArena();
510-
try self.cdp.network_state.putOrAppendReceivedHeader(data.request_id, data.status, data.header);
511-
}
512-
513501
pub fn onHttpRequestFail(ctx: *anyopaque, data: *const Notification.RequestFail) !void {
514502
const self: *Self = @alignCast(@ptrCast(ctx));
515503
defer self.resetNotificationArena();
516504
return @import("domains/network.zig").httpRequestFail(self.notification_arena, self, data);
517505
}
518506

519-
pub fn onHttpHeadersDoneReceiving(ctx: *anyopaque, data: *const Notification.ResponseHeadersDone) !void {
507+
pub fn onHttpHeadersDone(ctx: *anyopaque, data: *const Notification.ResponseHeadersDone) !void {
520508
const self: *Self = @alignCast(@ptrCast(ctx));
521509
defer self.resetNotificationArena();
522-
return @import("domains/network.zig").httpHeadersDoneReceiving(self.notification_arena, self, data);
510+
return @import("domains/network.zig").httpHeadersDone(self.notification_arena, self, data);
523511
}
524512

525513
fn resetNotificationArena(self: *Self) void {

‎src/cdp/domains/network.zig‎

Lines changed: 26 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const Allocator = std.mem.Allocator;
2222
const Notification = @import("../../notification.zig").Notification;
2323
const log = @import("../../log.zig");
2424
const CdpStorage = @import("storage.zig");
25+
const Transfer = @import("../../http/Client.zig").Transfer;
2526

2627
pub fn processMessage(cmd: anytype) !void {
2728
const action = std.meta.stringToEnum(enum {
@@ -61,43 +62,6 @@ const Response = struct {
6162
// be loaded with each chunks as Network.dataReceiveds are coming in.
6263
};
6364

64-
// Stored in CDP
65-
pub const NetworkState = struct {
66-
arena: std.heap.ArenaAllocator,
67-
received: std.AutoArrayHashMap(u64, Response),
68-
69-
pub fn init(allocator: Allocator) !NetworkState {
70-
return .{
71-
.arena = std.heap.ArenaAllocator.init(allocator),
72-
.received = std.AutoArrayHashMap(u64, Response).init(allocator),
73-
};
74-
}
75-
76-
pub fn deinit(self: *NetworkState) void {
77-
self.received.deinit();
78-
self.arena.deinit();
79-
}
80-
81-
pub fn putOrAppendReceivedHeader(self: *NetworkState, request_id: u64, status: u16, header: std.http.Header) !void {
82-
const kv = try self.received.getOrPut(request_id);
83-
if (!kv.found_existing) {
84-
kv.value_ptr.* = .{ .status = status };
85-
}
86-
87-
const a = self.arena.allocator();
88-
const name = try a.dupe(u8, header.name);
89-
const value = try a.dupe(u8, header.value);
90-
try kv.value_ptr.headers.put(a, name, value);
91-
}
92-
};
93-
94-
pub fn pageRemove(bc: anytype) !void {
95-
// The main page is going to be removed
96-
const state = &bc.cdp.network_state;
97-
state.received.clearRetainingCapacity(); // May need to be in pageRemoved
98-
_ = state.arena.reset(.{ .retain_with_limit = 1024 });
99-
}
100-
10165
fn enable(cmd: anytype) !void {
10266
const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
10367
try bc.networkEnable();
@@ -314,7 +278,7 @@ pub fn httpRequestStart(arena: Allocator, bc: anytype, data: *const Notification
314278
}, .{ .session_id = session_id });
315279
}
316280

317-
pub fn httpHeadersDoneReceiving(arena: Allocator, bc: anytype, request: *const Notification.ResponseHeadersDone) !void {
281+
pub fn httpHeadersDone(arena: Allocator, bc: anytype, request: *const Notification.ResponseHeadersDone) !void {
318282
// Isn't possible to do a network request within a Browser (which our
319283
// notification is tied to), without a page.
320284
std.debug.assert(bc.session.page != null);
@@ -333,17 +297,17 @@ pub fn httpHeadersDoneReceiving(arena: Allocator, bc: anytype, request: *const N
333297
.query = true,
334298
});
335299

336-
const response = bc.cdp.network_state.received.get(request.transfer.id) orelse return error.ResponseNotFound;
300+
const status = request.transfer.response_header.?.status;
337301

338302
// We're missing a bunch of fields, but, for now, this seems like enough
339303
try cdp.sendEvent("Network.responseReceived", .{
340304
.requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{request.transfer.id}),
341305
.loaderId = bc.loader_id,
342306
.response = .{
343307
.url = url,
344-
.status = response.status,
345-
.statusText = @as(std.http.Status, @enumFromInt(response.status)).phrase() orelse "Unknown",
346-
.headers = std.json.ArrayHashMap([]const u8){ .map = response.headers },
308+
.status = status,
309+
.statusText = @as(std.http.Status, @enumFromInt(status)).phrase() orelse "Unknown",
310+
.headers = ResponseHeaderWriter.init(request.transfer),
347311
},
348312
.frameId = target_id,
349313
}, .{ .session_id = session_id });
@@ -355,6 +319,26 @@ pub fn urlToString(arena: Allocator, url: *const std.Uri, opts: std.Uri.WriteToS
355319
return buf.items;
356320
}
357321

322+
const ResponseHeaderWriter = struct {
323+
transfer: *Transfer,
324+
325+
fn init(transfer: *Transfer) ResponseHeaderWriter {
326+
return .{
327+
.transfer = transfer,
328+
};
329+
}
330+
331+
pub fn jsonStringify(self: *const ResponseHeaderWriter, writer: anytype) !void {
332+
try writer.beginObject();
333+
var it = self.transfer.responseHeaderIterator();
334+
while (it.next()) |hdr| {
335+
try writer.objectField(hdr.name);
336+
try writer.write(hdr.value);
337+
}
338+
try writer.endObject();
339+
}
340+
};
341+
358342
const testing = @import("../testing.zig");
359343
test "cdp.network setExtraHTTPHeaders" {
360344
var ctx = testing.context();

‎src/http/Client.zig‎

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -668,14 +668,20 @@ pub const Transfer = struct {
668668
}
669669

670670
if (buf_len == 2) {
671+
if (getResponseHeader(easy, "content-type")) |value| {
672+
const len = @min(value.len, hdr._content_type.len);
673+
hdr._content_type_len = len;
674+
@memcpy(hdr._content_type[0..len], value[0..len]);
675+
}
676+
671677
transfer.req.header_done_callback(transfer) catch |err| {
672678
log.err(.http, "header_done_callback", .{ .err = err, .req = transfer });
673679
// returning < buf_len terminates the request
674680
return 0;
675681
};
676682

677683
if (transfer.client.notification) |notification| {
678-
notification.dispatch(.http_headers_done_receiving, &.{
684+
notification.dispatch(.http_headers_done, &.{
679685
.transfer = transfer,
680686
});
681687
}
@@ -686,16 +692,6 @@ pub const Transfer = struct {
686692
return 0;
687693
};
688694
}
689-
690-
if (transfer.client.notification) |notification| {
691-
if (Http.Headers.parseHeader(header)) |hdr_name_value| {
692-
notification.dispatch(.http_header_received, &.{
693-
.request_id = transfer.id,
694-
.status = hdr.status,
695-
.header = hdr_name_value,
696-
});
697-
} else log.err(.http, "invalid header", .{ .line = header });
698-
}
699695
}
700696
return buf_len;
701697
}
@@ -721,6 +717,12 @@ pub const Transfer = struct {
721717
return chunk_len;
722718
}
723719

720+
// we assume that the caller is smart and only calling this after being
721+
// told that the header was ready.
722+
pub fn responseHeaderIterator(self: *Transfer) HeaderIterator {
723+
return .{ .easy = self._handle.?.conn.easy };
724+
}
725+
724726
// pub because Page.printWaitAnalysis uses it
725727
pub fn fromEasy(easy: *c.CURL) !*Transfer {
726728
var private: *anyopaque = undefined;
@@ -742,3 +744,37 @@ pub const Header = struct {
742744
return self._content_type[0..self._content_type_len];
743745
}
744746
};
747+
748+
const HeaderIterator = struct {
749+
easy: *c.CURL,
750+
prev: ?*c.curl_header = null,
751+
752+
pub fn next(self: *HeaderIterator) ?struct { name: []const u8, value: []const u8 } {
753+
const h = c.curl_easy_nextheader(self.easy, c.CURLH_HEADER, -1, self.prev) orelse return null;
754+
self.prev = h;
755+
756+
const header = h.*;
757+
return .{
758+
.name = std.mem.span(header.name),
759+
.value = std.mem.span(header.value),
760+
};
761+
}
762+
};
763+
764+
fn getResponseHeader(easy: *c.CURL, name: [:0]const u8) ?[]const u8 {
765+
var hdr: [*c]c.curl_header = null;
766+
const result = c.curl_easy_header(easy, name, 0, c.CURLH_HEADER, -1, &hdr);
767+
if (result == c.CURLE_OK) {
768+
return std.mem.span(hdr.*.value);
769+
}
770+
771+
if (result == c.CURLE_FAILED_INIT) {
772+
// seems to be what it returns if the header isn't found
773+
return null;
774+
}
775+
log.err(.http, "get response header", .{
776+
.name = name,
777+
.err = @import("errors.zig").fromCode(result),
778+
});
779+
return null;
780+
}

‎src/http/Http.zig‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ const std = @import("std");
2121
pub const c = @cImport({
2222
@cInclude("curl/curl.h");
2323
});
24-
const errors = @import("errors.zig");
24+
2525
const Client = @import("Client.zig");
26+
const errors = @import("errors.zig");
2627

2728
const Allocator = std.mem.Allocator;
2829
const ArenaAllocator = std.heap.ArenaAllocator;

‎src/notification.zig‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,7 @@ pub const Notification = struct {
6363
http_request_fail: List = .{},
6464
http_request_start: List = .{},
6565
http_request_intercept: List = .{},
66-
http_header_received: List = .{},
67-
http_headers_done_receiving: List = .{},
66+
http_headers_done: List = .{},
6867
notification_created: List = .{},
6968
};
7069

@@ -76,8 +75,7 @@ pub const Notification = struct {
7675
http_request_fail: *const RequestFail,
7776
http_request_start: *const RequestStart,
7877
http_request_intercept: *const RequestIntercept,
79-
http_header_received: *const ResponseHeader,
80-
http_headers_done_receiving: *const ResponseHeadersDone,
78+
http_headers_done: *const ResponseHeadersDone,
8179
notification_created: *Notification,
8280
};
8381
const EventType = std.meta.FieldEnum(Events);
@@ -104,12 +102,6 @@ pub const Notification = struct {
104102
wait_for_interception: *bool,
105103
};
106104

107-
pub const ResponseHeader = struct {
108-
request_id: u64,
109-
status: u16,
110-
header: std.http.Header,
111-
};
112-
113105
pub const ResponseHeadersDone = struct {
114106
transfer: *Transfer,
115107
};

0 commit comments

Comments
 (0)