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

Lock step loop codegen issue #38175

Closed
bluss opened this issue Dec 5, 2016 · 4 comments
Closed

Lock step loop codegen issue #38175

bluss opened this issue Dec 5, 2016 · 4 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@bluss
Copy link
Member

bluss commented Dec 5, 2016

The original issue is the performance regression in #38021 when using the TrustedLen specialization of Extend<&T> for Vec.

In pseudocode you can describe their difference like this:

extend_from_slice:
for i in 0..len {
    dst[i] = src[i];
}

extend/TrustedLen:
while src != src_end {
    *dst++ = *src++;
}

A reduced case to demonstrate the issue is https://gist.github.com/bluss/61c32daae1bd31d9d7605983df3ace5f

@bluss
Copy link
Member Author

bluss commented Dec 5, 2016

This code looks like enough to reproduce in current nightly:

pub fn extend_1(v: &mut Vec<u8>, data: &[u8; 16]) {
    v.extend(data)
}

@bluss
Copy link
Member Author

bluss commented Dec 5, 2016

Reduced to not use so much library code (playground link)

Replacing .map(|&x| x) with a manual dereference of the element in place resolves the issue.

fn extend(v: &mut Vec<u8>, input: &[u8]) {
    v.reserve(input.len());
    unsafe {
        let mut dst = v.as_mut_ptr().offset(v.len() as isize);
        let mut iter = input.iter().map(|&x| x);
        while let Some(elt) = iter.next() {
            *dst = elt;
            dst = dst.offset(1);
        }
        let new_len = v.len() + input.len();
        v.set_len(new_len);
    }
}

#[no_mangle]
pub fn extend_1(v: &mut Vec<u8>, data: &[u8; 16]) {
    extend(v, data);
}

@bluss
Copy link
Member Author

bluss commented Dec 5, 2016

The reduced case above can be fixed like this (look at dst and i).

fn extend(v: &mut Vec<u8>, input: &[u8]) {
    v.reserve(input.len());
    unsafe {
        let dst = v.as_mut_ptr();
        let mut i = v.len() as isize;
        let mut iter = input.iter().map(|&x| x);
        while let Some(elt) = iter.next() {
            *dst.offset(i) = elt;
            i += 1;
        }
        let new_len = v.len() + input.len();
        v.set_len(new_len);
    }
}

but this fix does not seem to work to resolve the serialization regression in extend/TrustedLen.

@Mark-Simulacrum Mark-Simulacrum added the I-slow Issue: Problems and improvements with respect to performance of generated code. label May 16, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@saethlin
Copy link
Member

I extracted what I think is supposed to be the slow and fast version from the playground link above. It looks like this was fixed with rustc 1.20: https://godbolt.org/z/r8vcx64Kx Squinting at the LLVM IR in the gist linked above, it seems like the same pile of 1-byte moves that are emitted by rustc 1.19, but collapsed down by rust 1.20.

cc @alice-i-cecile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

No branches or pull requests

4 participants