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

Avoid excessive reallocations during item-bodies checking #31929

Merged
merged 1 commit into from
Feb 27, 2016

Conversation

dotdash
Copy link
Contributor

@dotdash dotdash commented Feb 27, 2016

When foldings Substs, we map over VecPerParamSpace instances using
EnumeratedItems which does not provide an accurate size_hint()
in its Iterator implementation. This leads to quite a large number or
reallocations. Providing a suitable size_hint() implementation reduces
the time spent in item-bodies checking quite a bit.

crate  | before | after | ~change
-------|-------------------------
core   |  7.28s | 5.44s |   -25%
std    |  2.07s | 1.88s |  -9.2%
syntax |  8.86s | 8.30s |  -6.3%

When foldings Substs, we map over VecPerParamSpace instances using
EnumeratedItems which does not provide an accurate size_hint()
in its Iterator implementation. This leads to quite a large number or
reallocations. Providing a suitable size_hint() implementation reduces
the time spent in item-bodies checking quite a bit.

```
crate  | before | after | ~change
-------|-------------------------
core   |  7.28s | 5.44s |   -25%
std    |  2.07s | 1.88s |  -9.2%
syntax |  8.86s | 8.30s |  -6.3%
```
@rust-highfive
Copy link
Collaborator

r? @jroesch

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

@alexcrichton
Copy link
Member

@bors: r+ 31fef23

Holy cow! Nice wins!


fn size_hint(&self) -> (usize, Option<usize>) {
let size = self.vec.as_slice().len();
(size, Some(size))
Copy link
Member

Choose a reason for hiding this comment

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

This overestimates the number of elements in the iterator, doesn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it doesn't. The underlying vector is used in its entirety. If you check VecPerParamSpace::limits() one can see how it is split into three consecutive ranges.

(We discussed this on IRC, I'm just adding this comment for reference.)

Copy link
Member

Choose a reason for hiding this comment

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

I now understand the iterator! It's fine. We cleared this up on irc.

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 27, 2016
When foldings Substs, we map over VecPerParamSpace instances using
EnumeratedItems which does not provide an accurate size_hint()
in its Iterator implementation. This leads to quite a large number or
reallocations. Providing a suitable size_hint() implementation reduces
the time spent in item-bodies checking quite a bit.

```
crate  | before | after | ~change
-------|-------------------------
core   |  7.28s | 5.44s |   -25%
std    |  2.07s | 1.88s |  -9.2%
syntax |  8.86s | 8.30s |  -6.3%
```
bors added a commit that referenced this pull request Feb 27, 2016
@bors bors merged commit 31fef23 into rust-lang:master Feb 27, 2016
@dotdash dotdash deleted the less_rallocx branch May 17, 2016 03: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.

6 participants