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

incr.comp.: Do not hard-wire DepGraph into Session and CrateStore. #44390

Closed
7 tasks done
michaelwoerister opened this issue Sep 7, 2017 · 5 comments
Closed
7 tasks done
Labels
A-incr-comp Area: Incremental compilation

Comments

@michaelwoerister
Copy link
Member

michaelwoerister commented Sep 7, 2017

In the current setup, we create the DepGraph very early, so it can be put into the CrateStore and the Session object. This means that we construct the DepGraph long before we have enough information to know where to load the previous DepGraph from and, because the CrateStore is used very early too, we are registering dep-nodes and edges before the graph is fully operational. This is ugly but harmless in the current implementation. In the new tracking system it would become a problem.

So the plan is to construct the DepGraph only when we can already load the previous DepGraph. This needs some refactoring:

Any work in this direction will want to be based on top of #44341 the master branch.

@michaelwoerister
Copy link
Member Author

cc @nikomatsakis @alexcrichton

@alexcrichton alexcrichton added the A-incr-comp Area: Incremental compilation label Sep 7, 2017
@alexcrichton
Copy link
Member

@michaelwoerister should the DepGraph argument all throughout Decoder also be removed? For example in this function.

After #44142 lands then 100% of the existing cstore methods are now queries (yay!) or otherwise don't need to be tracked.

@michaelwoerister
Copy link
Member Author

@michaelwoerister should the DepGraph argument all throughout Decoder also be removed? For example in this function.

Ideally the "raw" CrateStore implementation would not know anything about the DepGraph anymore, just the queries (if that's possible).

@alexcrichton
Copy link
Member

Ok! Sounds like a "yes"

@alexcrichton
Copy link
Member

I have a commit for completely removing DepGraph from the CrateStore at alexcrichton@74f6a22, just waiting on #44142 to land

bors added a commit that referenced this issue Sep 10, 2017
rustc: Remove `DepGraph` handling from rustc_metadata

This should now be entirely tracked through queries, so no need to have a
`DepGraph` in the `CStore` object any more!

cc #44390
frewsxcv added a commit to frewsxcv/rust that referenced this issue Sep 11, 2017
…aelwoerister

rustc: Make `CrateStore` private to `TyCtxt`

This commit makes the `CrateStore` object private to the `ty/context.rs` module and also absent on the `Session` itself.

cc rust-lang#44390
cc rust-lang#44341 (initial commit pulled and rebased from here)
frewsxcv added a commit to frewsxcv/rust that referenced this issue Sep 11, 2017
…aelwoerister

rustc: Make `CrateStore` private to `TyCtxt`

This commit makes the `CrateStore` object private to the `ty/context.rs` module and also absent on the `Session` itself.

cc rust-lang#44390
cc rust-lang#44341 (initial commit pulled and rebased from here)
bors added a commit that referenced this issue Sep 13, 2017
rustc: Make `CrateStore` private to `TyCtxt`

This commit makes the `CrateStore` object private to the `ty/context.rs` module and also absent on the `Session` itself.

cc #44390
cc #44341 (initial commit pulled and rebased from here)
bors added a commit that referenced this issue Sep 13, 2017
rustc: Make `CrateStore` private to `TyCtxt`

This commit makes the `CrateStore` object private to the `ty/context.rs` module and also absent on the `Session` itself.

cc #44390
cc #44341 (initial commit pulled and rebased from here)
alexcrichton added a commit to alexcrichton/rust that referenced this issue Sep 14, 2017
This commit removes the `dep_graph` field from the `Session` type according to
issue rust-lang#44390. Most of the fallout here was relatively straightforward and the
`prepare_session_directory` function was rejiggered a bit to reuse the results
in the later-called `load_dep_graph` function.

Closes rust-lang#44390
bors added a commit that referenced this issue Sep 14, 2017
…elwoerister

rustc: Remove `Session::dep_graph`

This commit removes the `dep_graph` field from the `Session` type according to
issue #44390. Most of the fallout here was relatively straightforward and the
`prepare_session_directory` function was rejiggered a bit to reuse the results
in the later-called `load_dep_graph` function.

Closes #44390
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
Projects
None yet
Development

No branches or pull requests

2 participants