Skip to content

Commit 90ae6a8

Browse files
borsbors
committed
Auto merge of #131422 - GnomedDev:smallvec-predicate-obligations, 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`
2 parents df1b5d3 + fda8da9 commit 90ae6a8

File tree

38 files changed

+311
-254
lines changed

38 files changed

+311
-254
lines changed

Diff for: ‎compiler/rustc_borrowck/src/type_check/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use rustc_infer::infer::region_constraints::RegionConstraintData;
1818
use rustc_infer::infer::{
1919
BoundRegion, BoundRegionConversionTime, InferCtxt, NllRegionVariableOrigin,
2020
};
21+
use rustc_infer::traits::PredicateObligations;
2122
use rustc_middle::mir::tcx::PlaceTy;
2223
use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor};
2324
use rustc_middle::mir::*;
@@ -40,7 +41,6 @@ use rustc_span::source_map::Spanned;
4041
use rustc_span::symbol::sym;
4142
use rustc_span::{DUMMY_SP, Span};
4243
use rustc_target::abi::{FIRST_VARIANT, FieldIdx};
43-
use rustc_trait_selection::traits::PredicateObligation;
4444
use rustc_trait_selection::traits::query::type_op::custom::{
4545
CustomTypeOp, scrape_region_constraints,
4646
};
@@ -2940,7 +2940,7 @@ impl NormalizeLocation for Location {
29402940
pub(super) struct InstantiateOpaqueType<'tcx> {
29412941
pub base_universe: Option<ty::UniverseIndex>,
29422942
pub region_constraints: Option<RegionConstraintData<'tcx>>,
2943-
pub obligations: Vec<PredicateObligation<'tcx>>,
2943+
pub obligations: PredicateObligations<'tcx>,
29442944
}
29452945

29462946
impl<'tcx> TypeOp<'tcx> for InstantiateOpaqueType<'tcx> {

Diff for: ‎compiler/rustc_data_structures/src/obligation_forest/mod.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ use std::fmt::Debug;
7575
use std::hash;
7676
use std::marker::PhantomData;
7777

78+
use smallvec::SmallVec;
7879
use tracing::debug;
7980

8081
use crate::fx::{FxHashMap, FxHashSet};
@@ -141,7 +142,7 @@ pub trait ObligationProcessor {
141142
#[derive(Debug)]
142143
pub enum ProcessResult<O, E> {
143144
Unchanged,
144-
Changed(Vec<O>),
145+
Changed(SmallVec<[O; 4]>),
145146
Error(E),
146147
}
147148

@@ -402,9 +403,10 @@ impl<O: ForestObligation> ObligationForest<O> {
402403
}
403404

404405
/// Returns the set of obligations that are in a pending state.
405-
pub fn map_pending_obligations<P, F>(&self, f: F) -> Vec<P>
406+
pub fn map_pending_obligations<P, F, R>(&self, f: F) -> R
406407
where
407408
F: Fn(&O) -> P,
409+
R: FromIterator<P>,
408410
{
409411
self.nodes
410412
.iter()

Diff for: ‎compiler/rustc_data_structures/src/obligation_forest/tests.rs

+36-32
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use std::fmt;
22

3+
use smallvec::smallvec;
4+
35
use super::*;
46

57
impl<'a> super::ForestObligation for &'a str {
@@ -101,9 +103,9 @@ fn push_pop() {
101103
// |-> A.3
102104
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
103105
|obligation| match *obligation {
104-
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
106+
"A" => ProcessResult::Changed(smallvec!["A.1", "A.2", "A.3"]),
105107
"B" => ProcessResult::Error("B is for broken"),
106-
"C" => ProcessResult::Changed(vec![]),
108+
"C" => ProcessResult::Changed(smallvec![]),
107109
"A.1" | "A.2" | "A.3" => ProcessResult::Unchanged,
108110
_ => unreachable!(),
109111
},
@@ -123,8 +125,8 @@ fn push_pop() {
123125
|obligation| match *obligation {
124126
"A.1" => ProcessResult::Unchanged,
125127
"A.2" => ProcessResult::Unchanged,
126-
"A.3" => ProcessResult::Changed(vec!["A.3.i"]),
127-
"D" => ProcessResult::Changed(vec!["D.1", "D.2"]),
128+
"A.3" => ProcessResult::Changed(smallvec!["A.3.i"]),
129+
"D" => ProcessResult::Changed(smallvec!["D.1", "D.2"]),
128130
"A.3.i" | "D.1" | "D.2" => ProcessResult::Unchanged,
129131
_ => unreachable!(),
130132
},
@@ -139,11 +141,11 @@ fn push_pop() {
139141
// |-> D.2 |-> D.2.i
140142
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
141143
|obligation| match *obligation {
142-
"A.1" => ProcessResult::Changed(vec![]),
144+
"A.1" => ProcessResult::Changed(smallvec![]),
143145
"A.2" => ProcessResult::Error("A is for apple"),
144-
"A.3.i" => ProcessResult::Changed(vec![]),
145-
"D.1" => ProcessResult::Changed(vec!["D.1.i"]),
146-
"D.2" => ProcessResult::Changed(vec!["D.2.i"]),
146+
"A.3.i" => ProcessResult::Changed(smallvec![]),
147+
"D.1" => ProcessResult::Changed(smallvec!["D.1.i"]),
148+
"D.2" => ProcessResult::Changed(smallvec!["D.2.i"]),
147149
"D.1.i" | "D.2.i" => ProcessResult::Unchanged,
148150
_ => unreachable!(),
149151
},
@@ -158,7 +160,7 @@ fn push_pop() {
158160
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
159161
|obligation| match *obligation {
160162
"D.1.i" => ProcessResult::Error("D is for dumb"),
161-
"D.2.i" => ProcessResult::Changed(vec![]),
163+
"D.2.i" => ProcessResult::Changed(smallvec![]),
162164
_ => panic!("unexpected obligation {:?}", obligation),
163165
},
164166
|_| {},
@@ -184,10 +186,10 @@ fn success_in_grandchildren() {
184186

185187
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
186188
|obligation| match *obligation {
187-
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
188-
"A.1" => ProcessResult::Changed(vec![]),
189-
"A.2" => ProcessResult::Changed(vec!["A.2.i", "A.2.ii"]),
190-
"A.3" => ProcessResult::Changed(vec![]),
189+
"A" => ProcessResult::Changed(smallvec!["A.1", "A.2", "A.3"]),
190+
"A.1" => ProcessResult::Changed(smallvec![]),
191+
"A.2" => ProcessResult::Changed(smallvec!["A.2.i", "A.2.ii"]),
192+
"A.3" => ProcessResult::Changed(smallvec![]),
191193
"A.2.i" | "A.2.ii" => ProcessResult::Unchanged,
192194
_ => unreachable!(),
193195
},
@@ -201,7 +203,7 @@ fn success_in_grandchildren() {
201203
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
202204
|obligation| match *obligation {
203205
"A.2.i" => ProcessResult::Unchanged,
204-
"A.2.ii" => ProcessResult::Changed(vec![]),
206+
"A.2.ii" => ProcessResult::Changed(smallvec![]),
205207
_ => unreachable!(),
206208
},
207209
|_| {},
@@ -211,7 +213,7 @@ fn success_in_grandchildren() {
211213

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

223225
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
224226
|obligation| match *obligation {
225-
"A.2.i.a" => ProcessResult::Changed(vec![]),
227+
"A.2.i.a" => ProcessResult::Changed(smallvec![]),
226228
_ => unreachable!(),
227229
},
228230
|_| {},
@@ -247,7 +249,7 @@ fn to_errors_no_throw() {
247249
forest.register_obligation("A");
248250
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
249251
|obligation| match *obligation {
250-
"A" => ProcessResult::Changed(vec!["A.1", "A.2", "A.3"]),
252+
"A" => ProcessResult::Changed(smallvec!["A.1", "A.2", "A.3"]),
251253
"A.1" | "A.2" | "A.3" => ProcessResult::Unchanged,
252254
_ => unreachable!(),
253255
},
@@ -269,7 +271,7 @@ fn diamond() {
269271
forest.register_obligation("A");
270272
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
271273
|obligation| match *obligation {
272-
"A" => ProcessResult::Changed(vec!["A.1", "A.2"]),
274+
"A" => ProcessResult::Changed(smallvec!["A.1", "A.2"]),
273275
"A.1" | "A.2" => ProcessResult::Unchanged,
274276
_ => unreachable!(),
275277
},
@@ -280,8 +282,8 @@ fn diamond() {
280282

281283
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
282284
|obligation| match *obligation {
283-
"A.1" => ProcessResult::Changed(vec!["D"]),
284-
"A.2" => ProcessResult::Changed(vec!["D"]),
285+
"A.1" => ProcessResult::Changed(smallvec!["D"]),
286+
"A.2" => ProcessResult::Changed(smallvec!["D"]),
285287
"D" => ProcessResult::Unchanged,
286288
_ => unreachable!(),
287289
},
@@ -295,7 +297,7 @@ fn diamond() {
295297
|obligation| match *obligation {
296298
"D" => {
297299
d_count += 1;
298-
ProcessResult::Changed(vec![])
300+
ProcessResult::Changed(smallvec![])
299301
}
300302
_ => unreachable!(),
301303
},
@@ -313,7 +315,7 @@ fn diamond() {
313315
forest.register_obligation("A'");
314316
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
315317
|obligation| match *obligation {
316-
"A'" => ProcessResult::Changed(vec!["A'.1", "A'.2"]),
318+
"A'" => ProcessResult::Changed(smallvec!["A'.1", "A'.2"]),
317319
"A'.1" | "A'.2" => ProcessResult::Unchanged,
318320
_ => unreachable!(),
319321
},
@@ -324,8 +326,8 @@ fn diamond() {
324326

325327
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
326328
|obligation| match *obligation {
327-
"A'.1" => ProcessResult::Changed(vec!["D'", "A'"]),
328-
"A'.2" => ProcessResult::Changed(vec!["D'"]),
329+
"A'.1" => ProcessResult::Changed(smallvec!["D'", "A'"]),
330+
"A'.2" => ProcessResult::Changed(smallvec!["D'"]),
329331
"D'" | "A'" => ProcessResult::Unchanged,
330332
_ => unreachable!(),
331333
},
@@ -366,7 +368,7 @@ fn done_dependency() {
366368

367369
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
368370
|obligation| match *obligation {
369-
"A: Sized" | "B: Sized" | "C: Sized" => ProcessResult::Changed(vec![]),
371+
"A: Sized" | "B: Sized" | "C: Sized" => ProcessResult::Changed(smallvec![]),
370372
_ => unreachable!(),
371373
},
372374
|_| {},
@@ -379,7 +381,9 @@ fn done_dependency() {
379381
forest.register_obligation("(A,B,C): Sized");
380382
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
381383
|obligation| match *obligation {
382-
"(A,B,C): Sized" => ProcessResult::Changed(vec!["A: Sized", "B: Sized", "C: Sized"]),
384+
"(A,B,C): Sized" => {
385+
ProcessResult::Changed(smallvec!["A: Sized", "B: Sized", "C: Sized"])
386+
}
383387
_ => unreachable!(),
384388
},
385389
|_| {},
@@ -399,10 +403,10 @@ fn orphan() {
399403

400404
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
401405
|obligation| match *obligation {
402-
"A" => ProcessResult::Changed(vec!["D", "E"]),
406+
"A" => ProcessResult::Changed(smallvec!["D", "E"]),
403407
"B" => ProcessResult::Unchanged,
404-
"C1" => ProcessResult::Changed(vec![]),
405-
"C2" => ProcessResult::Changed(vec![]),
408+
"C1" => ProcessResult::Changed(smallvec![]),
409+
"C2" => ProcessResult::Changed(smallvec![]),
406410
"D" | "E" => ProcessResult::Unchanged,
407411
_ => unreachable!(),
408412
},
@@ -416,7 +420,7 @@ fn orphan() {
416420
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
417421
|obligation| match *obligation {
418422
"D" | "E" => ProcessResult::Unchanged,
419-
"B" => ProcessResult::Changed(vec!["D"]),
423+
"B" => ProcessResult::Changed(smallvec!["D"]),
420424
_ => unreachable!(),
421425
},
422426
|_| {},
@@ -459,7 +463,7 @@ fn simultaneous_register_and_error() {
459463
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
460464
|obligation| match *obligation {
461465
"A" => ProcessResult::Error("An error"),
462-
"B" => ProcessResult::Changed(vec!["A"]),
466+
"B" => ProcessResult::Changed(smallvec!["A"]),
463467
_ => unreachable!(),
464468
},
465469
|_| {},
@@ -474,7 +478,7 @@ fn simultaneous_register_and_error() {
474478
let TestOutcome { completed: ok, errors: err, .. } = forest.process_obligations(&mut C(
475479
|obligation| match *obligation {
476480
"A" => ProcessResult::Error("An error"),
477-
"B" => ProcessResult::Changed(vec!["A"]),
481+
"B" => ProcessResult::Changed(smallvec!["A"]),
478482
_ => unreachable!(),
479483
},
480484
|_| {},

Diff for: ‎compiler/rustc_hir_analysis/src/autoderef.rs

+6-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use rustc_infer::infer::InferCtxt;
2+
use rustc_infer::traits::PredicateObligations;
23
use rustc_middle::ty::{self, Ty, TyCtxt, TypeVisitableExt};
34
use rustc_session::Limit;
45
use rustc_span::Span;
@@ -23,7 +24,7 @@ struct AutoderefSnapshot<'tcx> {
2324
reached_recursion_limit: bool,
2425
steps: Vec<(Ty<'tcx>, AutoderefKind)>,
2526
cur_ty: Ty<'tcx>,
26-
obligations: Vec<traits::PredicateObligation<'tcx>>,
27+
obligations: PredicateObligations<'tcx>,
2728
}
2829

2930
pub struct Autoderef<'a, 'tcx> {
@@ -119,7 +120,7 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
119120
state: AutoderefSnapshot {
120121
steps: vec![],
121122
cur_ty: infcx.resolve_vars_if_possible(base_ty),
122-
obligations: vec![],
123+
obligations: PredicateObligations::new(),
123124
at_start: true,
124125
reached_recursion_limit: false,
125126
},
@@ -165,7 +166,7 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
165166
pub fn structurally_normalize(
166167
&self,
167168
ty: Ty<'tcx>,
168-
) -> Option<(Ty<'tcx>, Vec<traits::PredicateObligation<'tcx>>)> {
169+
) -> Option<(Ty<'tcx>, PredicateObligations<'tcx>)> {
169170
let ocx = ObligationCtxt::new(self.infcx);
170171
let Ok(normalized_ty) = ocx.structurally_normalize(
171172
&traits::ObligationCause::misc(self.span, self.body_id),
@@ -204,11 +205,11 @@ impl<'a, 'tcx> Autoderef<'a, 'tcx> {
204205
self.state.steps.len()
205206
}
206207

207-
pub fn into_obligations(self) -> Vec<traits::PredicateObligation<'tcx>> {
208+
pub fn into_obligations(self) -> PredicateObligations<'tcx> {
208209
self.state.obligations
209210
}
210211

211-
pub fn current_obligations(&self) -> Vec<traits::PredicateObligation<'tcx>> {
212+
pub fn current_obligations(&self) -> PredicateObligations<'tcx> {
212213
self.state.obligations.clone()
213214
}
214215

Diff for: ‎compiler/rustc_hir_typeck/src/autoderef.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use std::iter;
55
use itertools::Itertools;
66
use rustc_hir_analysis::autoderef::{Autoderef, AutoderefKind};
77
use rustc_infer::infer::InferOk;
8+
use rustc_infer::traits::PredicateObligations;
89
use rustc_middle::ty::adjustment::{Adjust, Adjustment, OverloadedDeref};
910
use rustc_middle::ty::{self, Ty};
1011
use rustc_span::Span;
@@ -36,10 +37,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
3637
) -> InferOk<'tcx, Vec<Adjustment<'tcx>>> {
3738
let steps = autoderef.steps();
3839
if steps.is_empty() {
39-
return InferOk { obligations: vec![], value: vec![] };
40+
return InferOk { obligations: PredicateObligations::new(), value: vec![] };
4041
}
4142

42-
let mut obligations = vec![];
43+
let mut obligations = PredicateObligations::new();
4344
let targets =
4445
steps.iter().skip(1).map(|&(ty, _)| ty).chain(iter::once(autoderef.final_ty(false)));
4546
let steps: Vec<_> = steps

Diff for: ‎compiler/rustc_hir_typeck/src/closure.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_hir as hir;
88
use rustc_hir::lang_items::LangItem;
99
use rustc_hir_analysis::hir_ty_lowering::HirTyLowerer;
1010
use rustc_infer::infer::{BoundRegionConversionTime, DefineOpaqueTypes, InferOk, InferResult};
11-
use rustc_infer::traits::ObligationCauseCode;
11+
use rustc_infer::traits::{ObligationCauseCode, PredicateObligations};
1212
use rustc_macros::{TypeFoldable, TypeVisitable};
1313
use rustc_middle::span_bug;
1414
use rustc_middle::ty::visit::{TypeVisitable, TypeVisitableExt};
@@ -805,7 +805,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
805805
// [c1]: https://github.com/rust-lang/rust/pull/45072#issuecomment-341089706
806806
// [c2]: https://github.com/rust-lang/rust/pull/45072#issuecomment-341096796
807807
self.commit_if_ok(|_| {
808-
let mut all_obligations = vec![];
808+
let mut all_obligations = PredicateObligations::new();
809809
let supplied_sig = self.instantiate_binder_with_fresh_vars(
810810
self.tcx.def_span(expr_def_id),
811811
BoundRegionConversionTime::FnCall,

0 commit comments

Comments
 (0)