Skip to content

[incremental] introduce ensure() operation #45210

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

Closed
nikomatsakis opened this issue Oct 11, 2017 · 2 comments
Closed

[incremental] introduce ensure() operation #45210

nikomatsakis opened this issue Oct 11, 2017 · 2 comments
Labels
E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-incr-comp Working group: Incremental compilation

Comments

@nikomatsakis
Copy link
Contributor

As part of #45208, we would like to introduce a first-class ensure() operation. Invoking Q::ensure(K) for some query Q with key K is semantically equivalent to introducing a fresh query Q1 that has a return type of unit and a provider like:

fn Q1(tcx, key: K) {
    Q(tcx, key); // discard result
}

In other words, it executes the query Q but does not use the result. This is worthwhile because it avoids the need to load the result from disk (or execute the provider, if we are not storing the results of Q to disk).

In order to introduce ensure(), we want to modify the plumbing macros for the query infrastructure. I think we could add an accessor called ensure, somewhat like try_get. It would look something like this:

/// Ensures that the query's result for `key` is up-to-date,
/// without necessarily executing the query. In particular, if all the inputs
/// to the query are green, then the query will not be re-executed. If however
/// some inputs have changed, then the query will be re-executed, and its
/// results cached (after being compared against the hash of the previous results).
///
/// This function is particularly useful when executing passes for their side-effects --
/// e.g., in order to report errors for erroneous programs.
///
/// Because the query will not execute unless it has changed, and hence the result
/// may not actually be available, this function returns unit. 
pub fn ensure(
    tcx: TyCtxt<'a, $tcx, 'lcx>,
    key: $K)
{
    let dep_node = Self::to_dep_node(tcx, &key);

    // Ensuring an "input" query makes no sense, nor anonymous queries:
    assert!(!dep_node.kind.is_anon());
    assert!(!dep_node.kind.is_input());

    // if this node has no color, then try to mark it green
    if tcx.dep_graph.node_color(&dep_node).is_none() {
        // This is an adapted variant on [this code] -- the only difference is that it does not invoke
        // `load_from_disk_and_cache_in_memory` when done. It probably makes sense to extract
        // a helper function.
        //
        // [this code]: https://github.com/rust-lang/rust/blob/72d65019c789138f555c7cf7139508d2f9f0dffe/src/librustc/ty/maps/plumbing.rs#L313-L335
        if let Some(DepNodeColor::Green(dep_node_index)) = tcx.dep_graph.node_color(&dep_node) {
            profq_msg!(tcx, ProfileQueriesMsg::CacheHit);
            tcx.dep_graph.read_index(dep_node_index);
            return; 
        }

        debug!("ty::queries::{}::try_get_with(key={:?}) - running try_mark_green",
            stringify!($name),
            key);

        if let Some(dep_node_index) = tcx.dep_graph.try_mark_green(tcx, &dep_node) {
            debug_assert!(tcx.dep_graph.is_green(dep_node_index));
            profq_msg!(tcx, ProfileQueriesMsg::CacheHit);
            tcx.dep_graph.read_index(dep_node_index);
        return;
    }

    // If the node is not green, or we could not make it green,
    // just execute the query normally and discard the result:
    tcx.$name(key);
}

Next, just to use this function somewhere, we can convert the code in librustc_typeck to use ensure. Notably, change this line from self.tcx.typeck_tables_of(body_owner_def_id) to ty::maps::queries::typeck_tables_of::ensure(self.tcx, body_owner_def_id).

(This last step is not enough to skip type-checking, since there are other uses of typeck_tables_of, but it's a necessary precondition.)

@nikomatsakis nikomatsakis added E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-incr-comp Working group: Incremental compilation labels Oct 11, 2017
@theotherjimmy
Copy link
Contributor

Already working on this. Thanks for the extra detail!

@theotherjimmy
Copy link
Contributor

@nikomatsakis I think there may be a few problems with the above code. I had to reorder the check for if the node is already green out of the check to see if the node has no color. Without that re-ordering, that check would always be false, and we would always execute green queries. Both are bad.

bors added a commit that referenced this issue Oct 15, 2017
incr.comp.: Introduce `ensure` and `ensure` typeck_tables_of

Resolves #45210

In this Pull Request we introduce the `ensure` query/function. `ensure` has the
semantics and type of the function `Q1` below:
```rust
fn Q1::ensure(K){
    Q(K);
}
```
Further, `ensure` avoids the need to load the result from disk (or execute the
provider, if we are not storing the results of Q to disk).

@nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-incr-comp Working group: Incremental compilation
Projects
None yet
Development

No branches or pull requests

2 participants