Skip to content

Avoid most allocations in Canonicalizer. #52342

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 18, 2018
Merged

Avoid most allocations in Canonicalizer. #52342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
100 changes: 68 additions & 32 deletions src/librustc/infer/canonical/canonicalizer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
//! [c]: https://rust-lang-nursery.github.io/rustc-guide/traits/canonicalization.html

use infer::canonical::{
Canonical, CanonicalTyVarKind, CanonicalVarInfo, CanonicalVarKind, CanonicalVarValues,
Canonicalized,
Canonical, CanonicalTyVarKind, CanonicalVarInfo, CanonicalVarKind, Canonicalized,
SmallCanonicalVarValues,
};
use infer::InferCtxt;
use std::sync::atomic::Ordering;
Expand All @@ -26,7 +26,8 @@ use ty::subst::Kind;
use ty::{self, CanonicalVar, Lift, Slice, Ty, TyCtxt, TypeFlags};

use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::small_vec::SmallVec;

impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
/// Canonicalizes a query value `V`. When we canonicalize a query,
Expand All @@ -47,7 +48,8 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
pub fn canonicalize_query<V>(
&self,
value: &V,
) -> (Canonicalized<'gcx, V>, CanonicalVarValues<'tcx>)
var_values: &mut SmallCanonicalVarValues<'tcx>
) -> Canonicalized<'gcx, V>
where
V: TypeFoldable<'tcx> + Lift<'gcx>,
{
Expand All @@ -65,6 +67,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
static_region: true,
other_free_regions: true,
},
var_values,
)
}

Expand Down Expand Up @@ -96,10 +99,11 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
pub fn canonicalize_response<V>(
&self,
value: &V,
) -> (Canonicalized<'gcx, V>, CanonicalVarValues<'tcx>)
) -> Canonicalized<'gcx, V>
where
V: TypeFoldable<'tcx> + Lift<'gcx>,
{
let mut var_values = SmallVec::new();
Canonicalizer::canonicalize(
value,
Some(self),
Expand All @@ -108,6 +112,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
static_region: false,
other_free_regions: false,
},
&mut var_values
)
}

Expand All @@ -123,7 +128,8 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
pub fn canonicalize_hr_query_hack<V>(
&self,
value: &V,
) -> (Canonicalized<'gcx, V>, CanonicalVarValues<'tcx>)
var_values: &mut SmallCanonicalVarValues<'tcx>
) -> Canonicalized<'gcx, V>
where
V: TypeFoldable<'tcx> + Lift<'gcx>,
{
Expand All @@ -141,6 +147,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
static_region: false,
other_free_regions: true,
},
var_values
)
}
}
Expand All @@ -163,9 +170,11 @@ impl CanonicalizeRegionMode {
struct Canonicalizer<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
infcx: Option<&'cx InferCtxt<'cx, 'gcx, 'tcx>>,
tcx: TyCtxt<'cx, 'gcx, 'tcx>,
variables: IndexVec<CanonicalVar, CanonicalVarInfo>,
variables: SmallVec<[CanonicalVarInfo; 8]>,
var_values: &'cx mut SmallCanonicalVarValues<'tcx>,
// Note that indices is only used once `var_values` is big enough to be
// heap-allocated.
indices: FxHashMap<Kind<'tcx>, CanonicalVar>,
var_values: IndexVec<CanonicalVar, Kind<'tcx>>,
canonicalize_region_mode: CanonicalizeRegionMode,
needs_canonical_flags: TypeFlags,
}
Expand Down Expand Up @@ -295,7 +304,8 @@ impl<'cx, 'gcx, 'tcx> Canonicalizer<'cx, 'gcx, 'tcx> {
infcx: Option<&'cx InferCtxt<'cx, 'gcx, 'tcx>>,
tcx: TyCtxt<'cx, 'gcx, 'tcx>,
canonicalize_region_mode: CanonicalizeRegionMode,
) -> (Canonicalized<'gcx, V>, CanonicalVarValues<'tcx>)
var_values: &'cx mut SmallCanonicalVarValues<'tcx>
) -> Canonicalized<'gcx, V>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just return this? I guess it's more efficient this way...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Copying the SmallCanonicalVars reduces the size of the win by about 20--25%. I figure we need every saving we can get for NLL!

where
V: TypeFoldable<'tcx> + Lift<'gcx>,
{
Expand All @@ -320,20 +330,17 @@ impl<'cx, 'gcx, 'tcx> Canonicalizer<'cx, 'gcx, 'tcx> {
variables: Slice::empty(),
value: out_value,
};
let values = CanonicalVarValues {
var_values: IndexVec::default(),
};
return (canon_value, values);
return canon_value;
}

let mut canonicalizer = Canonicalizer {
infcx,
tcx,
canonicalize_region_mode,
needs_canonical_flags,
variables: IndexVec::default(),
variables: SmallVec::new(),
var_values,
indices: FxHashMap::default(),
var_values: IndexVec::default(),
};
let out_value = value.fold_with(&mut canonicalizer);

Expand All @@ -348,16 +355,12 @@ impl<'cx, 'gcx, 'tcx> Canonicalizer<'cx, 'gcx, 'tcx> {
)
});

let canonical_variables = tcx.intern_canonical_var_infos(&canonicalizer.variables.raw);
let canonical_variables = tcx.intern_canonical_var_infos(&canonicalizer.variables);

let canonical_value = Canonical {
Canonical {
variables: canonical_variables,
value: out_value,
};
let canonical_var_values = CanonicalVarValues {
var_values: canonicalizer.var_values,
};
(canonical_value, canonical_var_values)
}
}

/// Creates a canonical variable replacing `kind` from the input,
Expand All @@ -366,21 +369,54 @@ impl<'cx, 'gcx, 'tcx> Canonicalizer<'cx, 'gcx, 'tcx> {
/// potentially a free region).
fn canonical_var(&mut self, info: CanonicalVarInfo, kind: Kind<'tcx>) -> CanonicalVar {
let Canonicalizer {
indices,
variables,
var_values,
indices,
..
} = self;

indices
.entry(kind)
.or_insert_with(|| {
let cvar1 = variables.push(info);
let cvar2 = var_values.push(kind);
assert_eq!(cvar1, cvar2);
cvar1
})
.clone()
// This code is hot. `variables` and `var_values` are usually small
// (fewer than 8 elements ~95% of the time). They are SmallVec's to
// avoid allocations in those cases. We also don't use `indices` to
// determine if a kind has been seen before until the limit of 8 has
// been exceeded, to also avoid allocations for `indices`.
if var_values.is_array() {
// `var_values` is stack-allocated. `indices` isn't used yet. Do a
// direct linear search of `var_values`.
if let Some(idx) = var_values.iter().position(|&k| k == kind) {
// `kind` is already present in `var_values`.
CanonicalVar::new(idx)
} else {
// `kind` isn't present in `var_values`. Append it. Likewise
// for `info` and `variables`.
variables.push(info);
var_values.push(kind);
assert_eq!(variables.len(), var_values.len());

// If `var_values` has become big enough to be heap-allocated,
// fill up `indices` to facilitate subsequent lookups.
if !var_values.is_array() {
assert!(indices.is_empty());
*indices =
var_values.iter()
.enumerate()
.map(|(i, &kind)| (kind, CanonicalVar::new(i)))
.collect();
}
// The cv is the index of the appended element.
CanonicalVar::new(var_values.len() - 1)
}
} else {
// `var_values` is large. Do a hashmap search via `indices`.
*indices
.entry(kind)
.or_insert_with(|| {
variables.push(info);
var_values.push(kind);
assert_eq!(variables.len(), var_values.len());
CanonicalVar::new(variables.len() - 1)
})
}
}

/// Given a type variable `ty_var` of the given kind, first check
Expand Down
9 changes: 5 additions & 4 deletions src/librustc/infer/canonical/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

use infer::{InferCtxt, RegionVariableOrigin, TypeVariableOrigin};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::small_vec::SmallVec;
use rustc_data_structures::sync::Lrc;
use serialize::UseSpecializedDecodable;
use std::ops::Index;
Expand Down Expand Up @@ -74,6 +75,10 @@ pub struct CanonicalVarValues<'tcx> {
pub var_values: IndexVec<CanonicalVar, Kind<'tcx>>,
}

/// Like CanonicalVarValues, but for use in places where a SmallVec is
/// appropriate.
pub type SmallCanonicalVarValues<'tcx> = SmallVec<[Kind<'tcx>; 8]>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just use this everywhere...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because CanonicalVarValue derives Clone, Debug, PartialEq, Eq, Hash, RustcDecodable, and RustcEncodable. In contrast, SmallVec doesn't define any of those. Also, I don't know what the impact of possible copying of SmallVecs (which are quite large, in terms of the number of bytes they take up on the stack) in lots of other places.


/// Information about a canonical variable that is included with the
/// canonical value. This is sufficient information for code to create
/// a copy of the canonical value in some other inference context,
Expand Down Expand Up @@ -281,10 +286,6 @@ BraceStructLiftImpl! {
}

impl<'tcx> CanonicalVarValues<'tcx> {
fn iter<'a>(&'a self) -> impl Iterator<Item = Kind<'tcx>> + 'a {
self.var_values.iter().cloned()
}

fn len(&self) -> usize {
self.var_values.len()
}
Expand Down
29 changes: 15 additions & 14 deletions src/librustc/infer/canonical/query_result.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

use infer::canonical::substitute::substitute_value;
use infer::canonical::{Canonical, CanonicalVarKind, CanonicalVarValues, CanonicalizedQueryResult,
Certainty, QueryRegionConstraint, QueryResult};
Certainty, QueryRegionConstraint, QueryResult, SmallCanonicalVarValues};
use infer::region_constraints::{Constraint, RegionConstraintData};
use infer::InferCtxtBuilder;
use infer::{InferCtxt, InferOk, InferResult, RegionObligation};
Expand Down Expand Up @@ -103,7 +103,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
T: Debug + Lift<'gcx> + TypeFoldable<'tcx>,
{
let query_result = self.make_query_result(inference_vars, answer, fulfill_cx)?;
let (canonical_result, _) = self.canonicalize_response(&query_result);
let canonical_result = self.canonicalize_response(&query_result);

debug!(
"make_canonicalized_query_result: canonical_result = {:#?}",
Expand Down Expand Up @@ -186,7 +186,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
&self,
cause: &ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
original_values: &CanonicalVarValues<'tcx>,
original_values: &SmallCanonicalVarValues<'tcx>,
query_result: &Canonical<'tcx, QueryResult<'tcx, R>>,
) -> InferResult<'tcx, R>
where
Expand Down Expand Up @@ -252,7 +252,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
&self,
cause: &ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
original_values: &CanonicalVarValues<'tcx>,
original_values: &SmallCanonicalVarValues<'tcx>,
query_result: &Canonical<'tcx, QueryResult<'tcx, R>>,
output_query_region_constraints: &mut Vec<QueryRegionConstraint<'tcx>>,
) -> InferResult<'tcx, R>
Expand All @@ -274,10 +274,11 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
// variable...
let mut obligations = vec![];

for (index, original_value) in original_values.var_values.iter_enumerated() {
for (index, original_value) in original_values.iter().enumerate() {
// ...with the value `v_r` of that variable from the query.
let result_value = query_result
.substitute_projected(self.tcx, &result_subst, |v| &v.var_values[index]);
.substitute_projected(self.tcx, &result_subst,
|v| &v.var_values[CanonicalVar::new(index)]);
match (original_value.unpack(), result_value.unpack()) {
(UnpackedKind::Lifetime(ty::ReErased), UnpackedKind::Lifetime(ty::ReErased)) => {
// no action needed
Expand Down Expand Up @@ -341,7 +342,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
&self,
cause: &ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
original_values: &CanonicalVarValues<'tcx>,
original_values: &SmallCanonicalVarValues<'tcx>,
query_result: &Canonical<'tcx, QueryResult<'tcx, R>>,
) -> InferResult<'tcx, CanonicalVarValues<'tcx>>
where
Expand Down Expand Up @@ -382,7 +383,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
fn query_result_substitution_guess<R>(
&self,
cause: &ObligationCause<'tcx>,
original_values: &CanonicalVarValues<'tcx>,
original_values: &SmallCanonicalVarValues<'tcx>,
query_result: &Canonical<'tcx, QueryResult<'tcx, R>>,
) -> CanonicalVarValues<'tcx>
where
Expand Down Expand Up @@ -418,14 +419,14 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
// e.g., here `result_value` might be `?0` in the example above...
if let ty::TyInfer(ty::InferTy::CanonicalTy(index)) = result_value.sty {
// in which case we would set `canonical_vars[0]` to `Some(?U)`.
opt_values[index] = Some(original_value);
opt_values[index] = Some(*original_value);
}
}
UnpackedKind::Lifetime(result_value) => {
// e.g., here `result_value` might be `'?1` in the example above...
if let &ty::RegionKind::ReCanonical(index) = result_value {
// in which case we would set `canonical_vars[0]` to `Some('static)`.
opt_values[index] = Some(original_value);
opt_values[index] = Some(*original_value);
}
}
}
Expand Down Expand Up @@ -459,7 +460,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
&self,
cause: &ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
original_values: &CanonicalVarValues<'tcx>,
original_values: &SmallCanonicalVarValues<'tcx>,
result_subst: &CanonicalVarValues<'tcx>,
query_result: &Canonical<'tcx, QueryResult<'tcx, R>>,
) -> InferResult<'tcx, ()>
Expand Down Expand Up @@ -522,13 +523,13 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
&self,
cause: &ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
variables1: &CanonicalVarValues<'tcx>,
variables1: &SmallCanonicalVarValues<'tcx>,
variables2: impl Fn(CanonicalVar) -> Kind<'tcx>,
) -> InferResult<'tcx, ()> {
self.commit_if_ok(|_| {
let mut obligations = vec![];
for (index, value1) in variables1.var_values.iter_enumerated() {
let value2 = variables2(index);
for (index, value1) in variables1.iter().enumerate() {
let value2 = variables2(CanonicalVar::new(index));

match (value1.unpack(), value2.unpack()) {
(UnpackedKind::Type(v1), UnpackedKind::Type(v2)) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/canonical/substitute.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl<'tcx, V> Canonical<'tcx, V> {
where
T: TypeFoldable<'tcx>,
{
assert_eq!(self.variables.len(), var_values.var_values.len());
assert_eq!(self.variables.len(), var_values.len());
let value = projection_fn(&self.value);
substitute_value(tcx, var_values, value)
}
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/traits/query/dropck_outlives.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use infer::at::At;
use infer::InferOk;
use rustc_data_structures::small_vec::SmallVec;
use std::iter::FromIterator;
use syntax::codemap::Span;
use ty::subst::Kind;
Expand Down Expand Up @@ -50,7 +51,8 @@ impl<'cx, 'gcx, 'tcx> At<'cx, 'gcx, 'tcx> {
}

let gcx = tcx.global_tcx();
let (c_ty, orig_values) = self.infcx.canonicalize_query(&self.param_env.and(ty));
let mut orig_values = SmallVec::new();
let c_ty = self.infcx.canonicalize_query(&self.param_env.and(ty), &mut orig_values);
let span = self.cause.span;
debug!("c_ty = {:?}", c_ty);
match &gcx.dropck_outlives(c_ty) {
Expand Down
6 changes: 4 additions & 2 deletions src/librustc/traits/query/evaluate_obligation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// except according to those terms.

use infer::InferCtxt;
use rustc_data_structures::small_vec::SmallVec;
use traits::{EvaluationResult, PredicateObligation, SelectionContext,
TraitQueryMode, OverflowError};

Expand Down Expand Up @@ -38,8 +39,9 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
&self,
obligation: &PredicateObligation<'tcx>,
) -> EvaluationResult {
let (c_pred, _) =
self.canonicalize_query(&obligation.param_env.and(obligation.predicate));
let mut _orig_values = SmallVec::new();
let c_pred = self.canonicalize_query(&obligation.param_env.and(obligation.predicate),
&mut _orig_values);
// Run canonical query. If overflow occurs, rerun from scratch but this time
// in standard trait query mode so that overflow is handled appropriately
// within `SelectionContext`.
Expand Down
Loading