Skip to content

Commit

Permalink
Auto merge of rust-lang#131422 - GnomedDev:smallvec-predicate-obligat…
Browse files Browse the repository at this point in the history
…ions, r=<try>

WIP: Use `SmallVec` for `PredicateObligation` storage

I noticed while profiling clippy on a project that a large amount of time is being spent allocating `Vec`s for `PredicateObligation`, and the `Vec`s are often quite small. This is an attempt to optimise this by using SmallVec to avoid heap allocations for these common small Vecs.

First, I am making sure a single type alias is being used whenever referring to a Vec of PredicateObligation, then I can swap this type alias to `SmallVec` and test out different amounts of inline storage until I find a sweet spot.

This is WIP, now at the stage of checking if this is a performance win and tuning the inline capacity.
- Heavy (0-11%) performance drop with `4` inline capacity.

r? `@ghost`
  • Loading branch information
bors committed Oct 10, 2024
2 parents df1b5d3 + fda8da9 commit 90ae6a8
Show file tree
Hide file tree
Showing 38 changed files with 311 additions and 254 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use rustc_infer::infer::region_constraints::RegionConstraintData;
use rustc_infer::infer::{
BoundRegion, BoundRegionConversionTime, InferCtxt, NllRegionVariableOrigin,
};
use rustc_infer::traits::PredicateObligations;
use rustc_middle::mir::tcx::PlaceTy;
use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
Expand All @@ -40,7 +41,6 @@ use rustc_span::source_map::Spanned;
use rustc_span::symbol::sym;
use rustc_span::{DUMMY_SP, Span};
use rustc_target::abi::{FIRST_VARIANT, FieldIdx};
use rustc_trait_selection::traits::PredicateObligation;
use rustc_trait_selection::traits::query::type_op::custom::{
CustomTypeOp, scrape_region_constraints,
};
Expand Down Expand Up @@ -2940,7 +2940,7 @@ impl NormalizeLocation for Location {
pub(super) struct InstantiateOpaqueType<'tcx> {
pub base_universe: Option<ty::UniverseIndex>,
pub region_constraints: Option<RegionConstraintData<'tcx>>,
pub obligations: Vec<PredicateObligation<'tcx>>,
pub obligations: PredicateObligations<'tcx>,
}

impl<'tcx> TypeOp<'tcx> for InstantiateOpaqueType<'tcx> {
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_data_structures/src/obligation_forest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ use std::fmt::Debug;
use std::hash;
use std::marker::PhantomData;

use smallvec::SmallVec;
use tracing::debug;

use crate::fx::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -141,7 +142,7 @@ pub trait ObligationProcessor {
#[derive(Debug)]
pub enum ProcessResult<O, E> {
Unchanged,
Changed(Vec<O>),
Changed(SmallVec<[O; 4]>),
Error(E),
}

Expand Down Expand Up @@ -402,9 +403,10 @@ impl<O: ForestObligation> ObligationForest<O> {
}

/// Returns the set of obligations that are in a pending state.
pub fn map_pending_obligations<P, F>(&self, f: F) -> Vec<P>
pub fn map_pending_obligations<P, F, R>(&self, f: F) -> R
where
F: Fn(&O) -> P,
R: FromIterator<P>,
{
self.nodes
.iter()
Expand Down
68 changes: 36 additions & 32 deletions compiler/rustc_data_structures/src/obligation_forest/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use std::fmt;

use smallvec::smallvec;

use super::*;

impl<'a> super::ForestObligation for &'a str {
Expand Down Expand Up @@ -101,9 +103,9 @@ fn push_pop() {
// |-> A.3
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
"A" => ProcessResult::Changed(smallvec!["A.1", "A.2", "A.3"]),
"B" => ProcessResult::Error("B is for broken"),
"C" => ProcessResult::Changed(vec![]),
"C" => ProcessResult::Changed(smallvec![]),
"A.1" | "A.2" | "A.3" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -123,8 +125,8 @@ fn push_pop() {
|obligation| match *obligation {
"A.1" => ProcessResult::Unchanged,
"A.2" => ProcessResult::Unchanged,
"A.3" => ProcessResult::Changed(vec!["A.3.i"]),
"D" => ProcessResult::Changed(vec!["D.1", "D.2"]),
"A.3" => ProcessResult::Changed(smallvec!["A.3.i"]),
"D" => ProcessResult::Changed(smallvec!["D.1", "D.2"]),
"A.3.i" | "D.1" | "D.2" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -139,11 +141,11 @@ fn push_pop() {
// |-> D.2 |-> D.2.i
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A.1" => ProcessResult::Changed(vec![]),
"A.1" => ProcessResult::Changed(smallvec![]),
"A.2" => ProcessResult::Error("A is for apple"),
"A.3.i" => ProcessResult::Changed(vec![]),
"D.1" => ProcessResult::Changed(vec!["D.1.i"]),
"D.2" => ProcessResult::Changed(vec!["D.2.i"]),
"A.3.i" => ProcessResult::Changed(smallvec![]),
"D.1" => ProcessResult::Changed(smallvec!["D.1.i"]),
"D.2" => ProcessResult::Changed(smallvec!["D.2.i"]),
"D.1.i" | "D.2.i" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -158,7 +160,7 @@ fn push_pop() {
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"D.1.i" => ProcessResult::Error("D is for dumb"),
"D.2.i" => ProcessResult::Changed(vec![]),
"D.2.i" => ProcessResult::Changed(smallvec![]),
_ => panic!("unexpected obligation {:?}", obligation),
},
|_| {},
Expand All @@ -184,10 +186,10 @@ fn success_in_grandchildren() {

let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
"A.1" => ProcessResult::Changed(vec![]),
"A.2" => ProcessResult::Changed(vec!["A.2.i", "A.2.ii"]),
"A.3" => ProcessResult::Changed(vec![]),
"A" => ProcessResult::Changed(smallvec!["A.1", "A.2", "A.3"]),
"A.1" => ProcessResult::Changed(smallvec![]),
"A.2" => ProcessResult::Changed(smallvec!["A.2.i", "A.2.ii"]),
"A.3" => ProcessResult::Changed(smallvec![]),
"A.2.i" | "A.2.ii" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -201,7 +203,7 @@ fn success_in_grandchildren() {
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A.2.i" => ProcessResult::Unchanged,
"A.2.ii" => ProcessResult::Changed(vec![]),
"A.2.ii" => ProcessResult::Changed(smallvec![]),
_ => unreachable!(),
},
|_| {},
Expand All @@ -211,7 +213,7 @@ fn success_in_grandchildren() {

let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A.2.i" => ProcessResult::Changed(vec!["A.2.i.a"]),
"A.2.i" => ProcessResult::Changed(smallvec!["A.2.i.a"]),
"A.2.i.a" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -222,7 +224,7 @@ fn success_in_grandchildren() {

let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A.2.i.a" => ProcessResult::Changed(vec![]),
"A.2.i.a" => ProcessResult::Changed(smallvec![]),
_ => unreachable!(),
},
|_| {},
Expand All @@ -247,7 +249,7 @@ fn to_errors_no_throw() {
forest.register_obligation("A");
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
"A" => ProcessResult::Changed(smallvec!["A.1", "A.2", "A.3"]),
"A.1" | "A.2" | "A.3" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -269,7 +271,7 @@ fn diamond() {
forest.register_obligation("A");
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A" => ProcessResult::Changed(vec!["A.1", "A.2"]),
"A" => ProcessResult::Changed(smallvec!["A.1", "A.2"]),
"A.1" | "A.2" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -280,8 +282,8 @@ fn diamond() {

let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A.1" => ProcessResult::Changed(vec!["D"]),
"A.2" => ProcessResult::Changed(vec!["D"]),
"A.1" => ProcessResult::Changed(smallvec!["D"]),
"A.2" => ProcessResult::Changed(smallvec!["D"]),
"D" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -295,7 +297,7 @@ fn diamond() {
|obligation| match *obligation {
"D" => {
d_count += 1;
ProcessResult::Changed(vec![])
ProcessResult::Changed(smallvec![])
}
_ => unreachable!(),
},
Expand All @@ -313,7 +315,7 @@ fn diamond() {
forest.register_obligation("A'");
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A'" => ProcessResult::Changed(vec!["A'.1", "A'.2"]),
"A'" => ProcessResult::Changed(smallvec!["A'.1", "A'.2"]),
"A'.1" | "A'.2" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -324,8 +326,8 @@ fn diamond() {

let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A'.1" => ProcessResult::Changed(vec!["D'", "A'"]),
"A'.2" => ProcessResult::Changed(vec!["D'"]),
"A'.1" => ProcessResult::Changed(smallvec!["D'", "A'"]),
"A'.2" => ProcessResult::Changed(smallvec!["D'"]),
"D'" | "A'" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand Down Expand Up @@ -366,7 +368,7 @@ fn done_dependency() {

let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A: Sized" | "B: Sized" | "C: Sized" => ProcessResult::Changed(vec![]),
"A: Sized" | "B: Sized" | "C: Sized" => ProcessResult::Changed(smallvec![]),
_ => unreachable!(),
},
|_| {},
Expand All @@ -379,7 +381,9 @@ fn done_dependency() {
forest.register_obligation("(A,B,C): Sized");
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"(A,B,C): Sized" => ProcessResult::Changed(vec!["A: Sized", "B: Sized", "C: Sized"]),
"(A,B,C): Sized" => {
ProcessResult::Changed(smallvec!["A: Sized", "B: Sized", "C: Sized"])
}
_ => unreachable!(),
},
|_| {},
Expand All @@ -399,10 +403,10 @@ fn orphan() {

let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A" => ProcessResult::Changed(vec!["D", "E"]),
"A" => ProcessResult::Changed(smallvec!["D", "E"]),
"B" => ProcessResult::Unchanged,
"C1" => ProcessResult::Changed(vec![]),
"C2" => ProcessResult::Changed(vec![]),
"C1" => ProcessResult::Changed(smallvec![]),
"C2" => ProcessResult::Changed(smallvec![]),
"D" | "E" => ProcessResult::Unchanged,
_ => unreachable!(),
},
Expand All @@ -416,7 +420,7 @@ fn orphan() {
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"D" | "E" => ProcessResult::Unchanged,
"B" => ProcessResult::Changed(vec!["D"]),
"B" => ProcessResult::Changed(smallvec!["D"]),
_ => unreachable!(),
},
|_| {},
Expand Down Expand Up @@ -459,7 +463,7 @@ fn simultaneous_register_and_error() {
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A" => ProcessResult::Error("An error"),
"B" => ProcessResult::Changed(vec!["A"]),
"B" => ProcessResult::Changed(smallvec!["A"]),
_ => unreachable!(),
},
|_| {},
Expand All @@ -474,7 +478,7 @@ fn simultaneous_register_and_error() {
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
|obligation| match *obligation {
"A" => ProcessResult::Error("An error"),
"B" => ProcessResult::Changed(vec!["A"]),
"B" => ProcessResult::Changed(smallvec!["A"]),
_ => unreachable!(),
},
|_| {},
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_hir_analysis/src/autoderef.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use rustc_infer::infer::InferCtxt;
use rustc_infer::traits::PredicateObligations;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
use rustc_session::Limit;
use rustc_span::Span;
Expand All @@ -23,7 +24,7 @@ struct AutoderefSnapshot<'tcx> {
reached_recursion_limit: bool,
steps: Vec<(Ty<'tcx>, AutoderefKind)>,
cur_ty: Ty<'tcx>,
obligations: Vec<traits::PredicateObligation<'tcx>>,
obligations: PredicateObligations<'tcx>,
}

pub struct Autoderef<'a, 'tcx> {
Expand Down Expand Up @@ -119,7 +120,7 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
state: AutoderefSnapshot {
steps: vec![],
cur_ty: infcx.resolve_vars_if_possible(base_ty),
obligations: vec![],
obligations: PredicateObligations::new(),
at_start: true,
reached_recursion_limit: false,
},
Expand Down Expand Up @@ -165,7 +166,7 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
pub fn structurally_normalize(
&self,
ty: Ty<'tcx>,
) -> Option<(Ty<'tcx>, Vec<traits::PredicateObligation<'tcx>>)> {
) -> Option<(Ty<'tcx>, PredicateObligations<'tcx>)> {
let ocx = ObligationCtxt::new(self.infcx);
let Ok(normalized_ty) = ocx.structurally_normalize(
&traits::ObligationCause::misc(self.span, self.body_id),
Expand Down Expand Up @@ -204,11 +205,11 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
self.state.steps.len()
}

pub fn into_obligations(self) -> Vec<traits::PredicateObligation<'tcx>> {
pub fn into_obligations(self) -> PredicateObligations<'tcx> {
self.state.obligations
}

pub fn current_obligations(&self) -> Vec<traits::PredicateObligation<'tcx>> {
pub fn current_obligations(&self) -> PredicateObligations<'tcx> {
self.state.obligations.clone()
}

Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_hir_typeck/src/autoderef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::iter;
use itertools::Itertools;
use rustc_hir_analysis::autoderef::{Autoderef, AutoderefKind};
use rustc_infer::infer::InferOk;
use rustc_infer::traits::PredicateObligations;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, OverloadedDeref};
use rustc_middle::ty::{self, Ty};
use rustc_span::Span;
Expand Down Expand Up @@ -36,10 +37,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> InferOk<'tcx, Vec<Adjustment<'tcx>>> {
let steps = autoderef.steps();
if steps.is_empty() {
return InferOk { obligations: vec![], value: vec![] };
return InferOk { obligations: PredicateObligations::new(), value: vec![] };
}

let mut obligations = vec![];
let mut obligations = PredicateObligations::new();
let targets =
steps.iter().skip(1).map(|&(ty, _)| ty).chain(iter::once(autoderef.final_ty(false)));
let steps: Vec<_> = steps
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_typeck/src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_hir as hir;
use rustc_hir::lang_items::LangItem;
use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
use rustc_infer::infer::{BoundRegionConversionTime, DefineOpaqueTypes, InferOk, InferResult};
use rustc_infer::traits::ObligationCauseCode;
use rustc_infer::traits::{ObligationCauseCode, PredicateObligations};
use rustc_macros::{TypeFoldable, TypeVisitable};
use rustc_middle::span_bug;
use rustc_middle::ty::visit::{TypeVisitable, TypeVisitableExt};
Expand Down Expand Up @@ -805,7 +805,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// [c1]: https://github.com/rust-lang/rust/pull/45072#issuecomment-341089706
// [c2]: https://github.com/rust-lang/rust/pull/45072#issuecomment-341096796
self.commit_if_ok(|_| {
let mut all_obligations = vec![];
let mut all_obligations = PredicateObligations::new();
let supplied_sig = self.instantiate_binder_with_fresh_vars(
self.tcx.def_span(expr_def_id),
BoundRegionConversionTime::FnCall,
Expand Down
Loading

0 comments on commit 90ae6a8

Please sign in to comment.