Skip to content

/rust

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

consolidate type-variable handling in assemble_candidates #27984

Merged
merged 1 commit into from Sep 4, 2015

Conversation

Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@arielb1 @nikomatsakis @bors
@arielb1
Copy link
Contributor

arielb1 commented Aug 24, 2015

this resolves type-variables early in assemble_candidates and
bails out quickly if the self type is an inference variable (which would
fail anyway because of assemble_candidates_from_projected_tys).

In both these cases, assemble_candidates_from_impls would try to go
over all impls and match them, leading to O(n*m) performance. Fixing this
improves rustc type-checking performance by 10%. As type-checking is only
is 5% of compilation, this doesn't impact bootstrap times, but does
improve type-error-detection time which is nice.

Crates that have many dependencies and contain significant amounts of
generic functions could see a bigger perf boost. As a microbenchmark,
the crate generated by

echo '#![feature(rustc_private)]'
echo 'extern crate rustc_driver;'
for i in {1..1000}; do cat << _EOF_
    pub fn foo$i<T>() {
        let mut v = Vec::new();
        let _w = v.clone();
        v.push("");
    }
_EOF_
done

sees performance improve from 7.2 to 1.4 seconds. I imagine many crates
would fall somewhere in-between.

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis Aug 24, 2015

@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented Sep 1, 2015

Nice patch. r+ modulo nits.
Ariel Ben-Yehuda @arielb1
consolidate type-variable handling in assemble_candidates 
this resolves type-variables early in assemble_candidates and
bails out quickly if the self type is an inference variable (which would
fail anyway because of `assemble_candidates_from_projected_tys`).

In both these cases, `assemble_candidates_from_impls` would try to go
over all impls and match them, leading to O(n*m) performance. Fixing this
improves rustc type-checking performance by 10%. As type-checking is only
is 5% of compilation, this doesn't impact bootstrap times, but *does*
improve type-error-detection time which is nice.

Crates that have many dependencies and contain significant amounts of
generic functions could see a bigger perf boost. As a microbenchmark,
the crate generated by

echo '#![feature(rustc_private)]'
echo 'extern crate rustc_driver;'
for i in {1..1000}; do cat << _EOF_
    pub fn foo$i<T>() {
        let mut v = Vec::new();
        let _w = v.clone();
        v.push("");
    }
_EOF_
done

sees performance improve from 7.2 to 1.4 seconds. I imagine many crates
would fall somewhere in-between.
Loading status checks…
ab86bf5

@arielb1 arielb1 force-pushed the arielb1:misc-assemble-improvements branch from 35cfb91 to ab86bf5 Sep 2, 2015

@arielb1

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

arielb1 commented Sep 2, 2015

updated
@nikomatsakis

This comment has been minimized.

Sign in to view
Copy link
Contributor

nikomatsakis commented Sep 3, 2015

@bors r+
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 3, 2015

📌 Commit ab86bf5 has been approved by nikomatsakis

bors added a commit that referenced this pull request Sep 3, 2015

@bors
Auto merge of #27984 - arielb1:misc-assemble-improvements, r=nikomats… 
…akis

this resolves type-variables early in assemble_candidates and
bails out quickly if the self type is an inference variable (which would
fail anyway because of `assemble_candidates_from_projected_tys`).

In both these cases, `assemble_candidates_from_impls` would try to go
over all impls and match them, leading to O(`n*m`) performance. Fixing this
improves rustc type-checking performance by 10%. As type-checking is only
is 5% of compilation, this doesn't impact bootstrap times, but *does*
improve type-error-detection time which is nice.

Crates that have many dependencies and contain significant amounts of
generic functions could see a bigger perf boost. As a microbenchmark,
the crate generated by

```
echo '#![feature(rustc_private)]'
echo 'extern crate rustc_driver;'
for i in {1..1000}; do cat << _EOF_
    pub fn foo$i<T>() {
        let mut v = Vec::new();
        let _w = v.clone();
        v.push("");
    }
_EOF_
done
```

sees performance improve from 7.2 to 1.4 seconds. I imagine many crates
would fall somewhere in-between.

r? @nikomatsakis
1d2c20b
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 3, 2015

⌛️ Testing commit ab86bf5 with merge 1d2c20b...
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 3, 2015

⛄️ The build was interrupted to prioritize another pull request.
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 3, 2015

⌛️ Testing commit ab86bf5 with merge b399f8d...

bors added a commit that referenced this pull request Sep 3, 2015

@bors
Auto merge of #27984 - arielb1:misc-assemble-improvements, r=nikomats… 
…akis

this resolves type-variables early in assemble_candidates and
bails out quickly if the self type is an inference variable (which would
fail anyway because of `assemble_candidates_from_projected_tys`).

In both these cases, `assemble_candidates_from_impls` would try to go
over all impls and match them, leading to O(`n*m`) performance. Fixing this
improves rustc type-checking performance by 10%. As type-checking is only
is 5% of compilation, this doesn't impact bootstrap times, but *does*
improve type-error-detection time which is nice.

Crates that have many dependencies and contain significant amounts of
generic functions could see a bigger perf boost. As a microbenchmark,
the crate generated by

```
echo '#![feature(rustc_private)]'
echo 'extern crate rustc_driver;'
for i in {1..1000}; do cat << _EOF_
    pub fn foo$i<T>() {
        let mut v = Vec::new();
        let _w = v.clone();
        v.push("");
    }
_EOF_
done
```

sees performance improve from 7.2 to 1.4 seconds. I imagine many crates
would fall somewhere in-between.

r? @nikomatsakis
b399f8d
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 3, 2015

⛄️ The build was interrupted to prioritize another pull request.

bors added a commit that referenced this pull request Sep 4, 2015

@bors
Auto merge of #27984 - arielb1:misc-assemble-improvements, r=nikomats… 
…akis

this resolves type-variables early in assemble_candidates and
bails out quickly if the self type is an inference variable (which would
fail anyway because of `assemble_candidates_from_projected_tys`).

In both these cases, `assemble_candidates_from_impls` would try to go
over all impls and match them, leading to O(`n*m`) performance. Fixing this
improves rustc type-checking performance by 10%. As type-checking is only
is 5% of compilation, this doesn't impact bootstrap times, but *does*
improve type-error-detection time which is nice.

Crates that have many dependencies and contain significant amounts of
generic functions could see a bigger perf boost. As a microbenchmark,
the crate generated by

```
echo '#![feature(rustc_private)]'
echo 'extern crate rustc_driver;'
for i in {1..1000}; do cat << _EOF_
    pub fn foo$i<T>() {
        let mut v = Vec::new();
        let _w = v.clone();
        v.push("");
    }
_EOF_
done
```

sees performance improve from 7.2 to 1.4 seconds. I imagine many crates
would fall somewhere in-between.

r? @nikomatsakis
Loading status checks…
2727a8e
@bors

This comment has been minimized.

Sign in to view
Copy link
Contributor

bors commented Sep 4, 2015

⌛️ Testing commit ab86bf5 with merge 2727a8e...
@bors

@bors bors merged commit ab86bf5 into rust-lang:master Sep 4, 2015
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.