From e47469620b0656c0fd487d2e1599e2a47832802f Mon Sep 17 00:00:00 2001 From: chiri Date: Tue, 4 Aug 2026 18:05:26 +0300 Subject: [PATCH 1/4] optimize `linked_list` --- Cargo.lock | 2 +- src/internal/linked_list.rs | 608 ++++++++++++++++++++++-------------- 2 files changed, 374 insertions(+), 236 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 53ee21b..8251810 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -37,7 +37,7 @@ checksum = "72f5acc6cb2ba439de613abc23857ec3d78374d8ed5ac84e9d11336e87da8649" [[package]] name = "cachebox" -version = "6.2.1" +version = "6.2.2" dependencies = [ "allocator-api2", "cfg-if", diff --git a/src/internal/linked_list.rs b/src/internal/linked_list.rs index c6289f4..f7f5757 100644 --- a/src/internal/linked_list.rs +++ b/src/internal/linked_list.rs @@ -1,172 +1,222 @@ +use std::alloc::alloc; +use std::alloc::dealloc; +use std::alloc::handle_alloc_error; +use std::alloc::Layout; use std::marker::PhantomData; use std::mem; use std::ptr::NonNull; +use std::ptr::{self}; -/// [`LinkedList`]'s node -pub struct Node { - next: Option>>, - prev: Option>>, - element: T, +/// Intrusive doubly-linked pointers shared by every node and the sentinel. +pub struct Links { + prev: *mut Links, + next: *mut Links, } -impl Node { - fn new(element: T) -> Self { - Node { - next: None, - prev: None, - element, +impl Links { + /// Returns a `Links` with both pointers null. + #[inline] + const fn empty() -> Self { + Links { + prev: ptr::null_mut(), + next: ptr::null_mut(), } } - - #[allow(clippy::boxed_local)] - fn into_element(self: Box) -> T { - self.element - } - - pub fn element(&self) -> &T { - &self.element - } } -/// A doubly-linked list with owned nodes. +/// A single list element: link pointers plus the stored value. /// -/// The `LinkedList` allows pushing and popping elements at either end -/// in constant time. +/// `#[repr(C)]` guarantees `links` is the first field at offset 0, so a +/// `*mut Links` obtained from traversing the list can always be reinterpreted +/// as `*mut Node` (and vice versa) - this is what lets `Cursor` and the +/// sentinel-based traversal in `push_front_node`/`unlink_node` operate on +/// plain `Links` pointers without knowing `T`. +#[repr(C)] +pub struct Node { + links: Links, + element: T, +} + +/// A doubly-linked list with an internal free-list for node reuse. pub struct LinkedList { - head: Option>>, - tail: Option>>, + sentinel: NonNull, + free_head: *mut Links, len: usize, _marker: PhantomData>>, } -// private methods impl LinkedList { - /// Adds the given node to the front of the list. + /// Adds `node` to the front of the list. /// /// # Safety - /// `node` must point to a valid node that was boxed and leaked using the list's allocator. - /// This method takes ownership of the node, so the pointer should not be used again. + /// + /// - `node` must point to a valid, currently unlinked `Links` belonging + /// to this list's allocations (freshly allocated via `alloc_node`/`get_node`, + /// or previously unlinked via `unlink_node`/`pop_*_node`). + /// + /// - The caller must not use `node` again as an "unlinked" node until it is + /// unlinked again. #[inline] - unsafe fn push_front_node(&mut self, node: NonNull>) { - // This method takes care not to create mutable references to whole nodes, - // to maintain validity of aliasing pointers into `element`. + unsafe fn push_front_node(&mut self, node: NonNull) { + let s = self.sentinel.as_ptr(); + let n = node.as_ptr(); unsafe { - (*node.as_ptr()).next = self.head; - (*node.as_ptr()).prev = None; - let node = Some(node); - - match self.head { - None => self.tail = node, - // Not creating new mutable (unique!) references overlapping `element`. - Some(head) => (*head.as_ptr()).prev = node, - } + // SAFETY: `s` is the sentinel, always valid; `n` is valid per caller contract. + let first = (*s).next; + (*n).prev = s; + (*n).next = first; + (*first).prev = n; + (*s).next = n; + } + } - self.head = node; - self.len += 1; + /// Adds `node` to the back of the list. + /// + /// # Safety + /// + /// Same contract as [`push_front_node`](Self::push_front_node). + #[inline] + unsafe fn push_back_node(&mut self, node: NonNull) { + let s = self.sentinel.as_ptr(); + let n = node.as_ptr(); + unsafe { + // SAFETY: `s` is the sentinel, always valid; `n` is valid per caller contract. + let last = (*s).prev; + (*n).next = s; + (*n).prev = last; + (*last).next = n; + (*s).prev = n; } } - /// Removes and returns the node at the front of the list. + /// Removes and returns the node at the front of the list, if any. #[inline] - fn pop_front_node(&mut self) -> Option>> { - // This method takes care not to create mutable references to whole nodes, - // to maintain validity of aliasing pointers into `element`. - self.head.map(|node| unsafe { - let node = Box::from_raw(node.as_ptr()); - self.head = node.next; - - match self.head { - None => self.tail = None, - // Not creating new mutable (unique!) references overlapping `element`. - Some(head) => (*head.as_ptr()).prev = None, - } + fn pop_front_node(&mut self) -> Option> { + if self.len == 0 { + return None; + } + let s = self.sentinel.as_ptr(); + let node = unsafe { + // SAFETY: `self.len != 0`, so `(*s).next` points to a real node, + // and its `next` (`second`) is either another real node or `s` itself. + let node = (*s).next; + let second = (*node).next; + (*second).prev = s; + (*s).next = second; + node + }; + self.len -= 1; + // SAFETY: `node` was just read from a valid linked node, hence non-null. + Some(unsafe { NonNull::new_unchecked(node) }) + } - self.len -= 1; + /// Removes and returns the node at the back of the list, if any. + #[inline] + fn pop_back_node(&mut self) -> Option> { + if self.len == 0 { + return None; + } + let s = self.sentinel.as_ptr(); + let node = unsafe { + // SAFETY: `self.len != 0`, so `(*s).prev` points to a real node. + let node = (*s).prev; + let before = (*node).prev; + (*before).next = s; + (*s).prev = before; node - }) + }; + self.len -= 1; + // SAFETY: `node` was just read from a valid linked node, hence non-null. + Some(unsafe { NonNull::new_unchecked(node) }) } - /// Adds the given node to the back of the list. + /// Unlinks `node` from the list, without deallocating or reading its element. /// /// # Safety - /// `node` must point to a valid node that was boxed and leaked using the list's allocator. - /// This method takes ownership of the node, so the pointer should not be used again. + /// + /// `node` must point to a node currently linked into this list (not the sentinel). #[inline] - unsafe fn push_back_node(&mut self, node: NonNull>) { - // This method takes care not to create mutable references to whole nodes, - // to maintain validity of aliasing pointers into `element`. + unsafe fn unlink_node(&mut self, node: NonNull) { + let n = node.as_ptr(); unsafe { - (*node.as_ptr()).next = None; - (*node.as_ptr()).prev = self.tail; - let node = Some(node); - - match self.tail { - None => self.head = node, - // Not creating new mutable (unique!) references overlapping `element`. - Some(tail) => (*tail.as_ptr()).next = node, - } - - self.tail = node; - self.len += 1; + // SAFETY: caller guarantees `n` is linked, so `prev`/`next` point to + // valid nodes (or the sentinel). + let prev = (*n).prev; + let next = (*n).next; + (*prev).next = next; + (*next).prev = prev; } } - /// Removes and returns the node at the back of the list. + /// Pushes `node` onto the internal free list for reuse. + /// + /// # Safety + /// + /// `node` must be unlinked and its `element` must already be logically + /// moved out (dropped or read via `ptr::read`) — this only reuses the + /// `Links`/allocation, not the `T` storage. #[inline] - fn pop_back_node(&mut self) -> Option>> { - // This method takes care not to create mutable references to whole nodes, - // to maintain validity of aliasing pointers into `element`. - self.tail.map(|node| unsafe { - let node = Box::from_raw(node.as_ptr()); - self.tail = node.prev; - - match self.tail { - None => self.head = None, - // Not creating new mutable (unique!) references overlapping `element`. - Some(tail) => (*tail.as_ptr()).next = None, - } - - self.len -= 1; - node - }) + unsafe fn recycle_node(&mut self, node: *mut Links) { + let free_head = self.free_head; + unsafe { + // SAFETY: `node` is valid per caller contract; writing `next` does + // not touch `element`. + (*node).next = free_head; + } + self.free_head = node; } - /// Unlinks the specified node from the current list. + /// Reads the element out of `node` and recycles the node's storage. /// - /// Warning: this will not check that the provided node belongs to the current list. + /// # Safety /// - /// This method takes care not to create mutable references to `element`, to - /// maintain validity of aliasing pointers. + /// `node` must point to a node whose `element` is initialized and which is + /// no longer linked into the list (already unlinked by the caller). #[inline] - unsafe fn unlink_node(&mut self, mut node: NonNull>) { - let node = unsafe { node.as_mut() }; // this one is ours now, we can create an &mut. - - // Not creating new mutable (unique!) references overlapping `element`. - match node.prev { - Some(prev) => unsafe { (*prev.as_ptr()).next = node.next }, - // this node is the head node - None => self.head = node.next, - }; - - match node.next { - Some(next) => unsafe { (*next.as_ptr()).prev = node.prev }, - // this node is the tail node - None => self.tail = node.prev, - }; + unsafe fn take_element_and_recycle(&mut self, node: NonNull) -> T { + let node_ptr = node.as_ptr() as *mut Node; + // SAFETY: caller guarantees `element` is initialized; reading it does + // not run its destructor, so no double-drop. + let element = unsafe { ptr::read(&(*node_ptr).element) }; + // SAFETY: the element has been logically moved out; the node is safe to recycle. + unsafe { self.recycle_node(node.as_ptr()) }; + element + } + /// Unlinks `node` from the list and returns its element. + /// + /// # Safety + /// + /// `node` must point to a node currently linked into this list + #[inline] + unsafe fn remove_node(&mut self, node: NonNull) -> T { + // SAFETY: caller guarantees `node` is linked. + unsafe { self.unlink_node(node) }; self.len -= 1; + // SAFETY: `node` was just unlinked, and its `element` is still initialized. + unsafe { self.take_element_and_recycle(node) } } - /// Unlinks the specified node from the current list and returns the item. + /// Frees every entry on the internal free list. /// /// # Safety - /// This will not check that the provided node belongs to the current list. - unsafe fn remove_node(&mut self, node: NonNull>) -> T { - unsafe { - self.unlink_node(node); - let node = Box::from_raw(node.as_ptr()); - node.element + /// + /// Must only be called when no other references into the free list exist + /// (e.g. from `Drop`); each recycled node must have been allocated with + /// `Layout::new::>()`. + unsafe fn drop_freelist(&mut self) { + let mut node = self.free_head; + while !node.is_null() { + // SAFETY: `node` is a non-null pointer previously pushed by + // `recycle_node`, so it points to a valid `Node` allocation. + let next = unsafe { (*node).next }; + // SAFETY: `node` was allocated via the global allocator with this + // exact layout (see `alloc_node`/`LinkedList::new`), and is not + // used again afterward. + unsafe { dealloc(node as *mut u8, Layout::new::>()) }; + node = next; } + self.free_head = ptr::null_mut(); } } @@ -182,153 +232,207 @@ impl LinkedList { /// Creates an empty `LinkedList`. #[inline] #[must_use] - pub const fn new() -> Self { + pub fn new() -> Self { + let layout = Layout::new::(); + let raw = unsafe { + let p = alloc(layout) as *mut Links; + if p.is_null() { + handle_alloc_error(layout); + } + // SAFETY: `p` was just allocated with `layout` and checked non-null + // above; writing a fresh self-referential `Links` into it is a + // valid initialization of that memory. + ptr::write(p, Links { prev: p, next: p }); + p + }; + // SAFETY: `p` was checked non-null (or diverged via `handle_alloc_error`). + let sentinel = unsafe { NonNull::new_unchecked(raw) }; LinkedList { - head: None, - tail: None, + sentinel, + free_head: ptr::null_mut(), len: 0, _marker: PhantomData, } } - /// Returns `true` if the `LinkedList` is empty. - /// - /// This operation should compute in *O*(1) time. + /// Returns `true` if the list contains no elements. #[inline] #[must_use] pub fn is_empty(&self) -> bool { - self.head.is_none() + self.len == 0 } - /// Returns the length of the `LinkedList`. - /// - /// This operation should compute in *O*(1) time. + /// Returns the number of elements in the list. #[inline] #[must_use] pub fn len(&self) -> usize { self.len } - /// Removes all elements from the `LinkedList`. - /// - /// This operation should compute in *O*(*n*) time. + /// Removes all elements, dropping each element's value. #[inline] pub fn clear(&mut self) { - drop(LinkedList { - head: self.head.take(), - tail: self.tail.take(), - len: mem::take(&mut self.len), - _marker: PhantomData, - }); + self.drop_nodes(); } - /// Returns a [`Cursor`] to the front node, or `None` if the list is empty. + /// Returns a cursor to the front element, or `None` if the list is empty. #[inline] #[must_use] pub fn cursor_front(&self) -> Option> { - self.head.map(Cursor::new) - } - - /// Returns a [`Cursor`] to the back node, or `None` if the list is empty. + if self.len == 0 { + return None; + } + let s = self.sentinel.as_ptr(); + // SAFETY: sentinel is always valid; `self.len != 0` guarantees + // `(*s).next` points to a real, linked node rather than back to `s`. + let node = unsafe { (*s).next }; + // SAFETY: `node` is non-null (established above) and, by `#[repr(C)]` + // on `Node`, `*mut Links` and `*mut Node` share the same address. + Some(Cursor(unsafe { + NonNull::new_unchecked(node as *mut Node) + })) + } + + /// Returns a cursor to the back element, or `None` if the list is empty. #[inline] #[must_use] pub fn cursor_back(&self) -> Option> { - self.tail.map(Cursor::new) + if self.len == 0 { + return None; + } + let s = self.sentinel.as_ptr(); + // SAFETY: sentinel is always valid; `self.len != 0` guarantees + // `(*s).prev` points to a real, linked node. + let node = unsafe { (*s).prev }; + // SAFETY: same reasoning as in `cursor_front`. + Some(Cursor(unsafe { + NonNull::new_unchecked(node as *mut Node) + })) } - /// Adds an element to the front of the list and returns a [`Cursor`] to it. - /// - /// This operation should compute in *O*(1) time. + /// Inserts `elt` at the front of the list and returns a cursor to it. #[inline] pub fn push_front(&mut self, elt: T) -> Cursor { - let node = Box::new(Node::new(elt)); - let node_ptr = NonNull::from(Box::leak(node)); + let node = self.get_node(elt); + // SAFETY: `get_node` always returns a non-null, freshly-usable `Node` pointer. + let links = unsafe { NonNull::new_unchecked(node as *mut Links) }; - // SAFETY: node_ptr is a unique pointer to a node we boxed with self.alloc and leaked - unsafe { - self.push_front_node(node_ptr); - } - Cursor::new(node_ptr) + // SAFETY: `links` refers to a node that was just obtained (allocated + // or recycled) and is not linked into any list yet. + unsafe { self.push_front_node(links) }; + self.len += 1; + // SAFETY: `node` is the same non-null pointer validated above. + Cursor(unsafe { NonNull::new_unchecked(node) }) } - /// Removes the first element and returns it, or `None` if the list is - /// empty. - /// - /// This operation should compute in *O*(1) time. + /// Removes and returns the front element, or `None` if the list is empty. #[inline] pub fn pop_front(&mut self) -> Option { - self.pop_front_node().map(Node::into_element) + let node = self.pop_front_node()?; + // SAFETY: `node` was just unlinked by `pop_front_node`, so its + // `element` is still initialized and it is safe to take/recycle. + let element = unsafe { self.take_element_and_recycle(node) }; + Some(element) } - /// Adds an element to the back of the list and returns a [`Cursor`] to it. - /// - /// This operation should compute in *O*(1) time. + /// Inserts `elt` at the back of the list and returns a cursor to it. #[inline] pub fn push_back(&mut self, elt: T) -> Cursor { - let node = Box::new(Node::new(elt)); - let node_ptr = NonNull::from(Box::leak(node)); + let node = self.get_node(elt); + // SAFETY: `get_node` always returns a non-null, freshly-usable `Node` pointer. + let links = unsafe { NonNull::new_unchecked(node as *mut Links) }; - // SAFETY: node_ptr is a unique pointer to a node we boxed with self.alloc and leaked - unsafe { - self.push_back_node(node_ptr); - } - Cursor::new(node_ptr) + // SAFETY: `links` refers to a node that was just obtained and is + // not linked into any list yet. + unsafe { self.push_back_node(links) }; + self.len += 1; + // SAFETY: `node` is the same non-null pointer validated above. + Cursor(unsafe { NonNull::new_unchecked(node) }) } - /// Removes the last element from a list and returns it, or `None` if - /// it is empty. - /// - /// This operation should compute in *O*(1) time. + /// Removes and returns the back element, or `None` if the list is empty. #[inline] pub fn pop_back(&mut self) -> Option { - self.pop_back_node().map(Node::into_element) + let node = self.pop_back_node()?; + // SAFETY: `node` was just unlinked by `pop_back_node`, so its + // `element` is still initialized and it is safe to take/recycle. + let element = unsafe { self.take_element_and_recycle(node) }; + Some(element) } - /// Returns a raw, lifetime-free iterator over the nodes of a LinkedList. + /// Returns a raw, unsynchronized iterator over cursors into the list. /// /// # Safety - /// The iterator must not outlive the list it was created from, and the list must not be structurally modified. + /// + /// The caller must not mutate or drop the list while the returned + /// `RawIter` (or any `Cursor` obtained from it) is in use, and must not + /// call [`Cursor::unlink`], [`Cursor::move_to_front`], or + /// [`Cursor::move_to_back`] on a yielded cursor while iteration is still + /// in progress, since that would invalidate `RawIter::next`. + #[inline] pub unsafe fn iter(&self) -> RawIter { + let s = self.sentinel.as_ptr(); + // SAFETY: sentinel is always valid, regardless of `self.len`. + let next = unsafe { (*s).next }; RawIter { - head: self.head, + next, len: self.len, + _marker: PhantomData, } } } +// Guard ensures that if dropping an element panics, we still free +// the free-list allocations instead of leaking them (the sentinel +// is freed unconditionally below regardless of panics). +struct DropGuard<'a, T>(&'a mut LinkedList); + impl Drop for LinkedList { fn drop(&mut self) { - struct DropGuard<'a, T>(&'a mut LinkedList); impl<'a, T> Drop for DropGuard<'a, T> { fn drop(&mut self) { - // Continue the same loop we do below. This only runs when a destructor has - // panicked. If another one panics this will abort. - while self.0.pop_front_node().is_some() {} + self.0.drop_nodes(); + // SAFETY: called only during unwind cleanup, after `drop_nodes` + // has already run (or partially run); the free list is not + // accessed again afterward. + unsafe { self.0.drop_freelist() }; } } - // Wrap self so that if a destructor panics, we can try to keep looping let guard = DropGuard(self); - while guard.0.pop_front_node().is_some() {} + guard.0.drop_nodes(); + // SAFETY: `drop_nodes` completed without panicking; the free list is + // only touched here and then never again (guard is forgotten next). + unsafe { guard.0.drop_freelist() }; mem::forget(guard); + // SAFETY: `self.sentinel` was allocated in `new` with exactly this + // layout (`Layout::new::()`), and after this point `self` is + // being destroyed, so the pointer is never dereferenced again. + unsafe { + dealloc(self.sentinel.as_ptr() as *mut u8, Layout::new::()); + } } } -/// An opaque handle to a node in a [`LinkedList`]. -/// -/// Obtained via [`LinkedList::push_front`], [`LinkedList::push_back`], -/// [`LinkedList::cursor_front`], or [`LinkedList::cursor_back`]. +/// A handle to a single node in a `LinkedList`. /// -/// `Cursor` is `Copy`; cloning or copying it produces a second handle to the -/// *same* node. Two cursors compare equal iff they point at the same node. +/// `Cursor` is a thin, `Copy`able pointer to a node, obtained from +/// [`LinkedList::push_front`], [`LinkedList::push_back`], +/// [`LinkedList::cursor_front`], or [`LinkedList::cursor_back`], and used to +/// later access or reposition that node via [`Cursor::element`], +/// [`Cursor::move_to_front`], [`Cursor::move_to_back`], or [`Cursor::unlink`]. /// -/// # Safety invariant -/// Every `unsafe` method on `Cursor` requires that: -/// - the cursor was obtained from the list it is passed to, **and** -/// - the node has not yet been removed from that list. +/// `#[repr(transparent)]` over `NonNull>` means a `Cursor` has the +/// exact same layout as the raw pointer it wraps — no extra state is tracked, +/// so the cursor does *not* know which `LinkedList` it came from, whether the +/// node is still linked, or whether other `Cursor`s alias the same node. All +/// of that is the caller's responsibility, which is why every non-trivial +/// method on `Cursor` is `unsafe`. /// -/// Violating either condition is undefined behaviour. +/// Because it is just a pointer, `Cursor` is cheaply `Copy`/`Clone`, and +/// equality (`PartialEq`/`Eq`) compares the underlying pointer, i.e. identity +/// of the node, not the value of the element. #[repr(transparent)] pub struct Cursor(NonNull>); @@ -351,78 +455,107 @@ impl PartialEq for Cursor { impl Eq for Cursor {} impl Cursor { + /// Returns the underlying node as a `Links` pointer, for use with the + /// list's internal `Links`-based operations. + /// + /// Relies on `#[repr(C)]` on `Node` placing `links` at offset 0, so the + /// cast is always valid regardless of `T`. #[inline] - fn new(node: NonNull>) -> Self { - Cursor(node) + fn links(&self) -> NonNull { + let ptr = self.0.as_ptr() as *mut Links; + // SAFETY: `self.0` is `NonNull`, so the reinterpreted pointer is non-null too. + unsafe { NonNull::new_unchecked(ptr) } } - /// Returns a shared reference to the element this cursor points to. + /// Returns a reference to the node's element, with a caller-chosen lifetime. /// /// # Safety - /// See the [struct-level safety invariant](Cursor). - /// The returned reference borrows for `'a`, which the caller must - /// ensure does not outlive the node or the list. + /// + /// - The node this cursor points to must still be linked (or otherwise + /// kept alive) in some `LinkedList`, i.e. not yet unlinked/recycled. + /// + /// - The returned `&'a T` must not outlive the underlying allocation, and + /// no `&mut T`/`element_mut` alias to the same node may exist while + /// this reference is live. #[inline] pub unsafe fn element<'a>(&self) -> &'a T { - &(*self.0.as_ptr()).element + let node = self.0.as_ptr(); + // SAFETY: caller guarantees the node is still allocated/linked and + // that no conflicting `&mut` aliases the element. + unsafe { &(*node).element } } - /// Returns a mutable reference to the element this cursor points to. + /// Returns a mutable reference to the node's element, with a caller-chosen lifetime. /// /// # Safety - /// See the [struct-level safety invariant](Cursor). - /// In addition, no other reference to this element may exist for the - /// duration of the returned `'a` borrow. + /// + /// Same contract as [`element`](Self::element), plus: no other reference + /// (shared or mutable) to this node's element may be alive at the same time. #[inline] pub unsafe fn element_mut<'a>(&mut self) -> &'a mut T { - &mut (*self.0.as_ptr()).element + let node = self.0.as_ptr(); + // SAFETY: caller guarantees exclusive access to this node's element. + unsafe { &mut (*node).element } } - /// Moves this node to the front of `list`. + /// Moves the node this cursor points to the front of `list`. /// /// # Safety - /// See the [struct-level safety invariant](Cursor). + /// + /// - The node must currently be linked into `list` (not some other list, + /// and not already unlinked/recycled). + /// + /// - No other `Cursor`/reference into this node may be used concurrently + /// with this call. #[inline] pub unsafe fn move_to_front(self, list: &mut LinkedList) { - list.unlink_node(self.0); - list.push_front_node(self.0); + let links = self.links(); + // SAFETY: caller guarantees `links` is currently linked into `list`. + unsafe { list.unlink_node(links) }; + // SAFETY: `links` was just unlinked above, so it's safe to relink. + unsafe { list.push_front_node(links) }; } - /// Moves this node to the back of `list`. + /// Moves the node this cursor points to the back of `list`. /// /// # Safety - /// See the [struct-level safety invariant](Cursor). + /// + /// Same contract as [`move_to_front`](Self::move_to_front). #[inline] pub unsafe fn move_to_back(self, list: &mut LinkedList) { - list.unlink_node(self.0); - list.push_back_node(self.0); + let links = self.links(); + // SAFETY: caller guarantees `links` is currently linked into `list`. + unsafe { list.unlink_node(links) }; + // SAFETY: `links` was just unlinked above, so it's safe to relink. + unsafe { list.push_back_node(links) }; } - /// Unlinks this node from `list` and returns its element. - /// - /// Consumes the cursor so it cannot be used after removal. + /// Removes the node this cursor points to from `list` and returns its element. /// /// # Safety - /// See the [struct-level safety invariant](Cursor). + /// + /// - The node must currently be linked into `list`. + /// + /// - This consumes the cursor (`self`, by value) because the node is + /// deallocated/recycled afterward — the cursor must not be used again. #[inline] pub unsafe fn unlink(self, list: &mut LinkedList) -> T { - list.remove_node(self.0) + let links = self.links(); + // SAFETY: caller guarantees `links` is currently linked into `list`. + unsafe { list.remove_node(links) } } } -/// A raw, lifetime-free iterator over the nodes of a [`LinkedList`]. +/// A raw, unsynchronized iterator over `Cursor`s in a `LinkedList`. /// -/// Yields a [`Cursor`] for each node, from front to back. -/// -/// Obtained via [`LinkedList::iter`]. -/// -/// # Safety invariant -/// The iterator must not outlive the list it was created from, and the list -/// must not be structurally modified (nodes added or removed) while iterating. -/// Violating either condition is undefined behaviour. +/// Created only via [`LinkedList::iter`], which is itself `unsafe` — see its +/// `# Safety` section for the invariants that make walking `next`/`len` here +/// sound (the list must not be mutated or dropped while this iterator, or any +/// `Cursor` it yields, is in use). pub struct RawIter { - head: Option>>, + next: *mut Links, len: usize, + _marker: PhantomData>>, } impl Iterator for RawIter { @@ -433,12 +566,17 @@ impl Iterator for RawIter { if self.len == 0 { return None; } - self.head.map(|node| { - self.len -= 1; - // SAFETY: node is a valid, live pointer for as long as the list lives. - self.head = unsafe { (*node.as_ptr()).next }; - Cursor::new(node) - }) + let node = self.next; + self.len -= 1; + // SAFETY: `self.len != 0` (checked above) guarantees `node` is a + // currently-valid, linked node, so `(*node).next` is a valid read; + // per `LinkedList::iter`'s contract, the list is not mutated/dropped + // while this iterator is alive. + self.next = unsafe { (*node).next }; + // SAFETY: `node` is non-null (came from a valid linked `Links`), and + // by `#[repr(C)]` on `Node` a `*mut Links` is a valid `*mut Node`. + let cursor = Cursor(unsafe { NonNull::new_unchecked(node as *mut Node) }); + Some(cursor) } #[inline] From 99c66f0b4d72cfa8bcd56c51446f23cea616ea0c Mon Sep 17 00:00:00 2001 From: chiri Date: Tue, 4 Aug 2026 18:13:38 +0300 Subject: [PATCH 2/4] fix --- src/internal/linked_list.rs | 76 +++++++++++++++++++++++++++++++------ 1 file changed, 65 insertions(+), 11 deletions(-) diff --git a/src/internal/linked_list.rs b/src/internal/linked_list.rs index f7f5757..ac78271 100644 --- a/src/internal/linked_list.rs +++ b/src/internal/linked_list.rs @@ -197,6 +197,31 @@ impl LinkedList { unsafe { self.take_element_and_recycle(node) } } + /// Drops every linked element and parks its node on the free list. + /// + /// Elements whose destructors panic are handled by the caller's + /// drop-guard strategy; nodes are recycled even on the panic path. + fn drop_nodes(&mut self) { + let s = self.sentinel.as_ptr(); + while self.len != 0 { + unsafe { + // SAFETY: `self.len != 0`, so `(*s).next` points to a real node. + let node = (*s).next; + let second = (*node).next; + (*second).prev = s; + (*s).next = second; + + let node_ptr = node as *mut Node; + // SAFETY: `#[repr(C)]` puts `links` at offset 0, so `node` is a + // valid `Node` whose `element` is still initialized. + ptr::drop_in_place(&mut (*node_ptr).element); + // SAFETY: the element was just dropped; the node is unlinked. + self.recycle_node(node); + } + self.len -= 1; + } + } + /// Frees every entry on the internal free list. /// /// # Safety @@ -218,6 +243,36 @@ impl LinkedList { } self.free_head = ptr::null_mut(); } + + /// Returns a node holding `elt`: reuses a free-list node when one is + /// available, otherwise allocates a fresh one. + #[inline] + fn get_node(&mut self, elt: T) -> *mut Node { + if !self.free_head.is_null() { + let links = self.free_head; + // SAFETY: `free_head` is non-null here and points to a recycled + // node threaded via `recycle_node`. + self.free_head = unsafe { (*links).next }; + let node = links as *mut Node; + // SAFETY: `#[repr(C)]` puts `links` at offset 0, and only the + // `element` slot of a recycled node is stale; `links` is + // rewritten by the push that follows. + unsafe { ptr::write(&mut (*node).element, elt) }; + node + } else { + Self::alloc_node(elt) + } + } + + /// Cold fallback of `get_node`: allocates a brand-new node. + #[inline(never)] + #[cold] + fn alloc_node(elt: T) -> *mut Node { + Box::into_raw(Box::new(Node { + links: Links::empty(), + element: elt, + })) + } } impl Default for LinkedList { @@ -387,19 +442,18 @@ impl LinkedList { // is freed unconditionally below regardless of panics). struct DropGuard<'a, T>(&'a mut LinkedList); -impl Drop for LinkedList { +impl<'a, T> Drop for DropGuard<'a, T> { fn drop(&mut self) { + self.0.drop_nodes(); + // SAFETY: called only during unwind cleanup, after `drop_nodes` + // has already run (or partially run); the free list is not + // accessed again afterward. + unsafe { self.0.drop_freelist() }; + } +} - impl<'a, T> Drop for DropGuard<'a, T> { - fn drop(&mut self) { - self.0.drop_nodes(); - // SAFETY: called only during unwind cleanup, after `drop_nodes` - // has already run (or partially run); the free list is not - // accessed again afterward. - unsafe { self.0.drop_freelist() }; - } - } - +impl Drop for LinkedList { + fn drop(&mut self) { let guard = DropGuard(self); guard.0.drop_nodes(); // SAFETY: `drop_nodes` completed without panicking; the free list is From 72cb26ee5e7ee84be0c82267b2457a2e48a44538 Mon Sep 17 00:00:00 2001 From: chiri Date: Tue, 4 Aug 2026 20:47:42 +0300 Subject: [PATCH 3/4] add FREE_LIST_CAPACITY --- src/internal/linked_list.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/internal/linked_list.rs b/src/internal/linked_list.rs index ac78271..e05e5ef 100644 --- a/src/internal/linked_list.rs +++ b/src/internal/linked_list.rs @@ -7,6 +7,9 @@ use std::mem; use std::ptr::NonNull; use std::ptr::{self}; +/// Maximum number of nodes parked on the free list +const FREE_LIST_CAPACITY: usize = 256; + /// Intrusive doubly-linked pointers shared by every node and the sentinel. pub struct Links { prev: *mut Links, @@ -37,10 +40,11 @@ pub struct Node { element: T, } -/// A doubly-linked list with an internal free-list for node reuse. +/// A doubly-linked list with an internal, bounded free-list for node reuse. pub struct LinkedList { sentinel: NonNull, free_head: *mut Links, + free_len: usize, len: usize, _marker: PhantomData>>, } @@ -148,7 +152,8 @@ impl LinkedList { } } - /// Pushes `node` onto the internal free list for reuse. + /// Pushes `node` onto the internal free list for reuse, or deallocates + /// it when the free list is already at [`FREE_LIST_CAPACITY`] entries. /// /// # Safety /// @@ -157,6 +162,14 @@ impl LinkedList { /// `Links`/allocation, not the `T` storage. #[inline] unsafe fn recycle_node(&mut self, node: *mut Links) { + if self.free_len >= FREE_LIST_CAPACITY { + // SAFETY: `node` is unlinked with its element moved out per the + // caller contract, and was allocated via the global allocator + // with exactly this layout (see `alloc_node`), the same layout + // `drop_freelist` uses for its deallocations. + unsafe { dealloc(node as *mut u8, Layout::new::>()) }; + return; + } let free_head = self.free_head; unsafe { // SAFETY: `node` is valid per caller contract; writing `next` does @@ -164,6 +177,7 @@ impl LinkedList { (*node).next = free_head; } self.free_head = node; + self.free_len += 1; } /// Reads the element out of `node` and recycles the node's storage. @@ -242,6 +256,7 @@ impl LinkedList { node = next; } self.free_head = ptr::null_mut(); + self.free_len = 0; } /// Returns a node holding `elt`: reuses a free-list node when one is @@ -253,6 +268,7 @@ impl LinkedList { // SAFETY: `free_head` is non-null here and points to a recycled // node threaded via `recycle_node`. self.free_head = unsafe { (*links).next }; + self.free_len -= 1; let node = links as *mut Node; // SAFETY: `#[repr(C)]` puts `links` at offset 0, and only the // `element` slot of a recycled node is stale; `links` is @@ -305,6 +321,7 @@ impl LinkedList { LinkedList { sentinel, free_head: ptr::null_mut(), + free_len: 0, len: 0, _marker: PhantomData, } From fed6f91d7d8b28d423d69d2b703f128d8c58f9f8 Mon Sep 17 00:00:00 2001 From: chiri Date: Tue, 4 Aug 2026 21:02:49 +0300 Subject: [PATCH 4/4] update SAFETY comments --- src/internal/linked_list.rs | 80 ++++++++++++++----------------------- 1 file changed, 30 insertions(+), 50 deletions(-) diff --git a/src/internal/linked_list.rs b/src/internal/linked_list.rs index e05e5ef..73c721d 100644 --- a/src/internal/linked_list.rs +++ b/src/internal/linked_list.rs @@ -65,7 +65,7 @@ impl LinkedList { let s = self.sentinel.as_ptr(); let n = node.as_ptr(); unsafe { - // SAFETY: `s` is the sentinel, always valid; `n` is valid per caller contract. + // SAFETY: sentinel always valid; `n` valid per fn contract. let first = (*s).next; (*n).prev = s; (*n).next = first; @@ -84,7 +84,7 @@ impl LinkedList { let s = self.sentinel.as_ptr(); let n = node.as_ptr(); unsafe { - // SAFETY: `s` is the sentinel, always valid; `n` is valid per caller contract. + // SAFETY: sentinel always valid; `n` valid per fn contract. let last = (*s).prev; (*n).next = s; (*n).prev = last; @@ -101,8 +101,7 @@ impl LinkedList { } let s = self.sentinel.as_ptr(); let node = unsafe { - // SAFETY: `self.len != 0`, so `(*s).next` points to a real node, - // and its `next` (`second`) is either another real node or `s` itself. + // SAFETY: `len != 0` -> `(*s).next` is a real node. let node = (*s).next; let second = (*node).next; (*second).prev = s; @@ -110,7 +109,7 @@ impl LinkedList { node }; self.len -= 1; - // SAFETY: `node` was just read from a valid linked node, hence non-null. + // SAFETY: read from a valid node above, so non-null. Some(unsafe { NonNull::new_unchecked(node) }) } @@ -122,7 +121,7 @@ impl LinkedList { } let s = self.sentinel.as_ptr(); let node = unsafe { - // SAFETY: `self.len != 0`, so `(*s).prev` points to a real node. + // SAFETY: `len != 0` -> `(*s).prev` is a real node. let node = (*s).prev; let before = (*node).prev; (*before).next = s; @@ -130,7 +129,7 @@ impl LinkedList { node }; self.len -= 1; - // SAFETY: `node` was just read from a valid linked node, hence non-null. + // SAFETY: read from a valid node above, so non-null. Some(unsafe { NonNull::new_unchecked(node) }) } @@ -143,8 +142,7 @@ impl LinkedList { unsafe fn unlink_node(&mut self, node: NonNull) { let n = node.as_ptr(); unsafe { - // SAFETY: caller guarantees `n` is linked, so `prev`/`next` point to - // valid nodes (or the sentinel). + // SAFETY: `n` linked per fn contract; `prev`/`next` valid. let prev = (*n).prev; let next = (*n).next; (*prev).next = next; @@ -163,17 +161,14 @@ impl LinkedList { #[inline] unsafe fn recycle_node(&mut self, node: *mut Links) { if self.free_len >= FREE_LIST_CAPACITY { - // SAFETY: `node` is unlinked with its element moved out per the - // caller contract, and was allocated via the global allocator - // with exactly this layout (see `alloc_node`), the same layout - // `drop_freelist` uses for its deallocations. + // SAFETY: unlinked + moved-out per fn contract; allocated with + // this exact layout in `alloc_node`. unsafe { dealloc(node as *mut u8, Layout::new::>()) }; return; } let free_head = self.free_head; unsafe { - // SAFETY: `node` is valid per caller contract; writing `next` does - // not touch `element`. + // SAFETY: valid per fn contract; `next` write doesn't touch `element`. (*node).next = free_head; } self.free_head = node; @@ -189,10 +184,9 @@ impl LinkedList { #[inline] unsafe fn take_element_and_recycle(&mut self, node: NonNull) -> T { let node_ptr = node.as_ptr() as *mut Node; - // SAFETY: caller guarantees `element` is initialized; reading it does - // not run its destructor, so no double-drop. + // SAFETY: `element` initialized per fn contract; `ptr::read` doesn't drop. let element = unsafe { ptr::read(&(*node_ptr).element) }; - // SAFETY: the element has been logically moved out; the node is safe to recycle. + // SAFETY: element moved out above, node now safe to recycle. unsafe { self.recycle_node(node.as_ptr()) }; element } @@ -207,7 +201,7 @@ impl LinkedList { // SAFETY: caller guarantees `node` is linked. unsafe { self.unlink_node(node) }; self.len -= 1; - // SAFETY: `node` was just unlinked, and its `element` is still initialized. + // SAFETY: just unlinked above; `element` still initialized. unsafe { self.take_element_and_recycle(node) } } @@ -226,10 +220,9 @@ impl LinkedList { (*s).next = second; let node_ptr = node as *mut Node; - // SAFETY: `#[repr(C)]` puts `links` at offset 0, so `node` is a - // valid `Node` whose `element` is still initialized. + // SAFETY: layout-compatible (see `Node` docs); `element` still init. ptr::drop_in_place(&mut (*node_ptr).element); - // SAFETY: the element was just dropped; the node is unlinked. + // SAFETY: element dropped above; node already unlinked. self.recycle_node(node); } self.len -= 1; @@ -246,12 +239,9 @@ impl LinkedList { unsafe fn drop_freelist(&mut self) { let mut node = self.free_head; while !node.is_null() { - // SAFETY: `node` is a non-null pointer previously pushed by - // `recycle_node`, so it points to a valid `Node` allocation. + // SAFETY: non-null, pushed by `recycle_node` -> valid `Node` alloc. let next = unsafe { (*node).next }; - // SAFETY: `node` was allocated via the global allocator with this - // exact layout (see `alloc_node`/`LinkedList::new`), and is not - // used again afterward. + // SAFETY: allocated with this exact layout; not reused after this. unsafe { dealloc(node as *mut u8, Layout::new::>()) }; node = next; } @@ -265,14 +255,11 @@ impl LinkedList { fn get_node(&mut self, elt: T) -> *mut Node { if !self.free_head.is_null() { let links = self.free_head; - // SAFETY: `free_head` is non-null here and points to a recycled - // node threaded via `recycle_node`. + // SAFETY: non-null, threaded via `recycle_node`. self.free_head = unsafe { (*links).next }; self.free_len -= 1; let node = links as *mut Node; - // SAFETY: `#[repr(C)]` puts `links` at offset 0, and only the - // `element` slot of a recycled node is stale; `links` is - // rewritten by the push that follows. + // SAFETY: layout-compatible (see `Node` docs); only `element` is stale. unsafe { ptr::write(&mut (*node).element, elt) }; node } else { @@ -310,13 +297,11 @@ impl LinkedList { if p.is_null() { handle_alloc_error(layout); } - // SAFETY: `p` was just allocated with `layout` and checked non-null - // above; writing a fresh self-referential `Links` into it is a - // valid initialization of that memory. + // SAFETY: `p` allocated with `layout`, checked non-null above. ptr::write(p, Links { prev: p, next: p }); p }; - // SAFETY: `p` was checked non-null (or diverged via `handle_alloc_error`). + // SAFETY: non-null checked above (or diverged). let sentinel = unsafe { NonNull::new_unchecked(raw) }; LinkedList { sentinel, @@ -355,11 +340,9 @@ impl LinkedList { return None; } let s = self.sentinel.as_ptr(); - // SAFETY: sentinel is always valid; `self.len != 0` guarantees - // `(*s).next` points to a real, linked node rather than back to `s`. + // SAFETY: sentinel always valid; `len != 0` -> real node, not `s`. let node = unsafe { (*s).next }; - // SAFETY: `node` is non-null (established above) and, by `#[repr(C)]` - // on `Node`, `*mut Links` and `*mut Node` share the same address. + // SAFETY: non-null above; layout-compatible (see `Node` docs). Some(Cursor(unsafe { NonNull::new_unchecked(node as *mut Node) })) @@ -373,10 +356,9 @@ impl LinkedList { return None; } let s = self.sentinel.as_ptr(); - // SAFETY: sentinel is always valid; `self.len != 0` guarantees - // `(*s).prev` points to a real, linked node. + // SAFETY: sentinel always valid; `len != 0` -> real node. let node = unsafe { (*s).prev }; - // SAFETY: same reasoning as in `cursor_front`. + // SAFETY: same as `cursor_front`. Some(Cursor(unsafe { NonNull::new_unchecked(node as *mut Node) })) @@ -386,14 +368,13 @@ impl LinkedList { #[inline] pub fn push_front(&mut self, elt: T) -> Cursor { let node = self.get_node(elt); - // SAFETY: `get_node` always returns a non-null, freshly-usable `Node` pointer. + // SAFETY: `get_node` never returns null. let links = unsafe { NonNull::new_unchecked(node as *mut Links) }; - // SAFETY: `links` refers to a node that was just obtained (allocated - // or recycled) and is not linked into any list yet. + // SAFETY: freshly obtained node, unlinked (see `push_front_node` contract). unsafe { self.push_front_node(links) }; self.len += 1; - // SAFETY: `node` is the same non-null pointer validated above. + // SAFETY: same pointer validated above. Cursor(unsafe { NonNull::new_unchecked(node) }) } @@ -462,9 +443,8 @@ struct DropGuard<'a, T>(&'a mut LinkedList); impl<'a, T> Drop for DropGuard<'a, T> { fn drop(&mut self) { self.0.drop_nodes(); - // SAFETY: called only during unwind cleanup, after `drop_nodes` - // has already run (or partially run); the free list is not - // accessed again afterward. + // SAFETY: `self` is being torn down during unwind; no other + // references to the free list exist, satisfying `drop_freelist`'s contract. unsafe { self.0.drop_freelist() }; } }