Skip to content

Use vec![] for vector creation #26904

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

Merged
merged 2 commits into from
Jul 9, 2015
Merged

Use vec![] for vector creation #26904

merged 2 commits into from
Jul 9, 2015

Conversation

bluss
Copy link
Member

@bluss bluss commented Jul 8, 2015

In a followup to PR #26849, improve one more location for I/O where
we can use Vec::resize to ensure better performance when zeroing
buffers.

Use the vec![elt; n] macro everywhere we can in the tree. It replaces
repeat(elt).take(n).collect() which is more verbose, requires type
hints, and right now produces worse code. vec![] is preferable for vector
initialization.

The vec![] replacement touches upon one I/O path too, Stdin::read
for windows, and that should be a small improvement.

r? @alexcrichton

// use `resize` so that the zero filling is as efficient as possible
let len = self.inner.len();
self.inner.resize(len + amt as usize, 0);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come this is surrounded with an if?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah great question, that's an artifact of the version I wrote first

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that though this all looks good to me! r=me with this changed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, fixed & pushed.

Vec::resize compiles to better code than .extend(repeat(0).take(n)) does
right now.
@bluss
Copy link
Member Author

bluss commented Jul 8, 2015

Thanks for looking at it. You're quick!

@bors: r=alexcrichton

@bors
Copy link
Collaborator

bors commented Jul 8, 2015

📌 Commit 475e0be has been approved by alexcrichton

@bors
Copy link
Collaborator

bors commented Jul 9, 2015

⌛ Testing commit 475e0be with merge ef2d987...

@bors
Copy link
Collaborator

bors commented Jul 9, 2015

💔 Test failed - auto-win-gnu-64-opt

The common pattern `iter::repeat(elt).take(n).collect::<Vec<_>>()` is
exactly equivalent to `vec![elt; n]`, do this replacement in the whole
tree.

(Actually, vec![] is smart enough to only call clone n - 1 times, while
the former solution would call clone n times, and this fact is
virtually irrelevant in practice.)
@bluss
Copy link
Member Author

bluss commented Jul 9, 2015

Forgot an unused import, updated.

@bluss
Copy link
Member Author

bluss commented Jul 9, 2015

@bors: r=alexcrichton

@bors
Copy link
Collaborator

bors commented Jul 9, 2015

📌 Commit 836f32e has been approved by alexcrichton

bors added a commit that referenced this pull request Jul 9, 2015
In a followup to PR #26849, improve one more location for I/O where
we can use `Vec::resize` to ensure better performance when zeroing
buffers.

Use the `vec![elt; n]` macro everywhere we can in the tree. It replaces
`repeat(elt).take(n).collect()` which is more verbose, requires type
hints, and right now produces worse code. `vec![]` is preferable for vector
initialization.

The `vec![]` replacement touches upon one I/O path too, Stdin::read
for windows, and that should be a small improvement.

r? @alexcrichton
@bors
Copy link
Collaborator

bors commented Jul 9, 2015

⌛ Testing commit 836f32e with merge f11502c...

@bors bors merged commit 836f32e into rust-lang:master Jul 9, 2015
@bluss bluss deleted the no-repeat branch July 9, 2015 12:43
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.

3 participants