Skip to content

Commit

Permalink
Auto merge of rust-lang#69808 - cjgillot:vtbl, r=pnkfelix
Browse files Browse the repository at this point in the history
Avoid duplicating code for each query

There are at the moment roughly 170 queries in librustc.
The way `ty::query` is structured, a lot of code is duplicated for each query.
I suspect this to be responsible for a part of librustc'c compile time.

The first part of this PR reduces the amount of code generic on the query,
replacing it by code generic on the key-value types. I can split it out if needed.

In a second part, the non-inlined methods in the `QueryAccessors` and `QueryDescription` traits
are made into a virtual dispatch table. This allows to reduce even more the number of generated
functions.

This allows to save 1.5s on check build, and 10% on the size of the librustc.rlib.
(Attributed roughly half and half).
My computer is not good enough to measure properly compiling time.
I have no idea of the effect on performance. A perf run may be required.

cc rust-lang#65031
  • Loading branch information
bors committed May 1, 2020
2 parents 7f65393 + e4976d0 commit dba944a
Show file tree
Hide file tree
Showing 10 changed files with 240 additions and 103 deletions.
4 changes: 2 additions & 2 deletions src/librustc_macros/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ fn add_query_description_impl(
#[allow(unused_variables, unused_braces)]
fn cache_on_disk(
#tcx: TyCtxt<'tcx>,
#key: Self::Key,
#key: &Self::Key,
#value: Option<&Self::Value>
) -> bool {
#expr
Expand Down Expand Up @@ -441,7 +441,7 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
.unwrap_or(false));

let key = <#arg as DepNodeParams<TyCtxt<'_>>>::recover($tcx, $dep_node).unwrap();
if queries::#name::cache_on_disk($tcx, key, None) {
if queries::#name::cache_on_disk($tcx, &key, None) {
let _ = $tcx.#name(key);
}
}
Expand Down
25 changes: 2 additions & 23 deletions src/librustc_middle/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,31 +183,10 @@ macro_rules! define_dep_nodes {
// tuple args
$({
erase!($tuple_arg_ty);
let hash = DepNodeParams::to_fingerprint(&arg, _tcx);
let dep_node = DepNode {
kind: DepKind::$variant,
hash
};

#[cfg(debug_assertions)]
{
if !dep_node.kind.can_reconstruct_query_key() &&
(_tcx.sess.opts.debugging_opts.incremental_info ||
_tcx.sess.opts.debugging_opts.query_dep_graph)
{
_tcx.dep_graph.register_dep_node_debug_str(dep_node, || {
arg.to_debug_str(_tcx)
});
}
}

return dep_node;
return DepNode::construct(_tcx, DepKind::$variant, &arg)
})*

DepNode {
kind: DepKind::$variant,
hash: Fingerprint::ZERO,
}
return DepNode::construct(_tcx, DepKind::$variant, &())
}
)*
}
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_middle/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ impl<'tcx> DepContext for TyCtxt<'tcx> {
fn debug_dep_tasks(&self) -> bool {
self.sess.opts.debugging_opts.dep_tasks
}
fn debug_dep_node(&self) -> bool {
self.sess.opts.debugging_opts.incremental_info
|| self.sess.opts.debugging_opts.query_dep_graph
}

fn try_force_from_dep_node(&self, dep_node: &DepNode) -> bool {
// FIXME: This match is just a workaround for incremental bugs and should
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_middle/ty/query/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::dep_graph::{self, DepConstructor, DepNode, DepNodeParams};
use crate::dep_graph::{self, DepNode, DepNodeParams};
use crate::hir::exports::Export;
use crate::hir::map;
use crate::infer::canonical::{self, Canonical};
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_middle/ty/query/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ where

state.iter_results(|results| {
for (key, value, dep_node) in results {
if Q::cache_on_disk(tcx, key.clone(), Some(&value)) {
if Q::cache_on_disk(tcx, &key, Some(&value)) {
let dep_node = SerializedDepNodeIndex::new(dep_node.index());

// Record position of the cache entry.
Expand Down
6 changes: 0 additions & 6 deletions src/librustc_middle/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,6 @@ macro_rules! define_queries_inner {
&tcx.queries.$name
}

#[allow(unused)]
#[inline(always)]
fn to_dep_node(tcx: TyCtxt<$tcx>, key: &Self::Key) -> DepNode {
DepConstructor::$node(tcx, *key)
}

#[inline]
fn compute(tcx: TyCtxt<'tcx>, key: Self::Key) -> Self::Value {
let provider = tcx.queries.providers.get(key.query_crate())
Expand Down
24 changes: 24 additions & 0 deletions src/librustc_query_system/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,24 @@ impl<K: DepKind> DepNode<K> {
debug_assert!(!kind.has_params());
DepNode { kind, hash: Fingerprint::ZERO }
}

pub fn construct<Ctxt, Key>(tcx: Ctxt, kind: K, arg: &Key) -> DepNode<K>
where
Ctxt: crate::query::QueryContext<DepKind = K>,
Key: DepNodeParams<Ctxt>,
{
let hash = arg.to_fingerprint(tcx);
let dep_node = DepNode { kind, hash };

#[cfg(debug_assertions)]
{
if !kind.can_reconstruct_query_key() && tcx.debug_dep_node() {
tcx.dep_graph().register_dep_node_debug_str(dep_node, || arg.to_debug_str(tcx));
}
}

return dep_node;
}
}

impl<K: DepKind> fmt::Debug for DepNode<K> {
Expand Down Expand Up @@ -120,6 +138,12 @@ where
}
}

impl<Ctxt: DepContext> DepNodeParams<Ctxt> for () {
fn to_fingerprint(&self, _: Ctxt) -> Fingerprint {
Fingerprint::ZERO
}
}

/// A "work product" corresponds to a `.o` (or other) file that we
/// save in between runs. These IDs do not have a `DefId` but rather
/// some independent path or string that persists between runs without
Expand Down
1 change: 1 addition & 0 deletions src/librustc_query_system/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub trait DepContext: Copy {
fn create_stable_hashing_context(&self) -> Self::StableHashingContext;

fn debug_dep_tasks(&self) -> bool;
fn debug_dep_node(&self) -> bool;

/// Try to force a dep node to execute and see if it's green.
fn try_force_from_dep_node(&self, dep_node: &DepNode<Self::DepKind>) -> bool;
Expand Down
79 changes: 76 additions & 3 deletions src/librustc_query_system/query/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,53 @@ pub trait QueryConfig<CTX> {
type Stored: Clone;
}

pub(crate) struct QueryVtable<CTX: QueryContext, K, V> {
pub anon: bool,
pub dep_kind: CTX::DepKind,
pub eval_always: bool,

// Don't use this method to compute query results, instead use the methods on TyCtxt
pub compute: fn(CTX, K) -> V,

pub hash_result: fn(&mut CTX::StableHashingContext, &V) -> Option<Fingerprint>,
pub handle_cycle_error: fn(CTX, CycleError<CTX::Query>) -> V,
pub cache_on_disk: fn(CTX, &K, Option<&V>) -> bool,
pub try_load_from_disk: fn(CTX, SerializedDepNodeIndex) -> Option<V>,
}

impl<CTX: QueryContext, K, V> QueryVtable<CTX, K, V> {
pub(crate) fn to_dep_node(&self, tcx: CTX, key: &K) -> DepNode<CTX::DepKind>
where
K: crate::dep_graph::DepNodeParams<CTX>,
{
DepNode::construct(tcx, self.dep_kind, key)
}

pub(crate) fn compute(&self, tcx: CTX, key: K) -> V {
(self.compute)(tcx, key)
}

pub(crate) fn hash_result(
&self,
hcx: &mut CTX::StableHashingContext,
value: &V,
) -> Option<Fingerprint> {
(self.hash_result)(hcx, value)
}

pub(crate) fn handle_cycle_error(&self, tcx: CTX, error: CycleError<CTX::Query>) -> V {
(self.handle_cycle_error)(tcx, error)
}

pub(crate) fn cache_on_disk(&self, tcx: CTX, key: &K, value: Option<&V>) -> bool {
(self.cache_on_disk)(tcx, key, value)
}

pub(crate) fn try_load_from_disk(&self, tcx: CTX, index: SerializedDepNodeIndex) -> Option<V> {
(self.try_load_from_disk)(tcx, index)
}
}

pub trait QueryAccessors<CTX: QueryContext>: QueryConfig<CTX> {
const ANON: bool;
const EVAL_ALWAYS: bool;
Expand All @@ -34,7 +81,12 @@ pub trait QueryAccessors<CTX: QueryContext>: QueryConfig<CTX> {
// Don't use this method to access query results, instead use the methods on TyCtxt
fn query_state<'a>(tcx: CTX) -> &'a QueryState<CTX, Self::Cache>;

fn to_dep_node(tcx: CTX, key: &Self::Key) -> DepNode<CTX::DepKind>;
fn to_dep_node(tcx: CTX, key: &Self::Key) -> DepNode<CTX::DepKind>
where
Self::Key: crate::dep_graph::DepNodeParams<CTX>,
{
DepNode::construct(tcx, Self::DEP_KIND, key)
}

// Don't use this method to compute query results, instead use the methods on TyCtxt
fn compute(tcx: CTX, key: Self::Key) -> Self::Value;
Expand All @@ -51,7 +103,7 @@ pub trait QueryDescription<CTX: QueryContext>: QueryAccessors<CTX> {
fn describe(tcx: CTX, key: Self::Key) -> Cow<'static, str>;

#[inline]
fn cache_on_disk(_: CTX, _: Self::Key, _: Option<&Self::Value>) -> bool {
fn cache_on_disk(_: CTX, _: &Self::Key, _: Option<&Self::Value>) -> bool {
false
}

Expand All @@ -60,6 +112,27 @@ pub trait QueryDescription<CTX: QueryContext>: QueryAccessors<CTX> {
}
}

pub(crate) trait QueryVtableExt<CTX: QueryContext, K, V> {
const VTABLE: QueryVtable<CTX, K, V>;
}

impl<CTX, Q> QueryVtableExt<CTX, Q::Key, Q::Value> for Q
where
CTX: QueryContext,
Q: QueryDescription<CTX>,
{
const VTABLE: QueryVtable<CTX, Q::Key, Q::Value> = QueryVtable {
anon: Q::ANON,
dep_kind: Q::DEP_KIND,
eval_always: Q::EVAL_ALWAYS,
compute: Q::compute,
hash_result: Q::hash_result,
handle_cycle_error: Q::handle_cycle_error,
cache_on_disk: Q::cache_on_disk,
try_load_from_disk: Q::try_load_from_disk,
};
}

impl<CTX: QueryContext, M> QueryDescription<CTX> for M
where
M: QueryAccessors<CTX, Key = DefId>,
Expand All @@ -73,7 +146,7 @@ where
}
}

default fn cache_on_disk(_: CTX, _: Self::Key, _: Option<&Self::Value>) -> bool {
default fn cache_on_disk(_: CTX, _: &Self::Key, _: Option<&Self::Value>) -> bool {
false
}

Expand Down
Loading

0 comments on commit dba944a

Please sign in to comment.