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.

Sign up for GitHub

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

Jump to bottom

println!() breaks incr. comp. #35593

Closed
michaelwoerister opened this issue Aug 11, 2016 · 2 comments · Fixed by #35854
Closed

println!() breaks incr. comp. #35593

michaelwoerister opened this issue Aug 11, 2016 · 2 comments · Fixed by #35854
Labels
A-incr-comp Area: Incremental compilation
Milestone
Incremental compi...

Comments

@michaelwoerister
Copy link
Member

The following test, when added to the incremental test suite, fails:

// revisions:rpass1 rpass2

#![feature(rustc_attrs)]
#![rustc_partition_reused(module="println", cfg="rpass2")]

fn main() {
    println!("hello world");
}

Something causes the hash for main to be different in the two revisions.
@michaelwoerister michaelwoerister added the A-incr-comp Area: Incremental compilation label Aug 11, 2016
@michaelwoerister michaelwoerister added this to the Incremental compilation alpha milestone Aug 11, 2016
@michaelwoerister
Copy link
Member Author

cc @nikomatsakis

@nikomatsakis nikomatsakis mentioned this issue Aug 18, 2016
Closed
@nikomatsakis
Copy link
Contributor

This is caused by the fact that typeck rewrites the result of name resolution in place, and hence the name resolution hashes after typeck are different than the ones from before. =(

The easiest fix is probably #35549

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Aug 20, 2016
@nikomatsakis
compute and cache HIR hashes at beginning
6bd80d1 
This avoids the compile-time overhead of computing them twice.  It also fixes
an issue where the hash computed after typeck is differen than the hash before,
because typeck mutates the def-map in place.

Fixes rust-lang#35549.
Fixes rust-lang#35593.
@nikomatsakis nikomatsakis mentioned this issue Aug 20, 2016
Merged
nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Aug 23, 2016
@nikomatsakis
add regression test for rust-lang#35593
226e1e5 
Fixes rust-lang#35593
bors added a commit that referenced this issue Aug 23, 2016
@bors
Auto merge of #35854 - nikomatsakis:incr-comp-cache-hash-35549, r=mw
012f45e 
compute and cache HIR hashes at beginning

This avoids the compile-time overhead of computing them twice.  It also fixes
an issue where the hash computed after typeck is differen than the hash before,
because typeck mutates the def-map in place.

Fixes #35549.
Fixes #35593.

Some performance measurements suggest this `HashesMap` is very small in memory (unobservable via `-Z time-passes`) and very cheap to construct. I do see some (very minor) performance wins in the incremental case after the first run -- the first run costs more because loading the dep-graph didn't have any hashing to do in that case. Example timings from two runs  of `libsyntex-syntax` -- the (1) indicates first run, (2) indicates second run, and (*) indicates both together:

| Phase | Master | Branch |
| ---- | ---- | ---- |
| compute_hashes_map (1) | N/A | 0.343 |
| load_dep_graph (1) | 0 | 0 |
| serialize dep graph (1) | 4.190 | 3.920 |
| total (1) | 4.190 | 4.260 |
| compute_hashes_map (2) | N/A | 0.344 |
| load_dep_graph (2) | 0.592 | 0.252 |
| serialize dep graph (2) | 4.119 | 3.779 |
| total (2) | 4.71 | 4.375 |
| total (*) | 8.9 | 8.635 |

r? @michaelwoerister
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-incr-comp Area: Incremental compilation
Projects
None yet
Milestone
Incremental compilation beta
Development

Successfully merging a pull request may close this issue.

compute and cache HIR hashes at beginning nikomatsakis/rust
2 participants
@nikomatsakis @michaelwoerister