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

Preallocate the vector containing predicates in decode_predicates #55534

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 19 additions & 15 deletions src/librustc/ty/codec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,22 +175,26 @@ pub fn decode_predicates<'a, 'tcx, D>(decoder: &mut D)
where D: TyDecoder<'a, 'tcx>,
'tcx: 'a,
{
let parent = Decodable::decode(decoder)?;
let decoder_len = decoder.read_usize()?;
let mut predicates = Vec::with_capacity(decoder_len);
for _ in 0..decoder_len {
// Handle shorthands first, if we have an usize > 0x80.
let predicate = if decoder.positioned_at_shorthand() {
let pos = decoder.read_usize()?;
assert!(pos >= SHORTHAND_OFFSET);
let shorthand = pos - SHORTHAND_OFFSET;

decoder.with_position(shorthand, ty::Predicate::decode)
} else {
ty::Predicate::decode(decoder)
}?;
predicates.push((predicate, Decodable::decode(decoder)?))
}

Ok(ty::GenericPredicates {
parent: Decodable::decode(decoder)?,
predicates: (0..decoder.read_usize()?).map(|_| {
// Handle shorthands first, if we have an usize > 0x80.
let predicate = if decoder.positioned_at_shorthand() {
let pos = decoder.read_usize()?;
assert!(pos >= SHORTHAND_OFFSET);
let shorthand = pos - SHORTHAND_OFFSET;

decoder.with_position(shorthand, ty::Predicate::decode)
} else {
ty::Predicate::decode(decoder)
}?;
Ok((predicate, Decodable::decode(decoder)?))
})
.collect::<Result<Vec<_>, _>>()?,
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless I'm mistaken, the iterator will already pre-allocate a vector of the right size here, so this doesn't really buy us anything.

Copy link
Contributor Author

@ljedrz ljedrz Nov 14, 2018

Choose a reason for hiding this comment

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

The lower bound of size_hint for the implementation of FromIterator for Result is equal to zero and the implementation of from_iter for Vec uses it to calculate the capacity. I don't think this is a case of TrustedLen (due to non-conforming size_hint), otherwise this PR (where the number of elements iterated over is known) wouldn't benefit from such a change either.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see, ok.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also see I missed a lot of discussion on this topic already 😊

parent,
predicates
})
}

Expand Down