From ac931203d21c540efb65fcd82a73f47d2e98d1f7 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Tue, 21 Jan 2020 19:03:14 +0100 Subject: [PATCH 1/8] Add extend_chain bench --- src/liballoc/benches/vec.rs | 63 ++++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/src/liballoc/benches/vec.rs b/src/liballoc/benches/vec.rs index a3da9e80cd0fc..2490906794218 100644 --- a/src/liballoc/benches/vec.rs +++ b/src/liballoc/benches/vec.rs @@ -1,4 +1,7 @@ -use std::iter::{repeat, FromIterator}; +use std::{ + hint::black_box, + iter::{repeat, FromIterator}, +}; use test::Bencher; #[bench] @@ -165,14 +168,31 @@ fn bench_from_iter_1000(b: &mut Bencher) { } fn do_bench_extend(b: &mut Bencher, dst_len: usize, src_len: usize) { - let dst: Vec<_> = FromIterator::from_iter(0..dst_len); let src: Vec<_> = FromIterator::from_iter(dst_len..dst_len + src_len); + do_bench_extend_iter(b, dst_len, src_len, src) +} + +fn do_bench_extend_chain(b: &mut Bencher, dst_len: usize, src_len: usize) { + let fst = Vec::from_iter(dst_len..dst_len + src_len / 2); + let snd = Vec::from_iter(dst_len + src_len / 2..dst_len + src_len); + let src = fst.iter().cloned().chain(snd.iter().cloned()); + do_bench_extend_iter(b, dst_len, src_len, src) +} + +#[inline(never)] +fn do_bench_extend_iter( + b: &mut Bencher, + dst_len: usize, + src_len: usize, + src: impl IntoIterator + Clone, +) { + let dst: Vec<_> = FromIterator::from_iter(0..dst_len); b.bytes = src_len as u64; b.iter(|| { - let mut dst = dst.clone(); - dst.extend(src.clone()); + let mut dst = black_box(dst.clone()); + dst.extend(black_box(src.clone())); assert_eq!(dst.len(), dst_len + src_len); assert!(dst.iter().enumerate().all(|(i, x)| i == *x)); }); @@ -213,6 +233,41 @@ fn bench_extend_1000_1000(b: &mut Bencher) { do_bench_extend(b, 1000, 1000) } +#[bench] +fn bench_extend_chain_0000_0000(b: &mut Bencher) { + do_bench_extend_chain(b, 0, 0) +} + +#[bench] +fn bench_extend_chain_0000_0010(b: &mut Bencher) { + do_bench_extend_chain(b, 0, 10) +} + +#[bench] +fn bench_extend_chain_0000_0100(b: &mut Bencher) { + do_bench_extend_chain(b, 0, 100) +} + +#[bench] +fn bench_extend_chain_0000_1000(b: &mut Bencher) { + do_bench_extend_chain(b, 0, 1000) +} + +#[bench] +fn bench_extend_chain_0010_0010(b: &mut Bencher) { + do_bench_extend_chain(b, 10, 10) +} + +#[bench] +fn bench_extend_chain_0100_0100(b: &mut Bencher) { + do_bench_extend_chain(b, 100, 100) +} + +#[bench] +fn bench_extend_chain_1000_1000(b: &mut Bencher) { + do_bench_extend_chain(b, 1000, 1000) +} + fn do_bench_push_all(b: &mut Bencher, dst_len: usize, src_len: usize) { let dst: Vec<_> = FromIterator::from_iter(0..dst_len); let src: Vec<_> = FromIterator::from_iter(dst_len..dst_len + src_len); From cd551b0f23eb1f32cb4bd456b7a28af7de613553 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Wed, 22 Jan 2020 17:36:55 +0100 Subject: [PATCH 2/8] More chain in extend benchmark --- src/liballoc/benches/vec.rs | 31 ++++++++++++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/liballoc/benches/vec.rs b/src/liballoc/benches/vec.rs index 2490906794218..58122e0d155db 100644 --- a/src/liballoc/benches/vec.rs +++ b/src/liballoc/benches/vec.rs @@ -173,9 +173,34 @@ fn do_bench_extend(b: &mut Bencher, dst_len: usize, src_len: usize) { } fn do_bench_extend_chain(b: &mut Bencher, dst_len: usize, src_len: usize) { - let fst = Vec::from_iter(dst_len..dst_len + src_len / 2); - let snd = Vec::from_iter(dst_len + src_len / 2..dst_len + src_len); - let src = fst.iter().cloned().chain(snd.iter().cloned()); + let src = Vec::from_iter(dst_len..dst_len + src_len); + let x1; + let x2; + let x3; + let x4; + let x5; + + if src.len() / 5 == 0 { + x1 = &[][..]; + x2 = &[][..]; + x3 = &[][..]; + x4 = &[][..]; + x5 = &[][..]; + } else { + let mut iter = src.chunks_exact(src.len() / 5); + x1 = iter.next().unwrap_or(&[][..]); + x2 = iter.next().unwrap_or(&[][..]); + x3 = iter.next().unwrap_or(&[][..]); + x4 = iter.next().unwrap_or(&[][..]); + x5 = iter.next().unwrap_or(&[][..]); + } + let src = x1 + .iter() + .cloned() + .chain(x2.iter().cloned()) + .chain(x3.iter().cloned()) + .chain(x4.iter().cloned()) + .chain(x5.iter().cloned()); do_bench_extend_iter(b, dst_len, src_len, src) } From 9b0cdeb1ef55c9a49f7ba406b0b9c1204175a746 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Mon, 27 Jan 2020 11:49:35 +0100 Subject: [PATCH 3/8] Use a more complicated chain in benchmarks --- src/liballoc/benches/vec.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/liballoc/benches/vec.rs b/src/liballoc/benches/vec.rs index 58122e0d155db..0f0ee9362ac19 100644 --- a/src/liballoc/benches/vec.rs +++ b/src/liballoc/benches/vec.rs @@ -194,13 +194,10 @@ fn do_bench_extend_chain(b: &mut Bencher, dst_len: usize, src_len: usize) { x4 = iter.next().unwrap_or(&[][..]); x5 = iter.next().unwrap_or(&[][..]); } - let src = x1 - .iter() - .cloned() - .chain(x2.iter().cloned()) - .chain(x3.iter().cloned()) - .chain(x4.iter().cloned()) - .chain(x5.iter().cloned()); + let src = + x1.iter().cloned().chain(x2.iter().cloned().chain(x3.iter().cloned())).chain( + Some(()).into_iter().flat_map(|()| x4.iter().cloned().chain(x5.iter().cloned())), + ); do_bench_extend_iter(b, dst_len, src_len, src) } From ddabd7643efb5cc7dc82d3024e6294bc98b9caf2 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Thu, 9 Jan 2020 09:04:23 +0100 Subject: [PATCH 4/8] perf: Use `for_each` in `Vec::extend` `for_each` are specialized for iterators such as `chain` allowing for faster iteration than a normal `for/while` loop. Note that since this only checks `size_hint` once at the start it may end up needing to call `reserve` more in the case that `size_hint` returns a larger and more accurate lower bound during iteration. This could maybe be alleviated with an implementation closure like the current one but the extra complexity will likely end up harming the normal case of an accurate or 0 (think `filter`) lower bound. ```rust while let Some(element) = iterator.next() { let (lower, _) = iterator.size_hint(); self.reserve(lower.saturating_add(1)); unsafe { let len = self.len(); ptr::write(self.get_unchecked_mut(len), element); // NB can't overflow since we would have had to alloc the address space self.set_len(len + 1); } iterator.by_ref().take(self.capacity()).for_each(|element| { unsafe { let len = self.len(); ptr::write(self.get_unchecked_mut(len), element); // NB can't overflow since we would have had to alloc the address space self.set_len(len + 1); } }); } // OR let (lower, _) = iterator.size_hint(); self.reserve(lower); loop { let result = iterator.by_ref().try_for_each(|element| { if self.len() == self.capacity() { return Err(element); } unsafe { let len = self.len(); ptr::write(self.get_unchecked_mut(len), element); // NB can't overflow since we would have had to alloc the address space self.set_len(len + 1); } Ok(()) }); match result { Ok(()) => break, Err(element) => { let (lower, _) = iterator.size_hint(); self.reserve(lower.saturating_add(1)); self.push(element); } } } ``` Closes #63340 --- src/liballoc/vec.rs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 4f6b7870e2e8c..362957ec05bb5 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -2122,7 +2122,7 @@ where } impl Vec { - fn extend_desugared>(&mut self, mut iterator: I) { + fn extend_desugared>(&mut self, iterator: I) { // This is the case for a general iterator. // // This function should be the moral equivalent of: @@ -2130,18 +2130,10 @@ impl Vec { // for item in iterator { // self.push(item); // } - while let Some(element) = iterator.next() { - let len = self.len(); - if len == self.capacity() { - let (lower, _) = iterator.size_hint(); - self.reserve(lower.saturating_add(1)); - } - unsafe { - ptr::write(self.get_unchecked_mut(len), element); - // NB can't overflow since we would have had to alloc the address space - self.set_len(len + 1); - } - } + let (lower, _) = iterator.size_hint(); + self.reserve(lower); + + iterator.for_each(|element| self.push(element)); } /// Creates a splicing iterator that replaces the specified range in the vector From 3cfa3aadbcf1ad81d7ceedc6c31daa7d99364318 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Tue, 21 Jan 2020 22:30:54 +0100 Subject: [PATCH 5/8] Use try_fold in Vec::extend Should put less stress on LLVM since there are less closures passed around and lets us refine how much we reserve with `size_hint` if the first guess is too low. --- src/liballoc/vec.rs | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 362957ec05bb5..9d922c95813e0 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -1559,8 +1559,8 @@ impl Vec { #[unstable(feature = "vec_resize_default", issue = "41758")] #[rustc_deprecated( reason = "This is moving towards being removed in favor \ - of `.resize_with(Default::default)`. If you disagree, please comment \ - in the tracking issue.", + of `.resize_with(Default::default)`. If you disagree, please comment \ + in the tracking issue.", since = "1.33.0" )] pub fn resize_default(&mut self, new_len: usize) { @@ -2122,7 +2122,7 @@ where } impl Vec { - fn extend_desugared>(&mut self, iterator: I) { + fn extend_desugared>(&mut self, mut iterator: I) { // This is the case for a general iterator. // // This function should be the moral equivalent of: @@ -2132,8 +2132,34 @@ impl Vec { // } let (lower, _) = iterator.size_hint(); self.reserve(lower); - - iterator.for_each(|element| self.push(element)); + loop { + let cap = self.capacity(); + let result = iterator.by_ref().try_fold((), |(), element| { + let len = self.len(); + if len == cap { + Err(element) + } else { + unsafe { + ptr::write(self.get_unchecked_mut(len), element); + self.set_len(len + 1); + Ok(()) + } + } + }); + match result { + Ok(()) => return, + Err(element) => { + let (lower, _) = iterator.size_hint(); + self.reserve(lower.saturating_add(1)); + unsafe { + let len = self.len(); + ptr::write(self.get_unchecked_mut(len), element); + // NB can't overflow since we would have had to alloc the address space + self.set_len(len + 1); + } + } + } + } } /// Creates a splicing iterator that replaces the specified range in the vector From 824151e25666137f94c54f25a4f584723ea5a03d Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Wed, 22 Jan 2020 23:19:56 +0100 Subject: [PATCH 6/8] No by_ref --- src/liballoc/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 9d922c95813e0..5777ebdc470dd 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -2134,7 +2134,7 @@ impl Vec { self.reserve(lower); loop { let cap = self.capacity(); - let result = iterator.by_ref().try_fold((), |(), element| { + let result = iterator.try_fold((), |(), element| { let len = self.len(); if len == cap { Err(element) From a092fe1719453c1bb85785a4d7f982ec5af61ef1 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Wed, 22 Jan 2020 23:22:41 +0100 Subject: [PATCH 7/8] Avoid a closure in Vec::extend_desugared --- src/liballoc/vec.rs | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 5777ebdc470dd..69e9b6c2865b8 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -2121,6 +2121,21 @@ where } } +fn extend_fold(self_: &mut Vec) -> impl FnMut((), T) -> Result<(), T> + '_ { + move |(), element| { + let len = self_.len(); + if len < self_.capacity() { + unsafe { + ptr::write(self_.get_unchecked_mut(len), element); + self_.set_len(len + 1); + Ok(()) + } + } else { + Err(element) + } + } +} + impl Vec { fn extend_desugared>(&mut self, mut iterator: I) { // This is the case for a general iterator. @@ -2133,21 +2148,8 @@ impl Vec { let (lower, _) = iterator.size_hint(); self.reserve(lower); loop { - let cap = self.capacity(); - let result = iterator.try_fold((), |(), element| { - let len = self.len(); - if len == cap { - Err(element) - } else { - unsafe { - ptr::write(self.get_unchecked_mut(len), element); - self.set_len(len + 1); - Ok(()) - } - } - }); - match result { - Ok(()) => return, + match iterator.try_fold((), extend_fold(self)) { + Ok(_) => return, Err(element) => { let (lower, _) = iterator.size_hint(); self.reserve(lower.saturating_add(1)); From e41f55eeb84beb29ea19bb5eb07635c9d17cdb71 Mon Sep 17 00:00:00 2001 From: Markus Westerlind Date: Tue, 28 Jan 2020 10:48:11 +0100 Subject: [PATCH 8/8] Massage extend_desugared to reduce the compiletime impact --- src/liballoc/vec.rs | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 69e9b6c2865b8..7e409bf96930d 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -2123,13 +2123,11 @@ where fn extend_fold(self_: &mut Vec) -> impl FnMut((), T) -> Result<(), T> + '_ { move |(), element| { - let len = self_.len(); - if len < self_.capacity() { + if self_.len() < self_.capacity() { unsafe { - ptr::write(self_.get_unchecked_mut(len), element); - self_.set_len(len + 1); - Ok(()) + self_.push_unchecked(element); } + Ok(()) } else { Err(element) } @@ -2137,6 +2135,23 @@ fn extend_fold(self_: &mut Vec) -> impl FnMut((), T) -> Result<(), T> + '_ } impl Vec { + unsafe fn push_unchecked(&mut self, value: T) { + let end = self.as_mut_ptr().add(self.len); + ptr::write(end, value); + // NB can't overflow since we would have had to alloc the address space + self.len += 1; + } + + #[cold] + #[inline(never)] + fn extend_desugared_fallback>(&mut self, iterator: &I, element: T) { + let (lower, _) = iterator.size_hint(); + self.reserve(lower.saturating_add(1)); + unsafe { + self.push_unchecked(element); + } + } + fn extend_desugared>(&mut self, mut iterator: I) { // This is the case for a general iterator. // @@ -2147,20 +2162,8 @@ impl Vec { // } let (lower, _) = iterator.size_hint(); self.reserve(lower); - loop { - match iterator.try_fold((), extend_fold(self)) { - Ok(_) => return, - Err(element) => { - let (lower, _) = iterator.size_hint(); - self.reserve(lower.saturating_add(1)); - unsafe { - let len = self.len(); - ptr::write(self.get_unchecked_mut(len), element); - // NB can't overflow since we would have had to alloc the address space - self.set_len(len + 1); - } - } - } + while let Err(element) = iterator.try_fold((), extend_fold(self)) { + self.extend_desugared_fallback(&iterator, element); } }