Commit 337e1109f5

Andrew Kelley <andrew@ziglang.org>
2025-04-04 00:57:35
std.DoublyLinkedList: remove length tracking
this is trivial to tack on, and in my experience it is rarely wanted.
1 parent 3b77a84
Changed files (2)
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(node);
+        pool.used.remove(&connection.pool_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;
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{};
-
-    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 };
+/// 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;
+}
 
-    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}
+test "basics" {
+    const L = struct {
+        data: u32,
+        node: DoublyLinkedList.Node = .{},
+    };
+    var list: DoublyLinkedList = .{};
+
+    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{};
-
-    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 };
-
-    list1.append(&one);
-    list1.append(&two);
-    list2.append(&three);
-    list2.append(&four);
-    list2.append(&five);
+test "concatenation" {
+    const L = struct {
+        data: u32,
+        node: DoublyLinkedList.Node = .{},
+    };
+    var list1: DoublyLinkedList = .{};
+    var list2: DoublyLinkedList = .{};
+
+    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.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;
         }
     }