From 1639fcea43549853f1fded32aa1d711d21771e1c Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 3 Apr 2025 14:11:38 -0700 Subject: [PATCH 1/4] de-genericify SinglyLinkedList by making it always intrusive, we make it a more broadly useful API, and avoid binary bloat. --- CMakeLists.txt | 1 - lib/std/SinglyLinkedList.zig | 166 ++++++++++++++++++++++++++++++ lib/std/Thread/Pool.zig | 31 +++--- lib/std/heap/arena_allocator.zig | 41 +++++--- lib/std/linked_list.zig | 168 ------------------------------- lib/std/std.zig | 2 +- 6 files changed, 207 insertions(+), 202 deletions(-) create mode 100644 lib/std/SinglyLinkedList.zig diff --git a/CMakeLists.txt b/CMakeLists.txt index ea25212fec..4292bc1b92 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -444,7 +444,6 @@ set(ZIG_STAGE2_SOURCES lib/std/json.zig lib/std/json/stringify.zig lib/std/leb128.zig - lib/std/linked_list.zig lib/std/log.zig lib/std/macho.zig lib/std/math.zig diff --git a/lib/std/SinglyLinkedList.zig b/lib/std/SinglyLinkedList.zig new file mode 100644 index 0000000000..d118ce0395 --- /dev/null +++ b/lib/std/SinglyLinkedList.zig @@ -0,0 +1,166 @@ +//! A singly-linked list is headed by a single forward pointer. The elements +//! are singly-linked for minimum space and pointer manipulation overhead at +//! the expense of O(n) removal for arbitrary elements. New elements can be +//! added to the list after an existing element or at the head of the list. +//! +//! A singly-linked list may only be traversed in the forward direction. +//! +//! Singly-linked lists are useful under these conditions: +//! * Ability to preallocate elements / requirement of infallibility for +//! insertion. +//! * Ability to allocate elements intrusively along with other data. +//! * Homogenous elements. + +const std = @import("std.zig"); +const debug = std.debug; +const assert = debug.assert; +const testing = std.testing; +const SinglyLinkedList = @This(); + +first: ?*Node = null, + +/// This struct contains only a next pointer and not any data payload. The +/// intended usage is to embed it intrusively into another data structure and +/// access the data with `@fieldParentPtr`. +pub const Node = struct { + next: ?*Node = null, + + pub fn insertAfter(node: *Node, new_node: *Node) void { + new_node.next = node.next; + node.next = new_node; + } + + /// Remove the node after the one provided, returning it. + pub fn removeNext(node: *Node) ?*Node { + const next_node = node.next orelse return null; + node.next = next_node.next; + return next_node; + } + + /// Iterate over the singly-linked list from this node, until the final + /// node is found. + /// + /// This operation is O(N). Instead of calling this function, consider + /// using a different data structure. + pub fn findLast(node: *Node) *Node { + var it = node; + while (true) { + it = it.next orelse return it; + } + } + + /// Iterate over each next node, returning the count of all nodes except + /// the starting one. + /// + /// This operation is O(N). Instead of calling this function, consider + /// using a different data structure. + pub fn countChildren(node: *const Node) usize { + var count: usize = 0; + var it: ?*const Node = node.next; + while (it) |n| : (it = n.next) { + count += 1; + } + return count; + } + + /// Reverse the list starting from this node in-place. + /// + /// This operation is O(N). Instead of calling this function, consider + /// using a different data structure. + pub fn reverse(indirect: *?*Node) void { + if (indirect.* == null) { + return; + } + var current: *Node = indirect.*.?; + while (current.next) |next| { + current.next = next.next; + next.next = indirect.*; + indirect.* = next; + } + } +}; + +pub fn prepend(list: *SinglyLinkedList, new_node: *Node) void { + new_node.next = list.first; + list.first = new_node; +} + +pub fn remove(list: *SinglyLinkedList, node: *Node) void { + if (list.first == node) { + list.first = node.next; + } else { + var current_elm = list.first.?; + while (current_elm.next != node) { + current_elm = current_elm.next.?; + } + current_elm.next = node.next; + } +} + +/// Remove and return the first node in the list. +pub fn popFirst(list: *SinglyLinkedList) ?*Node { + const first = list.first orelse return null; + list.first = first.next; + return first; +} + +/// 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: SinglyLinkedList) usize { + if (list.first) |n| { + return 1 + n.countChildren(); + } else { + return 0; + } +} + +test "basics" { + const L = struct { + data: u32, + node: SinglyLinkedList.Node = .{}, + }; + var list: SinglyLinkedList = .{}; + + try testing.expect(list.len() == 0); + + 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.prepend(&two.node); // {2} + two.node.insertAfter(&five.node); // {2, 5} + list.prepend(&one.node); // {1, 2, 5} + two.node.insertAfter(&three.node); // {1, 2, 3, 5} + three.node.insertAfter(&four.node); // {1, 2, 3, 4, 5} + + try testing.expect(list.len() == 5); + + // Traverse forwards. + { + var it = list.first; + var index: u32 = 1; + while (it) |node| : (it = node.next) { + const l: *L = @fieldParentPtr("node", node); + try testing.expect(l.data == index); + index += 1; + } + } + + _ = list.popFirst(); // {2, 3, 4, 5} + _ = list.remove(&five.node); // {2, 3, 4} + _ = two.node.removeNext(); // {2, 4} + + try testing.expect(@as(*L, @fieldParentPtr("node", list.first.?)).data == 2); + try testing.expect(@as(*L, @fieldParentPtr("node", list.first.?.next.?)).data == 4); + try testing.expect(list.first.?.next.?.next == null); + + SinglyLinkedList.Node.reverse(&list.first); + + try testing.expect(@as(*L, @fieldParentPtr("node", list.first.?)).data == 4); + try testing.expect(@as(*L, @fieldParentPtr("node", list.first.?.next.?)).data == 2); + try testing.expect(list.first.?.next.?.next == null); +} diff --git a/lib/std/Thread/Pool.zig b/lib/std/Thread/Pool.zig index 874050a35f..e836665d70 100644 --- a/lib/std/Thread/Pool.zig +++ b/lib/std/Thread/Pool.zig @@ -5,7 +5,7 @@ const WaitGroup = @import("WaitGroup.zig"); mutex: std.Thread.Mutex = .{}, cond: std.Thread.Condition = .{}, -run_queue: RunQueue = .{}, +run_queue: std.SinglyLinkedList = .{}, is_running: bool = true, allocator: std.mem.Allocator, threads: if (builtin.single_threaded) [0]std.Thread else []std.Thread, @@ -16,9 +16,9 @@ ids: if (builtin.single_threaded) struct { } } else std.AutoArrayHashMapUnmanaged(std.Thread.Id, void), -const RunQueue = std.SinglyLinkedList(Runnable); const Runnable = struct { runFn: RunProto, + node: std.SinglyLinkedList.Node = .{}, }; const RunProto = *const fn (*Runnable, id: ?usize) void; @@ -110,12 +110,11 @@ pub fn spawnWg(pool: *Pool, wait_group: *WaitGroup, comptime func: anytype, args const Closure = struct { arguments: Args, pool: *Pool, - run_node: RunQueue.Node = .{ .data = .{ .runFn = runFn } }, + runnable: Runnable = .{ .runFn = runFn }, wait_group: *WaitGroup, fn runFn(runnable: *Runnable, _: ?usize) void { - const run_node: *RunQueue.Node = @fieldParentPtr("data", runnable); - const closure: *@This() = @alignCast(@fieldParentPtr("run_node", run_node)); + const closure: *@This() = @alignCast(@fieldParentPtr("runnable", runnable)); @call(.auto, func, closure.arguments); closure.wait_group.finish(); @@ -143,7 +142,7 @@ pub fn spawnWg(pool: *Pool, wait_group: *WaitGroup, comptime func: anytype, args .wait_group = wait_group, }; - pool.run_queue.prepend(&closure.run_node); + pool.run_queue.prepend(&closure.runnable.node); pool.mutex.unlock(); } @@ -173,12 +172,11 @@ pub fn spawnWgId(pool: *Pool, wait_group: *WaitGroup, comptime func: anytype, ar const Closure = struct { arguments: Args, pool: *Pool, - run_node: RunQueue.Node = .{ .data = .{ .runFn = runFn } }, + runnable: Runnable = .{ .runFn = runFn }, wait_group: *WaitGroup, fn runFn(runnable: *Runnable, id: ?usize) void { - const run_node: *RunQueue.Node = @fieldParentPtr("data", runnable); - const closure: *@This() = @alignCast(@fieldParentPtr("run_node", run_node)); + const closure: *@This() = @alignCast(@fieldParentPtr("runnable", runnable)); @call(.auto, func, .{id.?} ++ closure.arguments); closure.wait_group.finish(); @@ -207,7 +205,7 @@ pub fn spawnWgId(pool: *Pool, wait_group: *WaitGroup, comptime func: anytype, ar .wait_group = wait_group, }; - pool.run_queue.prepend(&closure.run_node); + pool.run_queue.prepend(&closure.runnable.node); pool.mutex.unlock(); } @@ -225,11 +223,10 @@ pub fn spawn(pool: *Pool, comptime func: anytype, args: anytype) !void { const Closure = struct { arguments: Args, pool: *Pool, - run_node: RunQueue.Node = .{ .data = .{ .runFn = runFn } }, + runnable: Runnable = .{ .runFn = runFn }, fn runFn(runnable: *Runnable, _: ?usize) void { - const run_node: *RunQueue.Node = @fieldParentPtr("data", runnable); - const closure: *@This() = @alignCast(@fieldParentPtr("run_node", run_node)); + const closure: *@This() = @alignCast(@fieldParentPtr("runnable", runnable)); @call(.auto, func, closure.arguments); // The thread pool's allocator is protected by the mutex. @@ -251,7 +248,7 @@ pub fn spawn(pool: *Pool, comptime func: anytype, args: anytype) !void { .pool = pool, }; - pool.run_queue.prepend(&closure.run_node); + pool.run_queue.prepend(&closure.runnable.node); } // Notify waiting threads outside the lock to try and keep the critical section small. @@ -292,7 +289,8 @@ fn worker(pool: *Pool) void { pool.mutex.unlock(); defer pool.mutex.lock(); - run_node.data.runFn(&run_node.data, id); + const runnable: *Runnable = @fieldParentPtr("node", run_node); + runnable.runFn(runnable, id); } // Stop executing instead of waiting if the thread pool is no longer running. @@ -312,7 +310,8 @@ pub fn waitAndWork(pool: *Pool, wait_group: *WaitGroup) void { if (pool.run_queue.popFirst()) |run_node| { id = id orelse pool.ids.getIndex(std.Thread.getCurrentId()); pool.mutex.unlock(); - run_node.data.runFn(&run_node.data, id); + const runnable: *Runnable = @fieldParentPtr("node", run_node); + runnable.runFn(runnable, id); continue; } diff --git a/lib/std/heap/arena_allocator.zig b/lib/std/heap/arena_allocator.zig index f88bb7de16..c472ae80c5 100644 --- a/lib/std/heap/arena_allocator.zig +++ b/lib/std/heap/arena_allocator.zig @@ -14,7 +14,7 @@ pub const ArenaAllocator = struct { /// Inner state of ArenaAllocator. Can be stored rather than the entire ArenaAllocator /// as a memory-saving optimization. pub const State = struct { - buffer_list: std.SinglyLinkedList(usize) = .{}, + buffer_list: std.SinglyLinkedList = .{}, end_index: usize = 0, pub fn promote(self: State, child_allocator: Allocator) ArenaAllocator { @@ -37,7 +37,10 @@ pub const ArenaAllocator = struct { }; } - const BufNode = std.SinglyLinkedList(usize).Node; + const BufNode = struct { + data: usize, + node: std.SinglyLinkedList.Node = .{}, + }; const BufNode_alignment: mem.Alignment = .fromByteUnits(@alignOf(BufNode)); pub fn init(child_allocator: Allocator) ArenaAllocator { @@ -51,7 +54,8 @@ pub const ArenaAllocator = struct { while (it) |node| { // this has to occur before the free because the free frees node const next_it = node.next; - const alloc_buf = @as([*]u8, @ptrCast(node))[0..node.data]; + const buf_node: *BufNode = @fieldParentPtr("node", node); + const alloc_buf = @as([*]u8, @ptrCast(buf_node))[0..buf_node.data]; self.child_allocator.rawFree(alloc_buf, BufNode_alignment, @returnAddress()); it = next_it; } @@ -78,7 +82,8 @@ pub const ArenaAllocator = struct { while (it) |node| : (it = node.next) { // Compute the actually allocated size excluding the // linked list node. - size += node.data - @sizeOf(BufNode); + const buf_node: *BufNode = @fieldParentPtr("node", node); + size += buf_node.data - @sizeOf(BufNode); } return size; } @@ -130,7 +135,8 @@ pub const ArenaAllocator = struct { const next_it = node.next; if (next_it == null) break node; - const alloc_buf = @as([*]u8, @ptrCast(node))[0..node.data]; + const buf_node: *BufNode = @fieldParentPtr("node", node); + const alloc_buf = @as([*]u8, @ptrCast(buf_node))[0..buf_node.data]; self.child_allocator.rawFree(alloc_buf, BufNode_alignment, @returnAddress()); it = next_it; } else null; @@ -140,12 +146,13 @@ pub const ArenaAllocator = struct { if (maybe_first_node) |first_node| { self.state.buffer_list.first = first_node; // perfect, no need to invoke the child_allocator - if (first_node.data == total_size) + const first_buf_node: *BufNode = @fieldParentPtr("node", first_node); + if (first_buf_node.data == total_size) return true; - const first_alloc_buf = @as([*]u8, @ptrCast(first_node))[0..first_node.data]; + const first_alloc_buf = @as([*]u8, @ptrCast(first_buf_node))[0..first_buf_node.data]; if (self.child_allocator.rawResize(first_alloc_buf, BufNode_alignment, total_size, @returnAddress())) { // successful resize - first_node.data = total_size; + first_buf_node.data = total_size; } else { // manual realloc const new_ptr = self.child_allocator.rawAlloc(total_size, BufNode_alignment, @returnAddress()) orelse { @@ -153,9 +160,9 @@ pub const ArenaAllocator = struct { return false; }; self.child_allocator.rawFree(first_alloc_buf, BufNode_alignment, @returnAddress()); - const node: *BufNode = @ptrCast(@alignCast(new_ptr)); - node.* = .{ .data = total_size }; - self.state.buffer_list.first = node; + const buf_node: *BufNode = @ptrCast(@alignCast(new_ptr)); + buf_node.* = .{ .data = total_size }; + self.state.buffer_list.first = &buf_node.node; } } return true; @@ -169,7 +176,7 @@ pub const ArenaAllocator = struct { return null; const buf_node: *BufNode = @ptrCast(@alignCast(ptr)); buf_node.* = .{ .data = len }; - self.state.buffer_list.prepend(buf_node); + self.state.buffer_list.prepend(&buf_node.node); self.state.end_index = 0; return buf_node; } @@ -179,8 +186,8 @@ pub const ArenaAllocator = struct { _ = ra; const ptr_align = alignment.toByteUnits(); - var cur_node = if (self.state.buffer_list.first) |first_node| - first_node + var cur_node: *BufNode = if (self.state.buffer_list.first) |first_node| + @fieldParentPtr("node", first_node) else (self.createNode(0, n + ptr_align) orelse return null); while (true) { @@ -213,7 +220,8 @@ pub const ArenaAllocator = struct { _ = ret_addr; const cur_node = self.state.buffer_list.first orelse return false; - const cur_buf = @as([*]u8, @ptrCast(cur_node))[@sizeOf(BufNode)..cur_node.data]; + const cur_buf_node: *BufNode = @fieldParentPtr("node", cur_node); + const cur_buf = @as([*]u8, @ptrCast(cur_buf_node))[@sizeOf(BufNode)..cur_buf_node.data]; if (@intFromPtr(cur_buf.ptr) + self.state.end_index != @intFromPtr(buf.ptr) + buf.len) { // It's not the most recent allocation, so it cannot be expanded, // but it's fine if they want to make it smaller. @@ -248,7 +256,8 @@ pub const ArenaAllocator = struct { const self: *ArenaAllocator = @ptrCast(@alignCast(ctx)); const cur_node = self.state.buffer_list.first orelse return; - const cur_buf = @as([*]u8, @ptrCast(cur_node))[@sizeOf(BufNode)..cur_node.data]; + const cur_buf_node: *BufNode = @fieldParentPtr("node", cur_node); + const cur_buf = @as([*]u8, @ptrCast(cur_buf_node))[@sizeOf(BufNode)..cur_buf_node.data]; if (@intFromPtr(cur_buf.ptr) + self.state.end_index == @intFromPtr(buf.ptr) + buf.len) { self.state.end_index -= buf.len; diff --git a/lib/std/linked_list.zig b/lib/std/linked_list.zig index 07c5f5d4df..5b397a6ea7 100644 --- a/lib/std/linked_list.zig +++ b/lib/std/linked_list.zig @@ -3,174 +3,6 @@ const debug = std.debug; const assert = debug.assert; const testing = std.testing; -/// A singly-linked list is headed by a single forward pointer. The elements -/// are singly-linked for minimum space and pointer manipulation overhead at -/// the expense of O(n) removal for arbitrary elements. New elements can be -/// added to the list after an existing element or at the head of the list. -/// A singly-linked list may only be traversed in the forward direction. -/// Singly-linked lists are ideal for applications with large datasets and -/// few or no removals or for implementing a LIFO queue. -pub fn SinglyLinkedList(comptime T: type) type { - return struct { - const Self = @This(); - - /// Node inside the linked list wrapping the actual data. - pub const Node = struct { - next: ?*Node = null, - data: T, - - pub const Data = T; - - /// Insert a new node after the current one. - /// - /// Arguments: - /// new_node: Pointer to the new node to insert. - pub fn insertAfter(node: *Node, new_node: *Node) void { - new_node.next = node.next; - node.next = new_node; - } - - /// Remove a node from the list. - /// - /// Arguments: - /// node: Pointer to the node to be removed. - /// Returns: - /// node removed - pub fn removeNext(node: *Node) ?*Node { - const next_node = node.next orelse return null; - node.next = next_node.next; - return next_node; - } - - /// Iterate over the singly-linked list from this node, until the final node is found. - /// This operation is O(N). - pub fn findLast(node: *Node) *Node { - var it = node; - while (true) { - it = it.next orelse return it; - } - } - - /// Iterate over each next node, returning the count of all nodes except the starting one. - /// This operation is O(N). - pub fn countChildren(node: *const Node) usize { - var count: usize = 0; - var it: ?*const Node = node.next; - while (it) |n| : (it = n.next) { - count += 1; - } - return count; - } - - /// Reverse the list starting from this node in-place. - /// This operation is O(N). - pub fn reverse(indirect: *?*Node) void { - if (indirect.* == null) { - return; - } - var current: *Node = indirect.*.?; - while (current.next) |next| { - current.next = next.next; - next.next = indirect.*; - indirect.* = next; - } - } - }; - - first: ?*Node = null, - - /// Insert a new node at the head. - /// - /// Arguments: - /// new_node: Pointer to the new node to insert. - pub fn prepend(list: *Self, new_node: *Node) void { - new_node.next = list.first; - list.first = new_node; - } - - /// Remove a node from the list. - /// - /// Arguments: - /// node: Pointer to the node to be removed. - pub fn remove(list: *Self, node: *Node) void { - if (list.first == node) { - list.first = node.next; - } else { - var current_elm = list.first.?; - while (current_elm.next != node) { - current_elm = current_elm.next.?; - } - current_elm.next = node.next; - } - } - - /// Remove and return the first node in the list. - /// - /// Returns: - /// A pointer to the first node in the list. - pub fn popFirst(list: *Self) ?*Node { - const first = list.first orelse return null; - list.first = first.next; - return first; - } - - /// Iterate over all nodes, returning the count. - /// This operation is O(N). - pub fn len(list: Self) usize { - if (list.first) |n| { - return 1 + n.countChildren(); - } else { - return 0; - } - } - }; -} - -test "basic SinglyLinkedList test" { - const L = SinglyLinkedList(u32); - var list = L{}; - - try testing.expect(list.len() == 0); - - 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 }; - - list.prepend(&two); // {2} - two.insertAfter(&five); // {2, 5} - list.prepend(&one); // {1, 2, 5} - two.insertAfter(&three); // {1, 2, 3, 5} - three.insertAfter(&four); // {1, 2, 3, 4, 5} - - try testing.expect(list.len() == 5); - - // Traverse forwards. - { - var it = list.first; - var index: u32 = 1; - while (it) |node| : (it = node.next) { - try testing.expect(node.data == index); - index += 1; - } - } - - _ = list.popFirst(); // {2, 3, 4, 5} - _ = list.remove(&five); // {2, 3, 4} - _ = two.removeNext(); // {2, 4} - - try testing.expect(list.first.?.data == 2); - try testing.expect(list.first.?.next.?.data == 4); - try testing.expect(list.first.?.next.?.next == null); - - L.Node.reverse(&list.first); - - try testing.expect(list.first.?.data == 4); - try testing.expect(list.first.?.next.?.data == 2); - try testing.expect(list.first.?.next.?.next == null); -} - /// A doubly-linked list has a pair of pointers to both the head and /// tail of the list. List elements have pointers to both the previous /// and next elements in the sequence. The list can be traversed both diff --git a/lib/std/std.zig b/lib/std/std.zig index 558710015c..b3c583f861 100644 --- a/lib/std/std.zig +++ b/lib/std/std.zig @@ -33,7 +33,7 @@ pub const Random = @import("Random.zig"); pub const RingBuffer = @import("RingBuffer.zig"); pub const SegmentedList = @import("segmented_list.zig").SegmentedList; pub const SemanticVersion = @import("SemanticVersion.zig"); -pub const SinglyLinkedList = @import("linked_list.zig").SinglyLinkedList; +pub const SinglyLinkedList = @import("SinglyLinkedList.zig"); pub const StaticBitSet = bit_set.StaticBitSet; pub const StringHashMap = hash_map.StringHashMap; pub const StringHashMapUnmanaged = hash_map.StringHashMapUnmanaged; From 3b77a845f9a5409d19a9e330b82eece8af4ac18f Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 3 Apr 2025 15:35:21 -0700 Subject: [PATCH 2/4] de-genericify DoublyLinkedList by making it always intrusive, we make it more broadly useful API, and avoid binary bloat. --- lib/std/DoublyLinkedList.zig | 274 +++++++++++++++++++++++++++++++++ lib/std/linked_list.zig | 287 ----------------------------------- lib/std/std.zig | 2 +- 3 files changed, 275 insertions(+), 288 deletions(-) create mode 100644 lib/std/DoublyLinkedList.zig delete mode 100644 lib/std/linked_list.zig diff --git a/lib/std/DoublyLinkedList.zig b/lib/std/DoublyLinkedList.zig new file mode 100644 index 0000000000..e0424b8252 --- /dev/null +++ b/lib/std/DoublyLinkedList.zig @@ -0,0 +1,274 @@ +//! A doubly-linked list has a pair of pointers to both the head and +//! tail of the list. List elements have pointers to both the previous +//! and next elements in the sequence. The list can be traversed both +//! forward and backward. Some operations that take linear O(n) time +//! with a singly-linked list can be done without traversal in constant +//! O(1) time with a doubly-linked list: +//! +//! * Removing an element. +//! * Inserting a new element before an existing element. +//! * Pushing or popping an element from the end of the list. + +const std = @import("std.zig"); +const debug = std.debug; +const assert = debug.assert; +const testing = std.testing; +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 +/// structure and access the data with `@fieldParentPtr`. +pub const Node = struct { + prev: ?*Node = null, + next: ?*Node = null, +}; + +pub fn insertAfter(list: *DoublyLinkedList, existing_node: *Node, new_node: *Node) void { + new_node.prev = existing_node; + if (existing_node.next) |next_node| { + // Intermediate node. + new_node.next = next_node; + next_node.prev = new_node; + } else { + // Last element of the list. + new_node.next = null; + list.last = new_node; + } + existing_node.next = new_node; + + list.len += 1; +} + +pub fn insertBefore(list: *DoublyLinkedList, existing_node: *Node, new_node: *Node) void { + new_node.next = existing_node; + if (existing_node.prev) |prev_node| { + // Intermediate node. + new_node.prev = prev_node; + prev_node.next = new_node; + } else { + // First element of the list. + new_node.prev = null; + 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. +/// +/// Arguments: +/// list1: the list to concatenate onto +/// list2: the list to be concatenated +pub fn concatByMoving(list1: *DoublyLinkedList, list2: *DoublyLinkedList) void { + const l2_first = list2.first orelse return; + 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. +/// +/// Arguments: +/// new_node: Pointer to the new node to insert. +pub fn append(list: *DoublyLinkedList, new_node: *Node) void { + if (list.last) |last| { + // Insert after last. + list.insertAfter(last, new_node); + } else { + // Empty list. + list.prepend(new_node); + } +} + +/// Insert a new node at the beginning of the list. +/// +/// Arguments: +/// new_node: Pointer to the new node to insert. +pub fn prepend(list: *DoublyLinkedList, new_node: *Node) void { + if (list.first) |first| { + // Insert before first. + list.insertBefore(first, new_node); + } else { + // Empty list. + list.first = new_node; + list.last = new_node; + new_node.prev = null; + new_node.next = null; + + list.len = 1; + } +} + +/// Remove a node from the list. +/// +/// Arguments: +/// node: Pointer to the node to be removed. +pub fn remove(list: *DoublyLinkedList, node: *Node) void { + if (node.prev) |prev_node| { + // Intermediate node. + prev_node.next = node.next; + } else { + // First element of the list. + list.first = node.next; + } + + if (node.next) |next_node| { + // Intermediate node. + next_node.prev = node.prev; + } else { + // 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. +/// +/// Returns: +/// A pointer to the last node in the list. +pub fn pop(list: *DoublyLinkedList) ?*Node { + const last = list.last orelse return null; + list.remove(last); + return last; +} + +/// Remove and return the first node in the list. +/// +/// Returns: +/// A pointer to the first node in the list. +pub fn popFirst(list: *DoublyLinkedList) ?*Node { + const first = list.first orelse return null; + list.remove(first); + 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 }; + + 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} + + // Traverse forwards. + { + var it = list.first; + var index: u32 = 1; + while (it) |node| : (it = node.next) { + try testing.expect(node.data == index); + index += 1; + } + } + + // Traverse backwards. + { + var it = list.last; + var index: u32 = 1; + while (it) |node| : (it = node.prev) { + try testing.expect(node.data == (6 - index)); + index += 1; + } + } + + _ = list.popFirst(); // {2, 3, 4, 5} + _ = list.pop(); // {2, 3, 4} + list.remove(&three); // {2, 4} + + try testing.expect(list.first.?.data == 2); + try testing.expect(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); + + list1.concatByMoving(&list2); + + try testing.expect(list1.last == &five); + try testing.expect(list1.len == 5); + try testing.expect(list2.first == null); + try testing.expect(list2.last == null); + 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); + index += 1; + } + } + + // Traverse backwards. + { + var it = list1.last; + var index: u32 = 1; + while (it) |node| : (it = node.prev) { + try testing.expect(node.data == (6 - index)); + index += 1; + } + } + + // Swap them back, this verifies that concatenating to an empty list works. + list2.concatByMoving(&list1); + + // Traverse forwards. + { + var it = list2.first; + var index: u32 = 1; + while (it) |node| : (it = node.next) { + try testing.expect(node.data == index); + index += 1; + } + } + + // Traverse backwards. + { + var it = list2.last; + var index: u32 = 1; + while (it) |node| : (it = node.prev) { + try testing.expect(node.data == (6 - index)); + index += 1; + } + } +} diff --git a/lib/std/linked_list.zig b/lib/std/linked_list.zig deleted file mode 100644 index 5b397a6ea7..0000000000 --- a/lib/std/linked_list.zig +++ /dev/null @@ -1,287 +0,0 @@ -const std = @import("std.zig"); -const debug = std.debug; -const assert = debug.assert; -const testing = std.testing; - -/// A doubly-linked list has a pair of pointers to both the head and -/// tail of the list. List elements have pointers to both the previous -/// and next elements in the sequence. The list can be traversed both -/// forward and backward. Some operations that take linear O(n) time -/// with a singly-linked list can be done without traversal in constant -/// O(1) time with a doubly-linked list: -/// -/// - Removing an element. -/// - Inserting a new element before an existing element. -/// - Pushing or popping an element from the end of the list. -pub fn DoublyLinkedList(comptime T: type) type { - return struct { - const Self = @This(); - - /// Node inside the linked list wrapping the actual data. - pub const Node = struct { - prev: ?*Node = null, - next: ?*Node = null, - data: T, - }; - - first: ?*Node = null, - last: ?*Node = null, - len: usize = 0, - - /// Insert a new node after an existing one. - /// - /// Arguments: - /// node: Pointer to a node in the list. - /// new_node: Pointer to the new node to insert. - pub fn insertAfter(list: *Self, node: *Node, new_node: *Node) void { - new_node.prev = node; - if (node.next) |next_node| { - // Intermediate node. - new_node.next = next_node; - next_node.prev = new_node; - } else { - // Last element of the list. - new_node.next = null; - list.last = new_node; - } - node.next = new_node; - - list.len += 1; - } - - /// Insert a new node before an existing one. - /// - /// Arguments: - /// node: Pointer to a node in the list. - /// new_node: Pointer to the new node to insert. - pub fn insertBefore(list: *Self, node: *Node, new_node: *Node) void { - new_node.next = node; - if (node.prev) |prev_node| { - // Intermediate node. - new_node.prev = prev_node; - prev_node.next = new_node; - } else { - // First element of the list. - new_node.prev = null; - list.first = new_node; - } - node.prev = new_node; - - list.len += 1; - } - - /// Concatenate list2 onto the end of list1, removing all entries from the former. - /// - /// Arguments: - /// list1: the list to concatenate onto - /// list2: the list to be concatenated - pub fn concatByMoving(list1: *Self, list2: *Self) void { - const l2_first = list2.first orelse return; - 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. - /// - /// Arguments: - /// new_node: Pointer to the new node to insert. - pub fn append(list: *Self, new_node: *Node) void { - if (list.last) |last| { - // Insert after last. - list.insertAfter(last, new_node); - } else { - // Empty list. - list.prepend(new_node); - } - } - - /// Insert a new node at the beginning of the list. - /// - /// Arguments: - /// new_node: Pointer to the new node to insert. - pub fn prepend(list: *Self, new_node: *Node) void { - if (list.first) |first| { - // Insert before first. - list.insertBefore(first, new_node); - } else { - // Empty list. - list.first = new_node; - list.last = new_node; - new_node.prev = null; - new_node.next = null; - - list.len = 1; - } - } - - /// Remove a node from the list. - /// - /// Arguments: - /// node: Pointer to the node to be removed. - pub fn remove(list: *Self, node: *Node) void { - if (node.prev) |prev_node| { - // Intermediate node. - prev_node.next = node.next; - } else { - // First element of the list. - list.first = node.next; - } - - if (node.next) |next_node| { - // Intermediate node. - next_node.prev = node.prev; - } else { - // 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. - /// - /// Returns: - /// A pointer to the last node in the list. - pub fn pop(list: *Self) ?*Node { - const last = list.last orelse return null; - list.remove(last); - return last; - } - - /// Remove and return the first node in the list. - /// - /// Returns: - /// A pointer to the first node in the list. - pub fn popFirst(list: *Self) ?*Node { - const first = list.first orelse return null; - list.remove(first); - 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 }; - - 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} - - // Traverse forwards. - { - var it = list.first; - var index: u32 = 1; - while (it) |node| : (it = node.next) { - try testing.expect(node.data == index); - index += 1; - } - } - - // Traverse backwards. - { - var it = list.last; - var index: u32 = 1; - while (it) |node| : (it = node.prev) { - try testing.expect(node.data == (6 - index)); - index += 1; - } - } - - _ = list.popFirst(); // {2, 3, 4, 5} - _ = list.pop(); // {2, 3, 4} - list.remove(&three); // {2, 4} - - try testing.expect(list.first.?.data == 2); - try testing.expect(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); - - list1.concatByMoving(&list2); - - try testing.expect(list1.last == &five); - try testing.expect(list1.len == 5); - try testing.expect(list2.first == null); - try testing.expect(list2.last == null); - 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); - index += 1; - } - } - - // Traverse backwards. - { - var it = list1.last; - var index: u32 = 1; - while (it) |node| : (it = node.prev) { - try testing.expect(node.data == (6 - index)); - index += 1; - } - } - - // Swap them back, this verifies that concatenating to an empty list works. - list2.concatByMoving(&list1); - - // Traverse forwards. - { - var it = list2.first; - var index: u32 = 1; - while (it) |node| : (it = node.next) { - try testing.expect(node.data == index); - index += 1; - } - } - - // Traverse backwards. - { - var it = list2.last; - var index: u32 = 1; - while (it) |node| : (it = node.prev) { - try testing.expect(node.data == (6 - index)); - index += 1; - } - } -} diff --git a/lib/std/std.zig b/lib/std/std.zig index b3c583f861..94851657a6 100644 --- a/lib/std/std.zig +++ b/lib/std/std.zig @@ -16,7 +16,7 @@ pub const BufMap = @import("buf_map.zig").BufMap; pub const BufSet = @import("buf_set.zig").BufSet; pub const StaticStringMap = static_string_map.StaticStringMap; pub const StaticStringMapWithEql = static_string_map.StaticStringMapWithEql; -pub const DoublyLinkedList = @import("linked_list.zig").DoublyLinkedList; +pub const DoublyLinkedList = @import("DoublyLinkedList.zig"); pub const DynLib = @import("dynamic_library.zig").DynLib; pub const DynamicBitSet = bit_set.DynamicBitSet; pub const DynamicBitSetUnmanaged = bit_set.DynamicBitSetUnmanaged; From 337e1109f5c2894aad9519ee9f7ff1f1f4c65b56 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 3 Apr 2025 15:57:35 -0700 Subject: [PATCH 3/4] 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; From 810f70ef42fa013dc31b13445dd2910a43f8a0f7 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 3 Apr 2025 16:02:55 -0700 Subject: [PATCH 4/4] update compiler usage of DoublyLinkedList API --- src/Package/Fetch/git.zig | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/Package/Fetch/git.zig b/src/Package/Fetch/git.zig index f6e3dc1615..d74e83d8c3 100644 --- a/src/Package/Fetch/git.zig +++ b/src/Package/Fetch/git.zig @@ -473,14 +473,18 @@ const Object = struct { /// objects remaining in the cache will be freed when the cache itself is freed. const ObjectCache = struct { objects: std.AutoHashMapUnmanaged(u64, CacheEntry) = .empty, - lru_nodes: LruList = .{}, + lru_nodes: std.DoublyLinkedList = .{}, + lru_nodes_len: usize = 0, byte_size: usize = 0, const max_byte_size = 128 * 1024 * 1024; // 128MiB /// A list of offsets stored in the cache, with the most recently used /// entries at the end. - const LruList = std.DoublyLinkedList(u64); - const CacheEntry = struct { object: Object, lru_node: *LruList.Node }; + const LruListNode = struct { + data: u64, + node: std.DoublyLinkedList.Node, + }; + const CacheEntry = struct { object: Object, lru_node: *LruListNode }; fn deinit(cache: *ObjectCache, allocator: Allocator) void { var object_iterator = cache.objects.iterator(); @@ -496,8 +500,8 @@ const ObjectCache = struct { /// position if it is present. fn get(cache: *ObjectCache, offset: u64) ?Object { if (cache.objects.get(offset)) |entry| { - cache.lru_nodes.remove(entry.lru_node); - cache.lru_nodes.append(entry.lru_node); + cache.lru_nodes.remove(&entry.lru_node.node); + cache.lru_nodes.append(&entry.lru_node.node); return entry.object; } else { return null; @@ -510,26 +514,29 @@ const ObjectCache = struct { /// will not be evicted before the next call to `put` or `deinit` even if /// it exceeds the maximum cache size. fn put(cache: *ObjectCache, allocator: Allocator, offset: u64, object: Object) !void { - const lru_node = try allocator.create(LruList.Node); + const lru_node = try allocator.create(LruListNode); errdefer allocator.destroy(lru_node); lru_node.data = offset; const gop = try cache.objects.getOrPut(allocator, offset); if (gop.found_existing) { cache.byte_size -= gop.value_ptr.object.data.len; - cache.lru_nodes.remove(gop.value_ptr.lru_node); + cache.lru_nodes.remove(&gop.value_ptr.lru_node.node); + cache.lru_nodes_len -= 1; allocator.destroy(gop.value_ptr.lru_node); allocator.free(gop.value_ptr.object.data); } gop.value_ptr.* = .{ .object = object, .lru_node = lru_node }; cache.byte_size += object.data.len; - cache.lru_nodes.append(lru_node); + cache.lru_nodes.append(&lru_node.node); + cache.lru_nodes_len += 1; - while (cache.byte_size > max_byte_size and cache.lru_nodes.len > 1) { + while (cache.byte_size > max_byte_size and cache.lru_nodes_len > 1) { // The > 1 check is to make sure that we don't evict the most // recently added node, even if it by itself happens to exceed the // maximum size of the cache. - const evict_node = cache.lru_nodes.popFirst().?; + const evict_node: *LruListNode = @alignCast(@fieldParentPtr("node", cache.lru_nodes.popFirst().?)); + cache.lru_nodes_len -= 1; const evict_offset = evict_node.data; allocator.destroy(evict_node); const evict_object = cache.objects.get(evict_offset).?.object;