Skip to content

Commit bf3a87c

Browse files
authored
Rollup merge of #160813 - fereidani:linked_list_optimization, r=clarfonthey
Optimize linked list iterator performance This PR optimizes the linked list iterator performance by removing an extra branch check. This resulted in less instructions for single access(`last()`, `next()`, etc.) and optimal iterator optimization(results in longer instructions) as the compiler requires only check for `len==0` exit, I expect slight binary-size increase for much better performance. I did my best to find a counter-example where `len > 0` but head or tail are not initialized, I couldn't. Could you please double-check the claim just in case I missed something? I tested it using `alloctests` available benchmarks too, It seems that compiler is optimizing the benchmark test away, results seems weird(too good), I tried using `black_box` around the object but results stayed the same(0.18ns/iter from 60.49ns/iter): ```rust #[bench] fn bench_iter(b: &mut Bencher) { let v = &[0; 128]; let m: LinkedList<_> = v.iter().cloned().collect(); b.iter(|| { assert!(black_box(&m).iter().count() == 128); }) } ``` *9950x results:* Current: ``` linked_list::bench_collect_into 396.14ns/iter +/- 3.12 linked_list::bench_iter 60.49ns/iter +/- 0.91 linked_list::bench_iter_mut 60.32ns/iter +/- 0.97 linked_list::bench_iter_mut_rev 61.72ns/iter +/- 0.06 linked_list::bench_iter_rev 59.61ns/iter +/- 0.08 linked_list::bench_push_back 10.06ns/iter +/- 0.33 linked_list::bench_push_back_pop_back 4.54ns/iter +/- 0.12 linked_list::bench_push_front 2.87ns/iter +/- 0.05 linked_list::bench_push_front_pop_front 4.52ns/iter +/- 0.05 ``` New: ``` linked_list::bench_collect_into 391.06ns/iter +/- 24.49 linked_list::bench_iter 0.18ns/iter +/- 0.00 linked_list::bench_iter_mut 0.18ns/iter +/- 0.00 linked_list::bench_iter_mut_rev 0.18ns/iter +/- 0.00 linked_list::bench_iter_rev 0.18ns/iter +/- 0.00 linked_list::bench_push_back 10.11ns/iter +/- 0.18 linked_list::bench_push_back_pop_back 4.35ns/iter +/- 0.10 linked_list::bench_push_front 3.27ns/iter +/- 0.46 linked_list::bench_push_front_pop_front 4.66ns/iter +/- 0.04 ```
2 parents 49cd331 + 3f07563 commit bf3a87c

2 files changed

Lines changed: 72 additions & 50 deletions

File tree

library/alloc/src/collections/linked_list.rs

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
use core::alloc::AllocatorClone;
1616
use core::cmp::Ordering;
1717
use core::hash::{Hash, Hasher};
18-
use core::iter::FusedIterator;
18+
use core::iter::{FusedIterator, TrustedLen};
1919
use core::marker::PhantomData;
2020
use core::ptr::NonNull;
2121
use core::{fmt, mem};
@@ -1201,16 +1201,18 @@ impl<'a, T> Iterator for Iter<'a, T> {
12011201
#[inline]
12021202
fn next(&mut self) -> Option<&'a T> {
12031203
if self.len == 0 {
1204-
None
1205-
} else {
1206-
self.head.map(|node| unsafe {
1207-
// Need an unbound lifetime to get 'a
1208-
let node = &*node.as_ptr();
1209-
self.len -= 1;
1210-
self.head = node.next;
1211-
&node.element
1212-
})
1204+
return None;
12131205
}
1206+
// SAFETY: When `len > 0`, `head` and `tail` are guaranteed to be `Some`.
1207+
// The lifetime of the returned reference is bound to the lifetime of the iterator,
1208+
// which is valid because the iterator holds a reference to the list.
1209+
Some(unsafe {
1210+
// Need an unbound lifetime to get 'a
1211+
let node = &*self.head.unwrap_unchecked().as_ptr();
1212+
self.len -= 1;
1213+
self.head = node.next;
1214+
&node.element
1215+
})
12141216
}
12151217

12161218
#[inline]
@@ -1229,16 +1231,18 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> {
12291231
#[inline]
12301232
fn next_back(&mut self) -> Option<&'a T> {
12311233
if self.len == 0 {
1232-
None
1233-
} else {
1234-
self.tail.map(|node| unsafe {
1235-
// Need an unbound lifetime to get 'a
1236-
let node = &*node.as_ptr();
1237-
self.len -= 1;
1238-
self.tail = node.prev;
1239-
&node.element
1240-
})
1234+
return None;
12411235
}
1236+
// SAFETY: When `len > 0`, `head` and `tail` are guaranteed to be `Some`.
1237+
// The lifetime of the returned reference is bound to the lifetime of the iterator,
1238+
// which is valid because the iterator holds a reference to the list.
1239+
Some(unsafe {
1240+
// Need an unbound lifetime to get 'a
1241+
let node = &*self.tail.unwrap_unchecked().as_ptr();
1242+
self.len -= 1;
1243+
self.tail = node.prev;
1244+
&node.element
1245+
})
12421246
}
12431247
}
12441248

@@ -1248,6 +1252,9 @@ impl<T> ExactSizeIterator for Iter<'_, T> {}
12481252
#[stable(feature = "fused", since = "1.26.0")]
12491253
impl<T> FusedIterator for Iter<'_, T> {}
12501254

1255+
#[unstable(feature = "trusted_len", issue = "37572")]
1256+
unsafe impl<T> TrustedLen for Iter<'_, T> {}
1257+
12511258
#[stable(feature = "default_iters", since = "1.70.0")]
12521259
impl<T> Default for Iter<'_, T> {
12531260
/// Creates an empty `linked_list::Iter`.
@@ -1269,16 +1276,18 @@ impl<'a, T> Iterator for IterMut<'a, T> {
12691276
#[inline]
12701277
fn next(&mut self) -> Option<&'a mut T> {
12711278
if self.len == 0 {
1272-
None
1273-
} else {
1274-
self.head.map(|node| unsafe {
1275-
// Need an unbound lifetime to get 'a
1276-
let node = &mut *node.as_ptr();
1277-
self.len -= 1;
1278-
self.head = node.next;
1279-
&mut node.element
1280-
})
1279+
return None;
12811280
}
1281+
// SAFETY: When `len > 0`, `head` and `tail` are guaranteed to be `Some`.
1282+
// The lifetime of the returned reference is bound to the lifetime of the iterator,
1283+
// which is valid because the iterator holds a reference to the list.
1284+
Some(unsafe {
1285+
// Need an unbound lifetime to get 'a
1286+
let node = &mut *self.head.unwrap_unchecked().as_ptr();
1287+
self.len -= 1;
1288+
self.head = node.next;
1289+
&mut node.element
1290+
})
12821291
}
12831292

12841293
#[inline]
@@ -1297,16 +1306,18 @@ impl<'a, T> DoubleEndedIterator for IterMut<'a, T> {
12971306
#[inline]
12981307
fn next_back(&mut self) -> Option<&'a mut T> {
12991308
if self.len == 0 {
1300-
None
1301-
} else {
1302-
self.tail.map(|node| unsafe {
1303-
// Need an unbound lifetime to get 'a
1304-
let node = &mut *node.as_ptr();
1305-
self.len -= 1;
1306-
self.tail = node.prev;
1307-
&mut node.element
1308-
})
1309+
return None;
13091310
}
1311+
// SAFETY: When `len > 0`, `head` and `tail` are guaranteed to be `Some`.
1312+
// The lifetime of the returned reference is bound to the lifetime of the iterator,
1313+
// which is valid because the iterator holds a reference to the list.
1314+
Some(unsafe {
1315+
// Need an unbound lifetime to get 'a
1316+
let node = &mut *self.tail.unwrap_unchecked().as_ptr();
1317+
self.len -= 1;
1318+
self.tail = node.prev;
1319+
&mut node.element
1320+
})
13101321
}
13111322
}
13121323

@@ -1316,6 +1327,9 @@ impl<T> ExactSizeIterator for IterMut<'_, T> {}
13161327
#[stable(feature = "fused", since = "1.26.0")]
13171328
impl<T> FusedIterator for IterMut<'_, T> {}
13181329

1330+
#[unstable(feature = "trusted_len", issue = "37572")]
1331+
unsafe impl<T> TrustedLen for IterMut<'_, T> {}
1332+
13191333
#[stable(feature = "default_iters", since = "1.70.0")]
13201334
impl<T> Default for IterMut<'_, T> {
13211335
fn default() -> Self {
@@ -2032,6 +2046,9 @@ impl<T, A: Allocator> ExactSizeIterator for IntoIter<T, A> {}
20322046
#[stable(feature = "fused", since = "1.26.0")]
20332047
impl<T, A: Allocator> FusedIterator for IntoIter<T, A> {}
20342048

2049+
#[unstable(feature = "trusted_len", issue = "37572")]
2050+
unsafe impl<T, A: Allocator> TrustedLen for IntoIter<T, A> {}
2051+
20352052
#[stable(feature = "default_iters", since = "1.70.0")]
20362053
impl<T> Default for IntoIter<T> {
20372054
/// Creates an empty `linked_list::IntoIter`.

library/alloctests/benches/linked_list.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::collections::LinkedList;
22

3-
use test::Bencher;
3+
use test::{Bencher, black_box};
44

55
#[bench]
66
fn bench_collect_into(b: &mut Bencher) {
@@ -44,35 +44,40 @@ fn bench_push_front_pop_front(b: &mut Bencher) {
4444
})
4545
}
4646

47+
#[bench]
48+
fn bench_iter_count(b: &mut Bencher) {
49+
let m: LinkedList<_> = (0..128).collect();
50+
b.iter(|| {
51+
assert!(black_box(&m).iter().count() == 128);
52+
})
53+
}
54+
4755
#[bench]
4856
fn bench_iter(b: &mut Bencher) {
49-
let v = &[0; 128];
50-
let m: LinkedList<_> = v.iter().cloned().collect();
57+
let m: LinkedList<usize> = (0..128).collect();
5158
b.iter(|| {
52-
assert!(m.iter().count() == 128);
59+
assert!((0..128).sum::<usize>() == black_box(&m).iter().sum());
5360
})
5461
}
62+
5563
#[bench]
5664
fn bench_iter_mut(b: &mut Bencher) {
57-
let v = &[0; 128];
58-
let mut m: LinkedList<_> = v.iter().cloned().collect();
65+
let mut m: LinkedList<usize> = (0..128).collect();
5966
b.iter(|| {
60-
assert!(m.iter_mut().count() == 128);
67+
black_box(&mut m).iter_mut().for_each(|x| *x += 1);
6168
})
6269
}
6370
#[bench]
6471
fn bench_iter_rev(b: &mut Bencher) {
65-
let v = &[0; 128];
66-
let m: LinkedList<_> = v.iter().cloned().collect();
72+
let m: LinkedList<usize> = (0..128).collect();
6773
b.iter(|| {
68-
assert!(m.iter().rev().count() == 128);
74+
assert!((0..128).sum::<usize>() == black_box(&m).iter().rev().sum());
6975
})
7076
}
7177
#[bench]
7278
fn bench_iter_mut_rev(b: &mut Bencher) {
73-
let v = &[0; 128];
74-
let mut m: LinkedList<_> = v.iter().cloned().collect();
79+
let mut m: LinkedList<usize> = (0..128).collect();
7580
b.iter(|| {
76-
assert!(m.iter_mut().rev().count() == 128);
81+
black_box(&mut m).iter_mut().rev().for_each(|x| *x += 1);
7782
})
7883
}

0 commit comments

Comments
 (0)