This post describes my PR series which moves the rustc_interface queries into proper TyCtxt queries. I’ll attempt to describe the more important changes in the PRs and how the less ideal aspects of them could be improved in the future.
Non-incremental queries
Queries which are eval_always (always executes the query) and no_hash (assumes the query result changed if the query executed) are interesting because they do not require the dep graph to execute. Because of that property, I’ll call these non-incremental queries. This means we can execute these queries while we are loading the dep graph in the background, and that is implemented in #59338.
This kind of queries is used in all the PRs linked in this post. Because of no_hash we don’t have to hash the result of these queries which means they are faster and we also don’t need to implement StableHash for the types we return.
PRs
Turn HIR indexing into a query #59064
This adds a hir_map query which indexes the HIR and constructs the hir::map::Map type. Using this query directly would cause the caller to always execute, since hir_map is a non-incremental query. To avoid this we use with_ignore in the TyCtxt::hir accessor function so we not not gain a dependency on the non-incremental query. We can do this because the hir::map::Map type does its own custom dependency tracking.
The TyCtxt::hir accessor function is accessed quite frequently so we use a cache for the result of the hir_map query in GlobalCtxt. Since we use with_ignore anyway, this cache does not need to add any dependencies.
I imagine we’ll replace the hir_map and hir function and cache with a query which takes an item and returns a smaller map for just the ItemLocalIds for that item. Another krate query can replace the whole crate API’s in the HIR map. I’m not sure how well this scheme will perform. In particular we may want a method to avoid hashing the HIR twice. Having krate be a no_hash query might suffice here.
Another problem here is what should happen when using the hir_map query if parsing resulted in an error. A poison HIR map probably doesn’t make sense, so we’d probably want to return an empty HIR map. This can also be dealt with by always ensuring that parsing returns some value.
Turn HIR lowering into a query #59205
This adds a lowered_hir query which produces a new LoweredHir type which consists of fields that were previously in GlobalCtxt. This query uses () as the key since the query system internally depends on the lowered_hir query for the common key types CrateNum and DefId. This also means that there’s an assumption that the lowered_hir query has already been called and did not result in an error when executing a query which uses CrateNum, DefIndex, DefId or HirId as a key or as part of the result (due to hashing).
This issue can be fixed by making CrateNum and DefIndex interned values, instead of relying on the output of lowered_hir.
Due to its use internally in the query system, lowered_hir is cached in the same way as hir_map. We don’t add any dependency on lowered_hir when we use it in the query system. The correctness of this is a bit unclear, but since we assume that callers must call lowered_hir before using any queries requiring it, they must already have some form of dependency on it. More fine grained; we assume that the callers gets their keys (say DefId) from a possible indirect dependency on lowered_hir, hence they already have a dependency on the part of the lowered_hir query’s result that the query system uses. A similar argument holds for when DefId is used in the result of a query and requires hashing (which also uses lowered_hir). This issue also goes away by making CrateNum and DefIndex interned values, since the query system would no longer require the use of lowered_hir internally.
The cached lowered_hir is used for a number of narrowing queries which just extracts results from it (in_scope_traits_map, maybe_unused_trait_import, etc.). This is fine since these are marked eval_always and thus do not require dependencies to be recorded.
This PR also changes the task system to not require StableHashingContext for tasks that does not use hashing. This could be split to a separate PR.
The lowered_hir query requires the Resolver / BoxedResolver type, but that is not defined in librustc. Any is used to allow librustc to refer to it. This can be avoided by having a modular query system or by moving the Resolver or the whole librustc_resolve crate into librustc.
A prepare_outputs query is also added, which is the same as the output_filenames query, but it returns an error if the preparation failed. Currently my PR just makes the existing output_filenames query call prepare_outputs and then unwrap its result. This works because all the callers of output_filenames previously calls prepare_outputs and stops if it fails. A better solution would be to pipe the result of prepare_outputs through and remove the output_filenames query.
Turn macro expansion and name resolution into a query #59282
This adds a expand_macros query, which requires the CStore type, which again is not available in librustc so Any is used here too.
It also changes the lowered_hir cache function to also call allocate_metadata_dep_nodes. We’ll likely get rid of allocate_metadata_dep_nodes by querifying CStore.
Turn parsing into a query #59338
This adds parse, register_plugins and early_crate_name queries. Two other queries, crate_name and original_crate_name, depend on the early_crate_name, but uses unwrap on it; similar to the way output_filenames was handled. Instead of using Result for early_crate_name we could assume there’s no #![crate_name] if parsing fails.
We currently load the dep graph in the background, but this no longer works if we’re using queries immediately in the compiler. So the query system is changed to not require the dep graph for the non-incremental queries I described earlier. If we execute some other form of query, we’ll block on the dep graph loading to finish. Since the early queries are all non-incremental, we can keep loading it in the background avoiding regressing compile time.
A NonIncremental dep node is introduced which is always marked red, which means that any query which depend on it will always execute. When a query depends on an non-incremental query we add a dependency to NonIncremental. So the NonIncremental dep node represents all non-incremental queries. This is an easy way to ensure all non-incremental queries are marked red. We would otherwise have to record which queries we executed before the dep graph was loaded, and mark them red after.
A dep_graph_future query is added which starts the background loading of the dep graph, while the load_dep_graph query waits for it to finish, allowing you to access the dep graph.
A load_query_result_cache query is added which loads the query result cache and the query system is modified to load it on-demand.
When parallel queries are on by default, we can load both the dep graph and the query result cache by just executing the queries in the background like this:
tcx.scope.spawn(|_| {
tcx.dep_graph_future();
tcx.load_query_result_cache();
});
We can then get rid of the dep_graph_future query and related code.
Make ongoing_codegen a query #59404
This adds a ongoing_codegen query. Here Any is used to refer to dyn CodegenBackend which is not available in librustc. Otherwise this PR isn’t very interesting.
Remove queries from rustc_interface #59904
Finally this PR removes the query types and APIs from rustc_interface. Users of rustc_interface now gets access to the TyCtxt in a closure instead.
Summary
The less than ideal aspects of these PRs are:
-
Using
Anyto refer to types not accessable fromlibrustc. This can be avoided by having a modular query system or by moving the types intolibrustc. -
Queries can now fail due to the
parsequery or theprepare_outputsquery failing. We can resolve theparsecase by just producing an AST anyway (potentially with poison nodes). Ifprepare_outputsfails we should skip producing outputs, and if it doesn’t, we should pipe the result through instead of usingunwrapon it later. -
The query system now internally depends more on other queries. This can be fixed by making
CrateNumandDefIndexinterned values. We also do some questionable things with constants and Miri allocations, but that is not made worse by this PR series. -
We now load the dep graph in the background to avoid regressing compile time. I don’t think this change is avoidable. What’s worse, as we make more things in the early stages incremental, we could regress compilation time because we no longer have as much time to load the dep graph, but we’d have more nodes to load. We may need some more complex scheme here, like having multiple dep graphs or allowing use of the dep graph while it’s still loading. A simpler solution might be to just decode the dep graph in parallel, which should scale pretty well and we need to decode pretty much all of it anyway, but this can hurt compile time unless the rest of the compiler is sufficiently parallelized.