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

Misc performance tweaks #57916

Merged
merged 10 commits into from
Feb 1, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,7 @@ define_dep_nodes!( <'tcx>
[] CheckModPrivacy(DefId),
[] CheckModIntrinsics(DefId),
[] CheckModLiveness(DefId),
[] CheckModImplWf(DefId),
[] CollectModItemTypes(DefId),

[] Reachability,
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/hir/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

use ty::TyCtxt;
use ty::query::Providers;
use ty::query::queries;

use hir;
use hir::def_id::DefId;
Expand Down Expand Up @@ -355,7 +354,7 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckAttrVisitor<'a, 'tcx> {

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for &module in tcx.hir().krate().modules.keys() {
queries::check_mod_attrs::ensure(tcx, tcx.hir().local_def_id(module));
tcx.ensure().check_mod_attrs(tcx.hir().local_def_id(module));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/intrinsicck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use hir::def::Def;
use hir::def_id::DefId;
use ty::{self, Ty, TyCtxt};
use ty::layout::{LayoutError, Pointer, SizeSkeleton, VariantIdx};
use ty::query::{Providers, queries};
use ty::query::Providers;

use rustc_target::spec::abi::Abi::RustIntrinsic;
use rustc_data_structures::indexed_vec::Idx;
Expand All @@ -12,7 +12,7 @@ use hir;

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for &module in tcx.hir().krate().modules.keys() {
queries::check_mod_intrinsics::ensure(tcx, tcx.hir().local_def_id(module));
tcx.ensure().check_mod_intrinsics(tcx.hir().local_def_id(module));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ use self::VarKind::*;
use hir::def::*;
use hir::Node;
use ty::{self, TyCtxt};
use ty::query::{Providers, queries};
use ty::query::Providers;
use lint;
use errors::Applicability;
use util::nodemap::{NodeMap, HirIdMap, HirIdSet};
Expand Down Expand Up @@ -187,7 +187,7 @@ fn check_mod_liveness<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) {

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for &module in tcx.hir().krate().modules.keys() {
queries::check_mod_liveness::ensure(tcx, tcx.hir().local_def_id(module));
tcx.ensure().check_mod_liveness(tcx.hir().local_def_id(module));
}
tcx.sess.abort_if_errors();
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/middle/stability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use hir::def::Def;
use hir::def_id::{CrateNum, CRATE_DEF_INDEX, DefId, LOCAL_CRATE};
use hir::intravisit::{self, Visitor, NestedVisitorMap};
use ty::query::Providers;
use ty::query::queries;
use middle::privacy::AccessLevels;
use session::{DiagnosticMessageId, Session};
use syntax::symbol::Symbol;
Expand Down Expand Up @@ -458,7 +457,7 @@ impl<'a, 'tcx> Index<'tcx> {

pub fn check_unstable_api_usage<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
for &module in tcx.hir().krate().modules.keys() {
queries::check_mod_unstable_api_usage::ensure(tcx, tcx.hir().local_def_id(module));
tcx.ensure().check_mod_unstable_api_usage(tcx.hir().local_def_id(module));
}
}

Expand Down
56 changes: 34 additions & 22 deletions src/librustc/ty/query/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub(super) trait QueryDescription<'tcx>: QueryAccessors<'tcx> {
fn describe(tcx: TyCtxt<'_, '_, '_>, key: Self::Key) -> Cow<'static, str>;

#[inline]
fn cache_on_disk(_: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, _: Self::Key) -> bool {
false
}

Expand Down Expand Up @@ -136,6 +136,15 @@ impl<'tcx> QueryDescription<'tcx> for queries::check_mod_liveness<'tcx> {
}
}

impl<'tcx> QueryDescription<'tcx> for queries::check_mod_impl_wf<'tcx> {
fn describe(
tcx: TyCtxt<'_, '_, '_>,
key: DefId,
) -> Cow<'static, str> {
format!("checking that impls are well-formed in {}", key.describe_as_module(tcx)).into()
}
}

impl<'tcx> QueryDescription<'tcx> for queries::collect_mod_item_types<'tcx> {
fn describe(
tcx: TyCtxt<'_, '_, '_>,
Expand Down Expand Up @@ -378,7 +387,7 @@ impl<'tcx> QueryDescription<'tcx> for queries::const_eval<'tcx> {
}

#[inline]
fn cache_on_disk(_key: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, _key: Self::Key) -> bool {
true
}

Expand All @@ -398,7 +407,7 @@ impl<'tcx> QueryDescription<'tcx> for queries::const_eval_raw<'tcx> {
}

#[inline]
fn cache_on_disk(_key: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, _key: Self::Key) -> bool {
true
}

Expand All @@ -422,7 +431,7 @@ impl<'tcx> QueryDescription<'tcx> for queries::symbol_name<'tcx> {
}

#[inline]
fn cache_on_disk(_: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, _: Self::Key) -> bool {
true
}

Expand Down Expand Up @@ -496,7 +505,7 @@ impl<'tcx> QueryDescription<'tcx> for queries::const_is_rvalue_promotable_to_sta
}

#[inline]
fn cache_on_disk(_: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, _: Self::Key) -> bool {
true
}

Expand Down Expand Up @@ -530,7 +539,7 @@ impl<'tcx> QueryDescription<'tcx> for queries::codegen_fulfill_obligation<'tcx>
}

#[inline]
fn cache_on_disk(_: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, _: Self::Key) -> bool {
true
}

Expand Down Expand Up @@ -868,7 +877,7 @@ impl<'tcx> QueryDescription<'tcx> for queries::features_query<'tcx> {

impl<'tcx> QueryDescription<'tcx> for queries::typeck_tables_of<'tcx> {
#[inline]
fn cache_on_disk(def_id: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, def_id: Self::Key) -> bool {
def_id.is_local()
}

Expand All @@ -885,7 +894,7 @@ impl<'tcx> QueryDescription<'tcx> for queries::typeck_tables_of<'tcx> {

impl<'tcx> QueryDescription<'tcx> for queries::optimized_mir<'tcx> {
#[inline]
fn cache_on_disk(def_id: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, def_id: Self::Key) -> bool {
def_id.is_local()
}

Expand Down Expand Up @@ -924,7 +933,7 @@ impl<'tcx> QueryDescription<'tcx> for queries::instance_def_size_estimate<'tcx>

impl<'tcx> QueryDescription<'tcx> for queries::generics_of<'tcx> {
#[inline]
fn cache_on_disk(def_id: Self::Key) -> bool {
fn cache_on_disk(_: TyCtxt<'_, 'tcx, 'tcx>, def_id: Self::Key) -> bool {
def_id.is_local()
}

Expand Down Expand Up @@ -974,10 +983,10 @@ impl<'tcx> QueryDescription<'tcx> for queries::backend_optimization_level<'tcx>
}

macro_rules! impl_disk_cacheable_query(
($query_name:ident, |$key:tt| $cond:expr) => {
($query_name:ident, |$tcx:tt, $key:tt| $cond:expr) => {
impl<'tcx> QueryDescription<'tcx> for queries::$query_name<'tcx> {
#[inline]
fn cache_on_disk($key: Self::Key) -> bool {
fn cache_on_disk($tcx: TyCtxt<'_, 'tcx, 'tcx>, $key: Self::Key) -> bool {
$cond
}

Expand All @@ -991,14 +1000,17 @@ macro_rules! impl_disk_cacheable_query(
}
);

impl_disk_cacheable_query!(unsafety_check_result, |def_id| def_id.is_local());
impl_disk_cacheable_query!(borrowck, |def_id| def_id.is_local());
impl_disk_cacheable_query!(mir_borrowck, |def_id| def_id.is_local());
impl_disk_cacheable_query!(mir_const_qualif, |def_id| def_id.is_local());
impl_disk_cacheable_query!(check_match, |def_id| def_id.is_local());
impl_disk_cacheable_query!(def_symbol_name, |_| true);
impl_disk_cacheable_query!(type_of, |def_id| def_id.is_local());
impl_disk_cacheable_query!(predicates_of, |def_id| def_id.is_local());
impl_disk_cacheable_query!(used_trait_imports, |def_id| def_id.is_local());
impl_disk_cacheable_query!(codegen_fn_attrs, |_| true);
impl_disk_cacheable_query!(specialization_graph_of, |_| true);
impl_disk_cacheable_query!(mir_borrowck, |tcx, def_id| {
def_id.is_local() && tcx.is_closure(def_id)
});
Copy link
Member

Choose a reason for hiding this comment

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

What's the motivation here? If the result is requested (even if it's not used) then the query will be re-run. So unless we make sure that only ensure is used in all cases that don't need the result, this could backfire. There's at least one case where ensure is not used here:

|| tcx.par_body_owners(|def_id| { tcx.mir_borrowck(def_id); }));

I wonder if this is worth the risk.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just to avoid spending time storing the results of mir_borrowck for cases where it will never be used again.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that makes sense. I wondering though if there's a way of making sure that this doesn't silently break. If somebody adds a non-ensure() call to mir_borrowck it would go mostly unnoticed. Well, I guess we would notice on perf.rlo after the fact.


impl_disk_cacheable_query!(unsafety_check_result, |_, def_id| def_id.is_local());
impl_disk_cacheable_query!(borrowck, |_, def_id| def_id.is_local());
impl_disk_cacheable_query!(mir_const_qualif, |_, def_id| def_id.is_local());
impl_disk_cacheable_query!(check_match, |_, def_id| def_id.is_local());
impl_disk_cacheable_query!(def_symbol_name, |_, _| true);
impl_disk_cacheable_query!(type_of, |_, def_id| def_id.is_local());
impl_disk_cacheable_query!(predicates_of, |_, def_id| def_id.is_local());
impl_disk_cacheable_query!(used_trait_imports, |_, def_id| def_id.is_local());
impl_disk_cacheable_query!(codegen_fn_attrs, |_, _| true);
impl_disk_cacheable_query!(specialization_graph_of, |_, _| true);
2 changes: 2 additions & 0 deletions src/librustc/ty/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,8 @@ define_queries! { <'tcx>

[] fn check_mod_liveness: CheckModLiveness(DefId) -> (),

[] fn check_mod_impl_wf: CheckModImplWf(DefId) -> (),

[] fn collect_mod_item_types: CollectModItemTypes(DefId) -> (),

/// Caches CoerceUnsized kinds for impls on custom types.
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/query/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ impl<'sess> OnDiskCache<'sess> {
assert!(cache.active.is_empty());
for (key, entry) in cache.results.iter() {
use ty::query::config::QueryDescription;
if const_eval::cache_on_disk(key.clone()) {
if const_eval::cache_on_disk(tcx, key.clone()) {
if let Ok(ref value) = entry.value {
let dep_node = SerializedDepNodeIndex::new(entry.index.index());

Expand Down Expand Up @@ -1086,7 +1086,7 @@ fn encode_query_results<'enc, 'a, 'tcx, Q, E>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let map = Q::query_cache(tcx).borrow();
assert!(map.active.is_empty());
for (key, entry) in map.results.iter() {
if Q::cache_on_disk(key.clone()) {
if Q::cache_on_disk(tcx, key.clone()) {
let dep_node = SerializedDepNodeIndex::new(entry.index.index());

// Record position of the cache entry
Expand Down
38 changes: 24 additions & 14 deletions src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
debug_assert!(self.dep_graph.is_green(dep_node));

// First we try to load the result from the on-disk cache
let result = if Q::cache_on_disk(key.clone()) &&
let result = if Q::cache_on_disk(self.global_tcx(), key.clone()) &&
self.sess.opts.debugging_opts.incremental_queries {
let result = Q::try_load_from_disk(self.global_tcx(), prev_dep_node_index);

Expand Down Expand Up @@ -969,20 +969,20 @@ macro_rules! define_queries_inner {
fn handle_cycle_error(tcx: TyCtxt<'_, 'tcx, '_>) -> Self::Value {
handle_cycle_error!([$($modifiers)*][tcx])
}
})*

#[derive(Copy, Clone)]
pub struct TyCtxtEnsure<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
pub tcx: TyCtxt<'a, 'gcx, 'tcx>,
}

impl<'a, $tcx, 'lcx> queries::$name<$tcx> {
/// Ensure that either this query has all green inputs or been executed.
/// Executing query::ensure(D) is considered a read of the dep-node D.
///
/// This function is particularly useful when executing passes for their
/// side-effects -- e.g., in order to report errors for erroneous programs.
///
/// Note: The optimization is only available during incr. comp.
pub fn ensure(tcx: TyCtxt<'a, $tcx, 'lcx>, key: $K) -> () {
tcx.ensure_query::<queries::$name<'_>>(key);
}
})*
impl<'a, $tcx, 'lcx> TyCtxtEnsure<'a, $tcx, 'lcx> {
$($(#[$attr])*
#[inline(always)]
pub fn $name(self, key: $K) {
self.tcx.ensure_query::<queries::$name<'_>>(key)
})*
}

#[derive(Copy, Clone)]
pub struct TyCtxtAt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
Expand All @@ -999,6 +999,15 @@ macro_rules! define_queries_inner {
}

impl<'a, $tcx, 'lcx> TyCtxt<'a, $tcx, 'lcx> {
/// Return a transparent wrapper for `TyCtxt` which ensures queries
/// are executed instead of returing their result
#[inline(always)]
pub fn ensure(self) -> TyCtxtEnsure<'a, $tcx, 'lcx> {
TyCtxtEnsure {
tcx: self,
}
}

/// Return a transparent wrapper for `TyCtxt` which uses
/// `span` as the location of queries performed through it.
#[inline(always)]
Expand Down Expand Up @@ -1251,6 +1260,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>,
DepKind::CheckModPrivacy => { force!(check_mod_privacy, def_id!()); }
DepKind::CheckModIntrinsics => { force!(check_mod_intrinsics, def_id!()); }
DepKind::CheckModLiveness => { force!(check_mod_liveness, def_id!()); }
DepKind::CheckModImplWf => { force!(check_mod_impl_wf, def_id!()); }
DepKind::CollectModItemTypes => { force!(collect_mod_item_types, def_id!()); }
DepKind::Reachability => { force!(reachable_set, LOCAL_CRATE); }
DepKind::MirKeys => { force!(mir_keys, LOCAL_CRATE); }
Expand Down Expand Up @@ -1433,7 +1443,7 @@ macro_rules! impl_load_from_cache {
match self.kind {
$(DepKind::$dep_kind => {
let def_id = self.extract_def_id(tcx).unwrap();
queries::$query_name::cache_on_disk(def_id)
queries::$query_name::cache_on_disk(tcx.global_tcx(), def_id)
})*
_ => false
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
return None;
};

ty::query::queries::coherent_trait::ensure(self, drop_trait);
self.ensure().coherent_trait(drop_trait);

let mut dtor_did = None;
let ty = self.type_of(adt_did);
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ pub type LoanDataFlow<'a, 'tcx> = DataFlowContext<'a, 'tcx, LoanDataFlowOperator

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
tcx.par_body_owners(|body_owner_def_id| {
tcx.borrowck(body_owner_def_id);
tcx.ensure().borrowck(body_owner_def_id);
});
}

Expand Down Expand Up @@ -121,7 +121,7 @@ fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId)
// Note that `mir_validated` is a "stealable" result; the
// thief, `optimized_mir()`, forces borrowck, so we know that
// is not yet stolen.
ty::query::queries::mir_validated::ensure(tcx, owner_def_id);
tcx.ensure().mir_validated(owner_def_id);

// option dance because you can't capture an uninitialized variable
// by mut-ref.
Expand Down
18 changes: 3 additions & 15 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,22 +28,10 @@ use syntax::ast;
use syntax::ptr::P;
use syntax_pos::{Span, DUMMY_SP, MultiSpan};

struct OuterVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx> }

impl<'a, 'tcx> Visitor<'tcx> for OuterVisitor<'a, 'tcx> {
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
NestedVisitorMap::OnlyBodies(&self.tcx.hir())
}

fn visit_body(&mut self, body: &'tcx hir::Body) {
intravisit::walk_body(self, body);
let def_id = self.tcx.hir().body_owner_def_id(body.id());
let _ = self.tcx.check_match(def_id);
}
}

pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
tcx.hir().krate().visit_all_item_likes(&mut OuterVisitor { tcx }.as_deep_visitor());
for def_id in tcx.body_owners() {
tcx.ensure().check_match(def_id);
}
tcx.sess.abort_if_errors();
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,10 @@ fn mir_validated<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx
fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx Mir<'tcx> {
// (Mir-)Borrowck uses `mir_validated`, so we have to force it to
// execute before we can steal.
let _ = tcx.mir_borrowck(def_id);
tcx.ensure().mir_borrowck(def_id);

if tcx.use_ast_borrowck() {
let _ = tcx.borrowck(def_id);
tcx.ensure().borrowck(def_id);
}

let mut mir = tcx.mir_validated(def_id).steal();
Expand Down
Loading