Skip to content

Specialization for Extend<&T> for vec#38182

Merged
bors merged 1 commit into
rust-lang:masterfrom
bluss:more-vec-extend
Dec 8, 2016
Merged

Specialization for Extend<&T> for vec#38182
bors merged 1 commit into
rust-lang:masterfrom
bluss:more-vec-extend

Conversation

@bluss

@bluss bluss commented Dec 5, 2016

Copy link
Copy Markdown
Contributor

Specialize to use copy_from_slice when extending a Vec with &[T] where
T: Copy.

This specialization results in .clone() not being called in extend_from_slice and extend when the element is Copy.

Fixes #38021

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@bluss

bluss commented Dec 5, 2016

Copy link
Copy Markdown
Contributor Author

r? @alexcrichton This fix goes forward instead of backwards, you could say, by adding more specializations instead of reverting.

Specialize to use copy_from_slice when extending a Vec with &[T] where
T: Copy.
@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+

Seems ok to me, thanks!

I'd still ideally prefer to explore the route of adding codegen tests eventually (to ensure we don't regress) but that can happen in a follow-up

@bors

bors commented Dec 6, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 02bf1ce has been approved by alexcrichton

@bluss

bluss commented Dec 6, 2016

Copy link
Copy Markdown
Contributor Author

Ok, there is no codegen test, but codegen for extend_with_slice was examined and was fine before it went in, so in that way it's unlikely that it would have been caught that way. It already optimized fine in many situations.

@bluss

bluss commented Dec 7, 2016

Copy link
Copy Markdown
Contributor Author

@dtolnay This PR fixes the regression in the sense that the regression I saw here #38021 (comment) was verified to be back at its old performance at 1.5-1.6 ms/iter. I haven't done further tests.

@bors

bors commented Dec 8, 2016

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 02bf1ce with merge 7537f95...

bors added a commit that referenced this pull request Dec 8, 2016
Specialization for Extend<&T> for vec

Specialize to use copy_from_slice when extending a Vec with &[T] where
T: Copy.

This specialization results in `.clone()` not being called in `extend_from_slice` and `extend` when the element is `Copy`.

Fixes #38021
@bors bors merged commit 02bf1ce into rust-lang:master Dec 8, 2016
@bluss bluss deleted the more-vec-extend branch December 8, 2016 20:18
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

Successfully merging this pull request may close these issues.

4 participants