Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vec::extend generates suboptimal code with .map #37232

Closed
arielb1 opened this issue Oct 17, 2016 · 2 comments
Closed

Vec::extend generates suboptimal code with .map #37232

arielb1 opened this issue Oct 17, 2016 · 2 comments
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@arielb1
Copy link
Contributor

arielb1 commented Oct 17, 2016

Meta

$ rustc -V
rustc 1.14.0-nightly (a3bc191b5 2016-10-10)

STR

#![crate_type="rlib"]
#![feature(test)]

extern crate test;

pub fn example_slow(l: &[(u32, u32)]) -> Vec<u32> {
    let mut result = Vec::with_capacity(l.len());
    result.extend(l.iter().map(|x| x.0));
    result
}

pub fn example_fast(l: &[(u32, u32)]) -> Vec<u32> {
    let mut result = Vec::with_capacity(l.len());
    for i in 0..l.len() {
        unsafe {
            *result.get_unchecked_mut(i) = l[i].0;
            result.set_len(i);
        }
    }
    result
}

#[bench]
fn bench_slow(b: &mut test::Bencher) {
    let data = [(0, 0); 16384];
    b.iter(|| {
        test::black_box(example_slow(test::black_box(&data)));
    });
}

#[bench]
fn bench_fast(b: &mut test::Bencher) {
    let data = [(0, 0); 16384];
    b.iter(|| {
        test::black_box(example_fast(test::black_box(&data)));
    });
}

Expected result

Both cases take the same amount of time

Actual result

$ ./slo-and-fast --bench

running 2 tests
test bench_fast ... bench:       7,490 ns/iter (+/- 271)
test bench_slow ... bench:      11,247 ns/iter (+/- 1,735)

test result: ok. 0 passed; 0 failed; 0 ignored; 2 measured
@arielb1 arielb1 added I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 17, 2016
@arielb1
Copy link
Contributor Author

arielb1 commented Oct 17, 2016

cc @bluss

@bluss
Copy link
Member

bluss commented Oct 17, 2016

It's not better with a regular (non-mapped) iterator. Only exception is the slice iterator, which is special cased. Reverse slice iterator is just as slow.

sophiajt pushed a commit to sophiajt/rust that referenced this issue Nov 4, 2016
Add Iterator trait TrustedLen to enable better FromIterator / Extend

This trait attempts to improve FromIterator / Extend code by enabling it to trust the iterator to produce an exact number of elements, which means that reallocation needs to happen only once and is moved out of the loop.

`TrustedLen` differs from `ExactSizeIterator` in that it attempts to include _more_ iterators by allowing for the case that the iterator's len does not fit in `usize`. Consumers must check for this case (for example they could panic, since they can't allocate a collection of that size).

For example, chain can be TrustedLen and all numerical ranges can be TrustedLen. All they need to do is to report an exact size if it fits in `usize`, and `None` as the upper bound otherwise.

The trait describes its contract like this:

```
An iterator that reports an accurate length using size_hint.

The iterator reports a size hint where it is either exact
(lower bound is equal to upper bound), or the upper bound is `None`.
The upper bound must only be `None` if the actual iterator length is
larger than `usize::MAX`.

The iterator must produce exactly the number of elements it reported.

This trait must only be implemented when the contract is upheld.
Consumers of this trait must inspect `.size_hint()`’s upper bound.
```

Fixes rust-lang#37232
bors added a commit that referenced this issue Nov 4, 2016
Add Iterator trait TrustedLen to enable better FromIterator / Extend

This trait attempts to improve FromIterator / Extend code by enabling it to trust the iterator to produce an exact number of elements, which means that reallocation needs to happen only once and is moved out of the loop.

`TrustedLen` differs from `ExactSizeIterator` in that it attempts to include _more_ iterators by allowing for the case that the iterator's len does not fit in `usize`. Consumers must check for this case (for example they could panic, since they can't allocate a collection of that size).

For example, chain can be TrustedLen and all numerical ranges can be TrustedLen. All they need to do is to report an exact size if it fits in `usize`, and `None` as the upper bound otherwise.

The trait describes its contract like this:

```
An iterator that reports an accurate length using size_hint.

The iterator reports a size hint where it is either exact
(lower bound is equal to upper bound), or the upper bound is `None`.
The upper bound must only be `None` if the actual iterator length is
larger than `usize::MAX`.

The iterator must produce exactly the number of elements it reported.

This trait must only be implemented when the contract is upheld.
Consumers of this trait must inspect `.size_hint()`’s upper bound.
```

Fixes #37232
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

2 participants