Skip to content

Commit

Permalink
Rollup merge of rust-lang#53025 - ljedrz:debug_asserts_limited, r=varkor
Browse files Browse the repository at this point in the history
Consider changing assert! to debug_assert! when it calls visit_with

The perf run from rust-lang#52956 revealed that there were 3 benchmarks that benefited most from changing `assert!`s to `debug_assert!`s:

- issue rust-lang#46449: avg -4.7% for -check
- deeply-nested (AKA rust-lang#38528): avg -3.4% for -check
- regression rust-lang#31157: avg -3.2% for -check

I analyzed their fixing PRs and decided to look for potentially heavy assertions in the files they modified. I noticed that all of the non-trivial ones contained indirect calls to `visit_with()`.

It might be a good idea to consider changing `assert!` to `debug_assert!` in those places in order to get the performance wins shown by the benchmarks.
  • Loading branch information
GuillaumeGomez authored Aug 12, 2018
2 parents 7d38649 + b187c42 commit fbe6241
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
debug!("normalize_projection_type(projection_ty={:?})",
projection_ty);

assert!(!projection_ty.has_escaping_regions());
debug_assert!(!projection_ty.has_escaping_regions());

// FIXME(#20304) -- cache

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1142,7 +1142,7 @@ fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>(
if !is_default {
true
} else if obligation.param_env.reveal == Reveal::All {
assert!(!poly_trait_ref.needs_infer());
debug_assert!(!poly_trait_ref.needs_infer());
if !poly_trait_ref.needs_subst() {
true
} else {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
pub fn select(&mut self, obligation: &TraitObligation<'tcx>)
-> SelectionResult<'tcx, Selection<'tcx>> {
debug!("select({:?})", obligation);
assert!(!obligation.predicate.has_escaping_regions());
debug_assert!(!obligation.predicate.has_escaping_regions());

let stack = self.push_stack(TraitObligationStackList::empty(), obligation);

Expand Down Expand Up @@ -662,7 +662,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {

match obligation.predicate {
ty::Predicate::Trait(ref t) => {
assert!(!t.has_escaping_regions());
debug_assert!(!t.has_escaping_regions());
let obligation = obligation.with(t.clone());
self.evaluate_trait_predicate_recursively(previous_stack, obligation)
}
Expand Down Expand Up @@ -1076,7 +1076,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
debug!("candidate_from_obligation(cache_fresh_trait_pred={:?}, obligation={:?})",
cache_fresh_trait_pred,
stack);
assert!(!stack.obligation.predicate.has_escaping_regions());
debug_assert!(!stack.obligation.predicate.has_escaping_regions());

if let Some(c) = self.check_candidate_cache(stack.obligation.param_env,
&cache_fresh_trait_pred) {
Expand Down Expand Up @@ -1586,7 +1586,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
snapshot: &infer::CombinedSnapshot<'cx, 'tcx>)
-> bool
{
assert!(!skol_trait_ref.has_escaping_regions());
debug_assert!(!skol_trait_ref.has_escaping_regions());
if self.infcx.at(&obligation.cause, obligation.param_env)
.sup(ty::Binder::dummy(skol_trait_ref), trait_bound).is_err() {
return false;
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
let univariant = |fields: &[TyLayout], repr: &ReprOptions, kind| {
Ok(tcx.intern_layout(univariant_uninterned(fields, repr, kind)?))
};
assert!(!ty.has_infer_types());
debug_assert!(!ty.has_infer_types());

Ok(match ty.sty {
// Basic scalars.
Expand Down Expand Up @@ -1283,7 +1283,7 @@ impl<'a, 'tcx> SizeSkeleton<'tcx> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
param_env: ty::ParamEnv<'tcx>)
-> Result<SizeSkeleton<'tcx>, LayoutError<'tcx>> {
assert!(!ty.has_infer_types());
debug_assert!(!ty.has_infer_types());

// First try computing a static layout.
let err = match tcx.layout_of(param_env.and(ty)) {
Expand All @@ -1300,7 +1300,7 @@ impl<'a, 'tcx> SizeSkeleton<'tcx> {
let tail = tcx.struct_tail(pointee);
match tail.sty {
ty::TyParam(_) | ty::TyProjection(_) => {
assert!(tail.has_param_types() || tail.has_self_ty());
debug_assert!(tail.has_param_types() || tail.has_self_ty());
Ok(SizeSkeleton::Pointer {
non_zero,
tail: tcx.erase_regions(&tail)
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ impl<'a, 'gcx, 'tcx> ExistentialTraitRef<'tcx> {
pub fn with_self_ty(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, self_ty: Ty<'tcx>)
-> ty::TraitRef<'tcx> {
// otherwise the escaping regions would be captured by the binder
assert!(!self_ty.has_escaping_regions());
debug_assert!(!self_ty.has_escaping_regions());

ty::TraitRef {
def_id: self.def_id,
Expand Down Expand Up @@ -753,7 +753,7 @@ impl<T> Binder<T> {
pub fn dummy<'tcx>(value: T) -> Binder<T>
where T: TypeFoldable<'tcx>
{
assert!(!value.has_escaping_regions());
debug_assert!(!value.has_escaping_regions());
Binder(value)
}

Expand Down Expand Up @@ -1247,7 +1247,7 @@ impl<'a, 'tcx, 'gcx> ExistentialProjection<'tcx> {
-> ty::ProjectionPredicate<'tcx>
{
// otherwise the escaping regions would be captured by the binders
assert!(!self_ty.has_escaping_regions());
debug_assert!(!self_ty.has_escaping_regions());

ty::ProjectionPredicate {
projection_ty: ty::ProjectionTy {
Expand Down

0 comments on commit fbe6241

Please sign in to comment.