Remove vector methods that enforce uneccessary allocations or copies #12456
Labels
I-slow
Issue: Problems and improvements with respect to performance of generated code.
Comments
bors
added a commit
that referenced
this issue
Mar 26, 2014
# Summary Changed `iter::Extendable` and `iter::FromIterator` to take a `Iterator` by value. These functions always exhaust the passed `Iterator`, and are often used for transferring the values of a new `Iterator` directly into a data structure, so using them usually require the use of the `&mut` operator: ``` foo.extend(&mut bar.move_iter()); // Transfer content from bar into foo let mut iter = ...; foo.extend(&mut iter); // iter is now empty ``` This patch changes both the `FromIterator` and `Extendable` traits to take the iterator by value instead, which makes the common case of using these traits less heavy: ``` foo.extend(bar.move_iter()); // Transfer content from bar into foo let iter = ...; foo.extend(iter); // iter is now inaccessible if it moved // or unchanged if it was Pod and copied. ``` # Composability This technically makes the traits less flexible from a type system pov, because they now require ownership. However, because `Iterator` provides the `ByRef` adapter, there is no loss of functionality: ``` foo.extend(iter.by_ref()); // Same semantic as today, for the few situations where you need it. ``` # Motivation This change makes it less painful to use iterators for shuffling values around between collections, which makes it more acceptable to always use them for this, enabling more flexibility. For example, `foo.extend(bar.move_iter())` can generally be the fastest way to append an collections content to another one, without both needing to have the same type. Making this easy to use would allow the removal of special cased methods like `push_all()` on vectors. (See #12456) I opened #13038 as well, to discuss this change in general if people object to it. # Further work This didn't change the `collect()` method to take by value `self`, nor any of the other adapters that also exhaust their iterator argument. For consistency this should probably happen in the long term, but for now this is too much trouble, as every use of them would need to be checked for accidentally changed semantic by going `&mut self -> self`. (which allows for the possibility that a `Pod` iterator got copied instead of exhausted without generating a type error by the change)
thestinger
added
the
I-slow
|
Should this be closed? The only one of these methods that still exists is |
|
Looks to be the case, thanks @gereeter! |
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Jul 25, 2022
internal: Shorten main thread names Linux effectively has a 15 byte limit, which resulted in `rust-analyzer s` and `rust-analyzer p`. That's still unambiguous, but probably not obvious.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
~[T]has a few legacy methods that enforce unnecessary copies or allocations:map<T, U>(&[T], f: |&T| -> U) -> ~[U]: requires the user to copy/clone through the &T references, and always allocates a new~[U].flat_map<T, U>(&[T], f: |&T| -> ~[U]) -> ~[U]: same asmap, but also requires allocation of temporary~[U]vectors.push_all_move(&mut ~[T], ~[T]): Requires a temporary~[T]vector just to move all values in at once.Today, all these functions can be more generally and precisely expressed with iterators:
vec.map(...) => vec.iter().map(...).collect(), with the options of callingmove_iter()instead, and the ability to collect into a arbitrary other type.vec.flat_map(...) => vec.iter().flat_map(...).collect(), same as above, but depending on iterator used internally no need for allocations and temporary vectors.vec.push_all_move(v) => vec.extend(&mut v.move_iter()), instead of passing a owned vector, passing a iterator instead that produces the values.So, I'm proposing that these functions get removed and replaced with the alternatives above. The only problem is that its getting more verbose, but there are a few ways how that can get reduced:
Iterabletrait, shorten of names likemove_iter, etc.(Everything said here also applies to
Vec<T>due the currently in-progress transition to it.)The text was updated successfully, but these errors were encountered: