Skip to content

Commit d6dec1e

Browse files
committed
Optimize Vec::retain
1 parent efdb859 commit d6dec1e

File tree

2 files changed

+140
-11
lines changed

2 files changed

+140
-11
lines changed

library/alloc/src/vec/mod.rs

Lines changed: 68 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,21 +1371,78 @@ impl<T, A: Allocator> Vec<T, A> {
13711371
F: FnMut(&T) -> bool,
13721372
{
13731373
let len = self.len();
1374-
let mut del = 0;
1375-
{
1376-
let v = &mut **self;
1377-
1378-
for i in 0..len {
1379-
if !f(&v[i]) {
1380-
del += 1;
1381-
} else if del > 0 {
1382-
v.swap(i - del, i);
1374+
// Avoid double drop if the drop guard is not executed,
1375+
// since we may make some holes during the process.
1376+
unsafe { self.set_len(0) };
1377+
1378+
// Vec: [Kept, Kept, Hole, Hole, Hole, Hole, Unchecked, Unchecked]
1379+
// |<- processed len ->| ^- next to check
1380+
// |<- deleted cnt ->|
1381+
// |<- original_len ->|
1382+
// Kept: Elements which predicate returns true on.
1383+
// Hole: Moved or dropped element slot.
1384+
// Unchecked: Unchecked valid elements.
1385+
//
1386+
// This drop guard will be invoked when predicate or `drop` of element panicked.
1387+
// It shifts unchecked elements to cover holes and `set_len` to the correct length.
1388+
// In cases when predicate and `drop` never panick, it will be optimized out.
1389+
struct BackshiftOnDrop<'a, T, A: Allocator> {
1390+
v: &'a mut Vec<T, A>,
1391+
processed_len: usize,
1392+
deleted_cnt: usize,
1393+
original_len: usize,
1394+
}
1395+
1396+
impl<T, A: Allocator> Drop for BackshiftOnDrop<'_, T, A> {
1397+
fn drop(&mut self) {
1398+
if self.deleted_cnt > 0 {
1399+
// SAFETY: Fill the hole of dropped or moved
1400+
unsafe {
1401+
ptr::copy(
1402+
self.v.as_ptr().offset(self.processed_len as isize),
1403+
self.v
1404+
.as_mut_ptr()
1405+
.offset(self.processed_len as isize - self.deleted_cnt as isize),
1406+
self.original_len - self.processed_len,
1407+
);
1408+
self.v.set_len(self.original_len - self.deleted_cnt);
1409+
}
13831410
}
13841411
}
13851412
}
1386-
if del > 0 {
1387-
self.truncate(len - del);
1413+
1414+
let mut guard = BackshiftOnDrop {
1415+
v: self,
1416+
processed_len: 0,
1417+
deleted_cnt: 0,
1418+
original_len: len,
1419+
};
1420+
1421+
let mut del = 0usize;
1422+
for i in 0..len {
1423+
// SAFETY: Unchecked element must be valid.
1424+
let cur = unsafe { &mut *guard.v.as_mut_ptr().offset(i as isize) };
1425+
if !f(cur) {
1426+
del += 1;
1427+
// Advance early to avoid double drop if `drop_in_place` panicked.
1428+
guard.processed_len = i + 1;
1429+
guard.deleted_cnt = del;
1430+
// SAFETY: We never touch this element again after dropped.
1431+
unsafe { ptr::drop_in_place(cur) };
1432+
} else if del > 0 {
1433+
// SAFETY: `del` > 0 so the hole slot must not overlap with current element.
1434+
// We use copy for move, and never touch this element again.
1435+
unsafe {
1436+
let hole_slot = guard.v.as_mut_ptr().offset(i as isize - del as isize);
1437+
ptr::copy_nonoverlapping(cur, hole_slot, 1);
1438+
}
1439+
guard.processed_len = i + 1;
1440+
}
13881441
}
1442+
1443+
// All holes are at the end now. Simply cut them out.
1444+
unsafe { guard.v.set_len(len - del) };
1445+
mem::forget(guard);
13891446
}
13901447

13911448
/// Removes all but the first of consecutive elements in the vector that resolve to the same

library/alloc/tests/vec.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,78 @@ fn test_retain() {
287287
assert_eq!(vec, [2, 4]);
288288
}
289289

290+
#[test]
291+
fn test_retain_pred_panic() {
292+
use std::sync::atomic::{AtomicU64, Ordering};
293+
294+
struct Wrap<'a>(&'a AtomicU64, u64, bool);
295+
296+
impl Drop for Wrap<'_> {
297+
fn drop(&mut self) {
298+
self.0.fetch_or(self.1, Ordering::SeqCst);
299+
}
300+
}
301+
302+
let dropped = AtomicU64::new(0);
303+
304+
let ret = std::panic::catch_unwind(|| {
305+
let mut v = vec![
306+
Wrap(&dropped, 1, false),
307+
Wrap(&dropped, 2, false),
308+
Wrap(&dropped, 4, false),
309+
Wrap(&dropped, 8, false),
310+
Wrap(&dropped, 16, false),
311+
];
312+
v.retain(|w| match w.1 {
313+
1 => true,
314+
2 => false,
315+
4 => true,
316+
_ => panic!(),
317+
});
318+
});
319+
assert!(ret.is_err());
320+
// Everything is dropped when predicate panicked.
321+
assert_eq!(dropped.load(Ordering::SeqCst), 1 | 2 | 4 | 8 | 16);
322+
}
323+
324+
#[test]
325+
fn test_retain_drop_panic() {
326+
use std::sync::atomic::{AtomicU64, Ordering};
327+
328+
struct Wrap<'a>(&'a AtomicU64, u64);
329+
330+
impl Drop for Wrap<'_> {
331+
fn drop(&mut self) {
332+
if self.1 == 8 {
333+
panic!();
334+
}
335+
self.0.fetch_or(self.1, Ordering::SeqCst);
336+
}
337+
}
338+
339+
let dropped = AtomicU64::new(0);
340+
341+
let ret = std::panic::catch_unwind(|| {
342+
let mut v = vec![
343+
Wrap(&dropped, 1),
344+
Wrap(&dropped, 2),
345+
Wrap(&dropped, 4),
346+
Wrap(&dropped, 8),
347+
Wrap(&dropped, 16),
348+
];
349+
v.retain(|w| match w.1 {
350+
1 => true,
351+
2 => false,
352+
4 => true,
353+
8 => false,
354+
_ => true,
355+
});
356+
});
357+
assert!(ret.is_err());
358+
// Other elements are dropped when `drop` of one element panicked.
359+
assert_eq!(dropped.load(Ordering::SeqCst), 1 | 2 | 4 | 16);
360+
}
361+
290362
#[test]
291363
fn test_dedup() {
292364
fn case(a: Vec<i32>, b: Vec<i32>) {

0 commit comments

Comments
 (0)