From 36b37e22de92b584b9cf4464ed1d4ad317b798be Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 9 Sep 2019 08:25:54 +1000 Subject: [PATCH 1/4] Remove two unnecessary `clone()` calls. --- src/librustc_typeck/check/closure.rs | 6 +++--- src/librustc_typeck/check/compare_method.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustc_typeck/check/closure.rs b/src/librustc_typeck/check/closure.rs index e9370429f3f55..d626bff150020 100644 --- a/src/librustc_typeck/check/closure.rs +++ b/src/librustc_typeck/check/closure.rs @@ -529,11 +529,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); // recreated from (*) above // Check that E' = S'. - let cause = &self.misc(hir_ty.span); + let cause = self.misc(hir_ty.span); let InferOk { value: (), obligations, - } = self.at(cause, self.param_env) + } = self.at(&cause, self.param_env) .eq(*expected_ty, supplied_ty)?; all_obligations.extend(obligations); @@ -549,7 +549,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ); all_obligations.push( Obligation::new( - cause.clone(), + cause, self.param_env, ty::Predicate::TypeOutlives( ty::Binder::dummy( diff --git a/src/librustc_typeck/check/compare_method.rs b/src/librustc_typeck/check/compare_method.rs index 8e187b7e05b51..74ea93fc44270 100644 --- a/src/librustc_typeck/check/compare_method.rs +++ b/src/librustc_typeck/check/compare_method.rs @@ -308,7 +308,7 @@ fn compare_predicate_entailment<'tcx>( let cause = ObligationCause { span: impl_err_span, - ..cause.clone() + ..cause }; let mut diag = struct_span_err!(tcx.sess, From c1b9a46f60440e2207f458e276b38239b1651a22 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 9 Sep 2019 14:06:14 +1000 Subject: [PATCH 2/4] Add some assertions on obligation type sizes. These are types that get memcpy'd a lot. --- src/librustc/traits/fulfill.rs | 4 ++++ src/librustc/traits/mod.rs | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index c1de4939c1d91..a59ea19896364 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -68,6 +68,10 @@ pub struct PendingPredicateObligation<'tcx> { pub stalled_on: Vec>, } +// `PendingPredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. +#[cfg(target_arch = "x86_64")] +static_assert_size!(PendingPredicateObligation<'_>, 160); + impl<'a, 'tcx> FulfillmentContext<'tcx> { /// Creates a new fulfillment context. pub fn new() -> FulfillmentContext<'tcx> { diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 1ca92d79fa5f6..f43fbbfee3e91 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -123,6 +123,10 @@ pub struct Obligation<'tcx, T> { pub type PredicateObligation<'tcx> = Obligation<'tcx, ty::Predicate<'tcx>>; pub type TraitObligation<'tcx> = Obligation<'tcx, ty::PolyTraitPredicate<'tcx>>; +// `PredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. +#[cfg(target_arch = "x86_64")] +static_assert_size!(PredicateObligation<'_>, 136); + /// The reason why we incurred this obligation; used for error reporting. #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct ObligationCause<'tcx> { @@ -269,6 +273,10 @@ pub enum ObligationCauseCode<'tcx> { TrivialBound, } +// `ObligationCauseCode` is used a lot. Make sure it doesn't unintentionally get bigger. +#[cfg(target_arch = "x86_64")] +static_assert_size!(ObligationCauseCode<'_>, 56); + #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct DerivedObligationCause<'tcx> { /// The trait reference of the parent obligation that led to the From b972ac818c98373b6d045956b049dc34932c41be Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 6 Sep 2019 16:55:19 +1000 Subject: [PATCH 3/4] Shrink `ObligationCauseCode` by boxing `MatchExpressionArm`. The reduction in `memcpy` calls greatly outweighs the cost of the extra allocations, for a net performance win. --- src/librustc/infer/error_reporting/mod.rs | 24 ++++++++++++----------- src/librustc/traits/fulfill.rs | 2 +- src/librustc/traits/mod.rs | 24 +++++++++++++---------- src/librustc/traits/structural_impls.rs | 8 ++++---- src/librustc_typeck/check/_match.rs | 18 +++++++++-------- 5 files changed, 42 insertions(+), 34 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 5883be6e26883..684e799b40319 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -55,7 +55,7 @@ use crate::hir::def_id::DefId; use crate::hir::Node; use crate::infer::opaque_types; use crate::middle::region; -use crate::traits::{ObligationCause, ObligationCauseCode}; +use crate::traits::{MatchExpressionArmCause, ObligationCause, ObligationCauseCode}; use crate::ty::error::TypeError; use crate::ty::{self, subst::{Subst, SubstsRef}, Region, Ty, TyCtxt, TypeFoldable}; use errors::{Applicability, DiagnosticBuilder, DiagnosticStyledString}; @@ -624,13 +624,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } } } - ObligationCauseCode::MatchExpressionArm { + ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { source, ref prior_arms, last_ty, discrim_hir_id, .. - } => match source { + }) => match source { hir::MatchSource::IfLetDesugar { .. } => { let msg = "`if let` arms have incompatible types"; err.span_label(cause.span, msg); @@ -1622,13 +1622,15 @@ impl<'tcx> ObligationCause<'tcx> { use crate::traits::ObligationCauseCode::*; match self.code { CompareImplMethodObligation { .. } => Error0308("method not compatible with trait"), - MatchExpressionArm { source, .. } => Error0308(match source { - hir::MatchSource::IfLetDesugar { .. } => "`if let` arms have incompatible types", - hir::MatchSource::TryDesugar => { - "try expression alternatives have incompatible types" - } - _ => "match arms have incompatible types", - }), + MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => + Error0308(match source { + hir::MatchSource::IfLetDesugar { .. } => + "`if let` arms have incompatible types", + hir::MatchSource::TryDesugar => { + "try expression alternatives have incompatible types" + } + _ => "match arms have incompatible types", + }), IfExpression { .. } => Error0308("if and else have incompatible types"), IfExpressionWithNoElse => Error0317("if may be missing an else clause"), MainFunctionType => Error0580("main function has wrong type"), @@ -1656,7 +1658,7 @@ impl<'tcx> ObligationCause<'tcx> { match self.code { CompareImplMethodObligation { .. } => "method type is compatible with trait", ExprAssignable => "expression is assignable", - MatchExpressionArm { source, .. } => match source { + MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => match source { hir::MatchSource::IfLetDesugar { .. } => "`if let` arms have compatible types", _ => "match arms have compatible types", }, diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index a59ea19896364..0b8de1f13d384 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -70,7 +70,7 @@ pub struct PendingPredicateObligation<'tcx> { // `PendingPredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] -static_assert_size!(PendingPredicateObligation<'_>, 160); +static_assert_size!(PendingPredicateObligation<'_>, 144); impl<'a, 'tcx> FulfillmentContext<'tcx> { /// Creates a new fulfillment context. diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index f43fbbfee3e91..548fb55098af1 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -125,7 +125,7 @@ pub type TraitObligation<'tcx> = Obligation<'tcx, ty::PolyTraitPredicate<'tcx>>; // `PredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] -static_assert_size!(PredicateObligation<'_>, 136); +static_assert_size!(PredicateObligation<'_>, 120); /// The reason why we incurred this obligation; used for error reporting. #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -151,7 +151,8 @@ impl<'tcx> ObligationCause<'tcx> { ObligationCauseCode::StartFunctionType => { tcx.sess.source_map().def_span(self.span) } - ObligationCauseCode::MatchExpressionArm { arm_span, .. } => arm_span, + ObligationCauseCode::MatchExpressionArm( + box MatchExpressionArmCause { arm_span, .. }) => arm_span, _ => self.span, } } @@ -227,13 +228,7 @@ pub enum ObligationCauseCode<'tcx> { ExprAssignable, /// Computing common supertype in the arms of a match expression - MatchExpressionArm { - arm_span: Span, - source: hir::MatchSource, - prior_arms: Vec, - last_ty: Ty<'tcx>, - discrim_hir_id: hir::HirId, - }, + MatchExpressionArm(Box>), /// Computing common supertype in the pattern guard for the arms of a match expression MatchExpressionArmPattern { span: Span, ty: Ty<'tcx> }, @@ -275,7 +270,16 @@ pub enum ObligationCauseCode<'tcx> { // `ObligationCauseCode` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] -static_assert_size!(ObligationCauseCode<'_>, 56); +static_assert_size!(ObligationCauseCode<'_>, 40); + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct MatchExpressionArmCause<'tcx> { + pub arm_span: Span, + pub source: hir::MatchSource, + pub prior_arms: Vec, + pub last_ty: Ty<'tcx>, + pub discrim_hir_id: hir::HirId, +} #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct DerivedObligationCause<'tcx> { diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index 05b698eb4c4ea..56324baffa038 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -508,21 +508,21 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> { trait_item_def_id, }), super::ExprAssignable => Some(super::ExprAssignable), - super::MatchExpressionArm { + super::MatchExpressionArm(box super::MatchExpressionArmCause { arm_span, source, ref prior_arms, last_ty, discrim_hir_id, - } => { + }) => { tcx.lift(&last_ty).map(|last_ty| { - super::MatchExpressionArm { + super::MatchExpressionArm(box super::MatchExpressionArmCause { arm_span, source, prior_arms: prior_arms.clone(), last_ty, discrim_hir_id, - } + }) }) } super::MatchExpressionArmPattern { span, ty } => { diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 7427ae9ce8de3..8ce08f523a1b0 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -2,7 +2,7 @@ use crate::check::{FnCtxt, Expectation, Diverges, Needs}; use crate::check::coercion::CoerceMany; use rustc::hir::{self, ExprKind}; use rustc::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; -use rustc::traits::{ObligationCause, ObligationCauseCode}; +use rustc::traits::{MatchExpressionArmCause, ObligationCause, ObligationCauseCode}; use rustc::ty::Ty; use syntax_pos::Span; @@ -146,13 +146,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // The reason for the first arm to fail is not that the match arms diverge, // but rather that there's a prior obligation that doesn't hold. 0 => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)), - _ => (expr.span, ObligationCauseCode::MatchExpressionArm { - arm_span, - source: match_src, - prior_arms: other_arms.clone(), - last_ty: prior_arm_ty.unwrap(), - discrim_hir_id: discrim.hir_id, - }), + _ => (expr.span, + ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { + arm_span, + source: match_src, + prior_arms: other_arms.clone(), + last_ty: prior_arm_ty.unwrap(), + discrim_hir_id: discrim.hir_id, + }) + ), }; let cause = self.cause(span, code); coercion.coerce(self, &cause, &arm.body, arm_ty); From 2e3b079836823446eb014c69866d6a0f8cca4ef2 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 9 Sep 2019 12:40:54 +1000 Subject: [PATCH 4/4] Shrink `ObligationCauseCode` by boxing `IfExpression`. The reduction in `memcpy` calls outweighs the cost of the extra allocations, for a net performance win. --- src/librustc/infer/error_reporting/mod.rs | 5 +++-- src/librustc/traits/fulfill.rs | 2 +- src/librustc/traits/mod.rs | 17 ++++++++++------- src/librustc/traits/structural_impls.rs | 12 +++++++----- src/librustc_typeck/check/_match.rs | 7 ++++--- 5 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 684e799b40319..ab24b3f2f059f 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -55,7 +55,8 @@ use crate::hir::def_id::DefId; use crate::hir::Node; use crate::infer::opaque_types; use crate::middle::region; -use crate::traits::{MatchExpressionArmCause, ObligationCause, ObligationCauseCode}; +use crate::traits::{IfExpressionCause, MatchExpressionArmCause, ObligationCause}; +use crate::traits::{ObligationCauseCode}; use crate::ty::error::TypeError; use crate::ty::{self, subst::{Subst, SubstsRef}, Region, Ty, TyCtxt, TypeFoldable}; use errors::{Applicability, DiagnosticBuilder, DiagnosticStyledString}; @@ -681,7 +682,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { } } }, - ObligationCauseCode::IfExpression { then, outer, semicolon } => { + ObligationCauseCode::IfExpression(box IfExpressionCause { then, outer, semicolon }) => { err.span_label(then, "expected because of this"); outer.map(|sp| err.span_label(sp, "if and else have incompatible types")); if let Some(sp) = semicolon { diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index 0b8de1f13d384..4494c034d51e2 100644 --- a/src/librustc/traits/fulfill.rs +++ b/src/librustc/traits/fulfill.rs @@ -70,7 +70,7 @@ pub struct PendingPredicateObligation<'tcx> { // `PendingPredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] -static_assert_size!(PendingPredicateObligation<'_>, 144); +static_assert_size!(PendingPredicateObligation<'_>, 136); impl<'a, 'tcx> FulfillmentContext<'tcx> { /// Creates a new fulfillment context. diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 548fb55098af1..d2683090add40 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -125,7 +125,7 @@ pub type TraitObligation<'tcx> = Obligation<'tcx, ty::PolyTraitPredicate<'tcx>>; // `PredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] -static_assert_size!(PredicateObligation<'_>, 120); +static_assert_size!(PredicateObligation<'_>, 112); /// The reason why we incurred this obligation; used for error reporting. #[derive(Clone, Debug, PartialEq, Eq, Hash)] @@ -234,11 +234,7 @@ pub enum ObligationCauseCode<'tcx> { MatchExpressionArmPattern { span: Span, ty: Ty<'tcx> }, /// Computing common supertype in an if expression - IfExpression { - then: Span, - outer: Option, - semicolon: Option, - }, + IfExpression(Box), /// Computing common supertype of an if expression with no else counter-part IfExpressionWithNoElse, @@ -270,7 +266,7 @@ pub enum ObligationCauseCode<'tcx> { // `ObligationCauseCode` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] -static_assert_size!(ObligationCauseCode<'_>, 40); +static_assert_size!(ObligationCauseCode<'_>, 32); #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct MatchExpressionArmCause<'tcx> { @@ -281,6 +277,13 @@ pub struct MatchExpressionArmCause<'tcx> { pub discrim_hir_id: hir::HirId, } +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct IfExpressionCause { + pub then: Span, + pub outer: Option, + pub semicolon: Option, +} + #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct DerivedObligationCause<'tcx> { /// The trait reference of the parent obligation that led to the diff --git a/src/librustc/traits/structural_impls.rs b/src/librustc/traits/structural_impls.rs index 56324baffa038..6930c9368282b 100644 --- a/src/librustc/traits/structural_impls.rs +++ b/src/librustc/traits/structural_impls.rs @@ -528,11 +528,13 @@ impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCauseCode<'a> { super::MatchExpressionArmPattern { span, ty } => { tcx.lift(&ty).map(|ty| super::MatchExpressionArmPattern { span, ty }) } - super::IfExpression { then, outer, semicolon } => Some(super::IfExpression { - then, - outer, - semicolon, - }), + super::IfExpression(box super::IfExpressionCause { then, outer, semicolon }) => { + Some(super::IfExpression(box super::IfExpressionCause { + then, + outer, + semicolon, + })) + } super::IfExpressionWithNoElse => Some(super::IfExpressionWithNoElse), super::MainFunctionType => Some(super::MainFunctionType), super::StartFunctionType => Some(super::StartFunctionType), diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 8ce08f523a1b0..308a3d8ebc2cf 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -2,7 +2,8 @@ use crate::check::{FnCtxt, Expectation, Diverges, Needs}; use crate::check::coercion::CoerceMany; use rustc::hir::{self, ExprKind}; use rustc::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; -use rustc::traits::{MatchExpressionArmCause, ObligationCause, ObligationCauseCode}; +use rustc::traits::{IfExpressionCause, MatchExpressionArmCause, ObligationCause}; +use rustc::traits::{ObligationCauseCode}; use rustc::ty::Ty; use syntax_pos::Span; @@ -347,11 +348,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; // Finally construct the cause: - self.cause(error_sp, ObligationCauseCode::IfExpression { + self.cause(error_sp, ObligationCauseCode::IfExpression(box IfExpressionCause { then: then_sp, outer: outer_sp, semicolon: remove_semicolon, - }) + })) } fn demand_discriminant_type(