From 337e1109f5c2894aad9519ee9f7ff1f1f4c65b56 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 3 Apr 2025 15:57:35 -0700 Subject: [PATCH] std.DoublyLinkedList: remove length tracking this is trivial to tack on, and in my experience it is rarely wanted. --- lib/std/DoublyLinkedList.zig | 116 +++++++++++++++++++---------------- lib/std/http/Client.zig | 87 +++++++++++++------------- 2 files changed, 107 insertions(+), 96 deletions(-) diff --git a/lib/std/DoublyLinkedList.zig b/lib/std/DoublyLinkedList.zig index e0424b8252..dcc656bf43 100644 --- a/lib/std/DoublyLinkedList.zig +++ b/lib/std/DoublyLinkedList.zig @@ -17,7 +17,6 @@ const DoublyLinkedList = @This(); first: ?*Node = null, last: ?*Node = null, -len: usize = 0, /// This struct contains only the prev and next pointers and not any data /// payload. The intended usage is to embed it intrusively into another data @@ -39,8 +38,6 @@ pub fn insertAfter(list: *DoublyLinkedList, existing_node: *Node, new_node: *Nod list.last = new_node; } existing_node.next = new_node; - - list.len += 1; } pub fn insertBefore(list: *DoublyLinkedList, existing_node: *Node, new_node: *Node) void { @@ -55,8 +52,6 @@ pub fn insertBefore(list: *DoublyLinkedList, existing_node: *Node, new_node: *No list.first = new_node; } existing_node.prev = new_node; - - list.len += 1; } /// Concatenate list2 onto the end of list1, removing all entries from the former. @@ -69,16 +64,13 @@ pub fn concatByMoving(list1: *DoublyLinkedList, list2: *DoublyLinkedList) void { if (list1.last) |l1_last| { l1_last.next = list2.first; l2_first.prev = list1.last; - list1.len += list2.len; } else { // list1 was empty list1.first = list2.first; - list1.len = list2.len; } list1.last = list2.last; list2.first = null; list2.last = null; - list2.len = 0; } /// Insert a new node at the end of the list. @@ -109,8 +101,6 @@ pub fn prepend(list: *DoublyLinkedList, new_node: *Node) void { list.last = new_node; new_node.prev = null; new_node.next = null; - - list.len = 1; } } @@ -134,9 +124,6 @@ pub fn remove(list: *DoublyLinkedList, node: *Node) void { // Last element of the list. list.last = node.prev; } - - list.len -= 1; - assert(list.len == 0 or (list.first != null and list.last != null)); } /// Remove and return the last node in the list. @@ -159,28 +146,43 @@ pub fn popFirst(list: *DoublyLinkedList) ?*Node { return first; } -test "basic DoublyLinkedList test" { - const L = DoublyLinkedList(u32); - var list = L{}; +/// Iterate over all nodes, returning the count. +/// +/// This operation is O(N). Consider tracking the length separately rather than +/// computing it. +pub fn len(list: DoublyLinkedList) usize { + var count: usize = 0; + var it: ?*const Node = list.first; + while (it) |n| : (it = n.next) count += 1; + return count; +} - var one = L.Node{ .data = 1 }; - var two = L.Node{ .data = 2 }; - var three = L.Node{ .data = 3 }; - var four = L.Node{ .data = 4 }; - var five = L.Node{ .data = 5 }; +test "basics" { + const L = struct { + data: u32, + node: DoublyLinkedList.Node = .{}, + }; + var list: DoublyLinkedList = .{}; - list.append(&two); // {2} - list.append(&five); // {2, 5} - list.prepend(&one); // {1, 2, 5} - list.insertBefore(&five, &four); // {1, 2, 4, 5} - list.insertAfter(&two, &three); // {1, 2, 3, 4, 5} + var one: L = .{ .data = 1 }; + var two: L = .{ .data = 2 }; + var three: L = .{ .data = 3 }; + var four: L = .{ .data = 4 }; + var five: L = .{ .data = 5 }; + + list.append(&two.node); // {2} + list.append(&five.node); // {2, 5} + list.prepend(&one.node); // {1, 2, 5} + list.insertBefore(&five.node, &four.node); // {1, 2, 4, 5} + list.insertAfter(&two.node, &three.node); // {1, 2, 3, 4, 5} // Traverse forwards. { var it = list.first; var index: u32 = 1; while (it) |node| : (it = node.next) { - try testing.expect(node.data == index); + const l: *L = @fieldParentPtr("node", node); + try testing.expect(l.data == index); index += 1; } } @@ -190,51 +192,56 @@ test "basic DoublyLinkedList test" { var it = list.last; var index: u32 = 1; while (it) |node| : (it = node.prev) { - try testing.expect(node.data == (6 - index)); + const l: *L = @fieldParentPtr("node", node); + try testing.expect(l.data == (6 - index)); index += 1; } } _ = list.popFirst(); // {2, 3, 4, 5} _ = list.pop(); // {2, 3, 4} - list.remove(&three); // {2, 4} + list.remove(&three.node); // {2, 4} - try testing.expect(list.first.?.data == 2); - try testing.expect(list.last.?.data == 4); - try testing.expect(list.len == 2); + try testing.expect(@as(*L, @fieldParentPtr("node", list.first.?)).data == 2); + try testing.expect(@as(*L, @fieldParentPtr("node", list.last.?)).data == 4); + try testing.expect(list.len() == 2); } -test "DoublyLinkedList concatenation" { - const L = DoublyLinkedList(u32); - var list1 = L{}; - var list2 = L{}; +test "concatenation" { + const L = struct { + data: u32, + node: DoublyLinkedList.Node = .{}, + }; + var list1: DoublyLinkedList = .{}; + var list2: DoublyLinkedList = .{}; - var one = L.Node{ .data = 1 }; - var two = L.Node{ .data = 2 }; - var three = L.Node{ .data = 3 }; - var four = L.Node{ .data = 4 }; - var five = L.Node{ .data = 5 }; + var one: L = .{ .data = 1 }; + var two: L = .{ .data = 2 }; + var three: L = .{ .data = 3 }; + var four: L = .{ .data = 4 }; + var five: L = .{ .data = 5 }; - list1.append(&one); - list1.append(&two); - list2.append(&three); - list2.append(&four); - list2.append(&five); + list1.append(&one.node); + list1.append(&two.node); + list2.append(&three.node); + list2.append(&four.node); + list2.append(&five.node); list1.concatByMoving(&list2); - try testing.expect(list1.last == &five); - try testing.expect(list1.len == 5); + try testing.expect(list1.last == &five.node); + try testing.expect(list1.len() == 5); try testing.expect(list2.first == null); try testing.expect(list2.last == null); - try testing.expect(list2.len == 0); + try testing.expect(list2.len() == 0); // Traverse forwards. { var it = list1.first; var index: u32 = 1; while (it) |node| : (it = node.next) { - try testing.expect(node.data == index); + const l: *L = @fieldParentPtr("node", node); + try testing.expect(l.data == index); index += 1; } } @@ -244,7 +251,8 @@ test "DoublyLinkedList concatenation" { var it = list1.last; var index: u32 = 1; while (it) |node| : (it = node.prev) { - try testing.expect(node.data == (6 - index)); + const l: *L = @fieldParentPtr("node", node); + try testing.expect(l.data == (6 - index)); index += 1; } } @@ -257,7 +265,8 @@ test "DoublyLinkedList concatenation" { var it = list2.first; var index: u32 = 1; while (it) |node| : (it = node.next) { - try testing.expect(node.data == index); + const l: *L = @fieldParentPtr("node", node); + try testing.expect(l.data == index); index += 1; } } @@ -267,7 +276,8 @@ test "DoublyLinkedList concatenation" { var it = list2.last; var index: u32 = 1; while (it) |node| : (it = node.prev) { - try testing.expect(node.data == (6 - index)); + const l: *L = @fieldParentPtr("node", node); + try testing.expect(l.data == (6 - index)); index += 1; } } diff --git a/lib/std/http/Client.zig b/lib/std/http/Client.zig index ecc1893194..d3041619b0 100644 --- a/lib/std/http/Client.zig +++ b/lib/std/http/Client.zig @@ -46,9 +46,9 @@ https_proxy: ?*Proxy = null, pub const ConnectionPool = struct { mutex: std.Thread.Mutex = .{}, /// Open connections that are currently in use. - used: Queue = .{}, + used: std.DoublyLinkedList = .{}, /// Open connections that are not currently in use. - free: Queue = .{}, + free: std.DoublyLinkedList = .{}, free_len: usize = 0, free_size: usize = 32, @@ -59,9 +59,6 @@ pub const ConnectionPool = struct { protocol: Connection.Protocol, }; - const Queue = std.DoublyLinkedList(Connection); - pub const Node = Queue.Node; - /// Finds and acquires a connection from the connection pool matching the criteria. This function is threadsafe. /// If no connection is found, null is returned. pub fn findConnection(pool: *ConnectionPool, criteria: Criteria) ?*Connection { @@ -70,33 +67,34 @@ pub const ConnectionPool = struct { var next = pool.free.last; while (next) |node| : (next = node.prev) { - if (node.data.protocol != criteria.protocol) continue; - if (node.data.port != criteria.port) continue; + const connection: *Connection = @fieldParentPtr("pool_node", node); + if (connection.protocol != criteria.protocol) continue; + if (connection.port != criteria.port) continue; // Domain names are case-insensitive (RFC 5890, Section 2.3.2.4) - if (!std.ascii.eqlIgnoreCase(node.data.host, criteria.host)) continue; + if (!std.ascii.eqlIgnoreCase(connection.host, criteria.host)) continue; - pool.acquireUnsafe(node); - return &node.data; + pool.acquireUnsafe(connection); + return connection; } return null; } /// Acquires an existing connection from the connection pool. This function is not threadsafe. - pub fn acquireUnsafe(pool: *ConnectionPool, node: *Node) void { - pool.free.remove(node); + pub fn acquireUnsafe(pool: *ConnectionPool, connection: *Connection) void { + pool.free.remove(&connection.pool_node); pool.free_len -= 1; - pool.used.append(node); + pool.used.append(&connection.pool_node); } /// Acquires an existing connection from the connection pool. This function is threadsafe. - pub fn acquire(pool: *ConnectionPool, node: *Node) void { + pub fn acquire(pool: *ConnectionPool, connection: *Connection) void { pool.mutex.lock(); defer pool.mutex.unlock(); - return pool.acquireUnsafe(node); + return pool.acquireUnsafe(connection); } /// Tries to release a connection back to the connection pool. This function is threadsafe. @@ -108,38 +106,37 @@ pub const ConnectionPool = struct { pool.mutex.lock(); defer pool.mutex.unlock(); - const node: *Node = @fieldParentPtr("data", connection); + pool.used.remove(&connection.pool_node); - pool.used.remove(node); - - if (node.data.closing or pool.free_size == 0) { - node.data.close(allocator); - return allocator.destroy(node); + if (connection.closing or pool.free_size == 0) { + connection.close(allocator); + return allocator.destroy(connection); } if (pool.free_len >= pool.free_size) { - const popped = pool.free.popFirst() orelse unreachable; + const popped: *Connection = @fieldParentPtr("pool_node", pool.free.popFirst().?); pool.free_len -= 1; - popped.data.close(allocator); + popped.close(allocator); allocator.destroy(popped); } - if (node.data.proxied) { - pool.free.prepend(node); // proxied connections go to the end of the queue, always try direct connections first + if (connection.proxied) { + // proxied connections go to the end of the queue, always try direct connections first + pool.free.prepend(&connection.pool_node); } else { - pool.free.append(node); + pool.free.append(&connection.pool_node); } pool.free_len += 1; } /// Adds a newly created node to the pool of used connections. This function is threadsafe. - pub fn addUsed(pool: *ConnectionPool, node: *Node) void { + pub fn addUsed(pool: *ConnectionPool, connection: *Connection) void { pool.mutex.lock(); defer pool.mutex.unlock(); - pool.used.append(node); + pool.used.append(&connection.pool_node); } /// Resizes the connection pool. This function is threadsafe. @@ -170,18 +167,18 @@ pub const ConnectionPool = struct { var next = pool.free.first; while (next) |node| { - defer allocator.destroy(node); + const connection: *Connection = @fieldParentPtr("pool_node", node); next = node.next; - - node.data.close(allocator); + connection.close(allocator); + allocator.destroy(connection); } next = pool.used.first; while (next) |node| { - defer allocator.destroy(node); + const connection: *Connection = @fieldParentPtr("pool_node", node); next = node.next; - - node.data.close(allocator); + connection.close(allocator); + allocator.destroy(node); } pool.* = undefined; @@ -194,6 +191,9 @@ pub const Connection = struct { /// undefined unless protocol is tls. tls_client: if (!disable_tls) *std.crypto.tls.Client else void, + /// Entry in `ConnectionPool.used` or `ConnectionPool.free`. + pool_node: std.DoublyLinkedList.Node, + /// The protocol that this connection is using. protocol: Protocol, @@ -1326,9 +1326,8 @@ pub fn connectTcp(client: *Client, host: []const u8, port: u16, protocol: Connec if (disable_tls and protocol == .tls) return error.TlsInitializationFailed; - const conn = try client.allocator.create(ConnectionPool.Node); + const conn = try client.allocator.create(Connection); errdefer client.allocator.destroy(conn); - conn.* = .{ .data = undefined }; const stream = net.tcpConnectToHost(client.allocator, host, port) catch |err| switch (err) { error.ConnectionRefused => return error.ConnectionRefused, @@ -1343,21 +1342,23 @@ pub fn connectTcp(client: *Client, host: []const u8, port: u16, protocol: Connec }; errdefer stream.close(); - conn.data = .{ + conn.* = .{ .stream = stream, .tls_client = undefined, .protocol = protocol, .host = try client.allocator.dupe(u8, host), .port = port, + + .pool_node = .{}, }; - errdefer client.allocator.free(conn.data.host); + errdefer client.allocator.free(conn.host); if (protocol == .tls) { if (disable_tls) unreachable; - conn.data.tls_client = try client.allocator.create(std.crypto.tls.Client); - errdefer client.allocator.destroy(conn.data.tls_client); + conn.tls_client = try client.allocator.create(std.crypto.tls.Client); + errdefer client.allocator.destroy(conn.tls_client); const ssl_key_log_file: ?std.fs.File = if (std.options.http_enable_ssl_key_log_file) ssl_key_log_file: { const ssl_key_log_path = std.process.getEnvVarOwned(client.allocator, "SSLKEYLOGFILE") catch |err| switch (err) { @@ -1375,19 +1376,19 @@ pub fn connectTcp(client: *Client, host: []const u8, port: u16, protocol: Connec } else null; errdefer if (ssl_key_log_file) |key_log_file| key_log_file.close(); - conn.data.tls_client.* = std.crypto.tls.Client.init(stream, .{ + conn.tls_client.* = std.crypto.tls.Client.init(stream, .{ .host = .{ .explicit = host }, .ca = .{ .bundle = client.ca_bundle }, .ssl_key_log_file = ssl_key_log_file, }) catch return error.TlsInitializationFailed; // This is appropriate for HTTPS because the HTTP headers contain // the content length which is used to detect truncation attacks. - conn.data.tls_client.allow_truncation_attacks = true; + conn.tls_client.allow_truncation_attacks = true; } client.connection_pool.addUsed(conn); - return &conn.data; + return conn; } pub const ConnectUnixError = Allocator.Error || std.posix.SocketError || error{NameTooLong} || std.posix.ConnectError;