Skip to content

Micro optimization idea: resue string buffer for FromIterator #51541

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

Closed
Lucretiel opened this issue Jun 13, 2018 · 3 comments
Closed

Micro optimization idea: resue string buffer for FromIterator #51541

Lucretiel opened this issue Jun 13, 2018 · 3 comments

Comments

@Lucretiel
Copy link
Contributor

Lucretiel commented Jun 13, 2018

rust/src/liballoc/string.rs

Lines 1720 to 1727 in 07c415c

#[stable(feature = "extend_string", since = "1.4.0")]
impl FromIterator<String> for String {
fn from_iter<I: IntoIterator<Item = String>>(iter: I) -> String {
let mut buf = String::new();
buf.extend(iter);
buf
}
}

Basically the idea is that, for impl FromIterator<String> for String, we reimplement it slightly so that newly returned string reuses and reallocates the buffer of the first string in the iterator. It would look something like this:

fn from_iter(iter: I) -> String {
    match iter.next() {
        None => String::new(),
        Some(base) => { base.extend(iter); base }
    }
}

Of course, this might not be worth the added complexity– in particular, I'm not sure what the implications are for added branching– so I'm writing this issue to solicit any feedback before creating a PR.

@ishitatsuyuki
Copy link
Contributor

I think that it will likely be a reallocation + move which is basically as expensive as allocating a brand new string. Thus this doesn't sound high priority to me; but maybe worth trying :)

@scottmcm
Copy link
Member

Hmm, I agree that in general it's unlikely that the first string happens to have enough capacity for the whole iterator, so this might be of minimal value in general.

It would, however, be a clear win for a case where there happens to only be one element in the iterator, though, so might be worth doing for that alone.

@lnicola
Copy link
Member

lnicola commented Dec 16, 2018

@Lucretiel Can this be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants