Skip to content

Conversation

@michaelwoerister
Copy link
Member

@michaelwoerister michaelwoerister commented Jun 8, 2017

This PR moves DepNode to a representation that does not need retracing and thus simplifies comparing dep-graphs from different compilation sessions. The code also gets a lot simpler in many places, since we don't need the generic parameter on DepNode anymore. See #42294 for details.

NOTE: Only the last commit of this is new, the rest is already reviewed in #42504.

This PR is almost done but there are some things I still want to do:

  • Add some module-level documentation to dep_node.rs, explaining especially what the define_dep_nodes!() macro is about.
  • Do another pass over the dep-graph loading logic. I suspect that we can get rid of building the edges map and also use arrays instead of hash maps in some places.

cc @rust-lang/compiler
r? @nikomatsakis

@michaelwoerister michaelwoerister added the A-incr-comp Area: Incremental compilation label Jun 8, 2017
@Mark-Simulacrum
Copy link
Member

Looks like tidy failed here:

[00:03:21] tidy error: /checkout/src/librustc_incremental/persist/preds/mod.rs:69: line longer than 100 chars

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 8, 2017
@michaelwoerister michaelwoerister added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 9, 2017
@michaelwoerister
Copy link
Member Author

OK, this should be ready for review.

@michaelwoerister michaelwoerister changed the title [WIP] Make DepNode Copy and valid across compilation sessions incr.comp.: Make DepNode Copy and valid across compilation sessions Jun 9, 2017
Copy link
Contributor

Choose a reason for hiding this comment

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

🥂 love the comments

// erase!() just makes tokens go away. It's used to specify which macro argument
// is repeated (i.e. which sub-expression of the macro we are in) but don't need
// to actually use any of the arguments.
macro_rules! erase {
Copy link
Contributor

Choose a reason for hiding this comment

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

cute.

@nikomatsakis
Copy link
Contributor

This is really nice. r=me once errors are resolved.

@michaelwoerister
Copy link
Member Author

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Jun 12, 2017

📌 Commit fdff2d3 has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Jun 12, 2017

⌛ Testing commit fdff2d3 with merge 3f8b936...

bors added a commit that referenced this pull request Jun 12, 2017
incr.comp.: Make DepNode `Copy` and valid across compilation sessions

This PR moves `DepNode` to a representation that does not need retracing and thus simplifies comparing dep-graphs from different compilation sessions. The code also gets a lot simpler in many places, since we don't need the generic parameter on `DepNode` anymore.  See #42294 for details.

~~NOTE: Only the last commit of this is new, the rest is already reviewed in #42504

This PR is almost done but there are some things I still want to do:
- [x] Add some module-level documentation to `dep_node.rs`, explaining especially what the `define_dep_nodes!()` macro is about.
- [x] Do another pass over the dep-graph loading logic. I suspect that we can get rid of building the `edges` map and also use arrays instead of hash maps in some places.

cc @rust-lang/compiler
r? @nikomatsakis
@bors
Copy link
Collaborator

bors commented Jun 12, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 3f8b936 to master...

@bors bors merged commit fdff2d3 into rust-lang:master Jun 12, 2017
bors added a commit that referenced this pull request Jun 14, 2017
incr.comp.: Make DepNode's std::fmt::Debug implementation useful again.

With #42537 a regular `DepNode` only contains an opaque hash as its identifier. In most cases, this hash is actually a `DefPathHash` and we can reconstruct the `DefId` it came from via a table lookup --- and then use that to print something intelligible for debug outputs. For cases where we cannot reconstruct information from the DepNode's hash, this PR will cache a string representation of the `DepNode` in a side-table. This string is later used for debug outputs.

r? @nikomatsakis
bors added a commit that referenced this pull request Jun 15, 2017
incr.comp.: Make DepNode's std::fmt::Debug implementation useful again.

With #42537 a regular `DepNode` only contains an opaque hash as its identifier. In most cases, this hash is actually a `DefPathHash` and we can reconstruct the `DefId` it came from via a table lookup --- and then use that to print something intelligible for debug outputs. For cases where we cannot reconstruct information from the DepNode's hash, this PR will cache a string representation of the `DepNode` in a side-table. This string is later used for debug outputs.

r? @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-incr-comp Area: Incremental compilation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants