Skip to content

Commit c3a3312

Browse files
committed
Auto merge of #162531 - khyperia:abby-perf, r=BoxyUwU
fix perf regression from abby canonical form #161306 regressed performance on stable I believe all the perf impact is due to this https://github.com/rust-lang/rust/blob/d8df82673d5911b6112a85bf91d9adefb2c66a1a/compiler/rustc_infer/src/infer/mod.rs#L186 before, `SolverRegionConstraintStorage` was a dummy simple thing, no allocations. now, it's a `RegionConstraint { and: Box([]), or: Box([Box([])]) }` the majority of the perf impact (70%ish I think) is because the compiler is not sufficiently smart to optimize `And::new([])` into `And(Box::new([]))` (the former does a bunch of `IndexSet` allocations and stuff, the latter is a no-op, just a nullptr plus zero length metadata) the the rest of the perf impact (30%ish) is due to the `or` case allocating the `Box([Box([])])`, it's not just a nullptr+zero options to fix: - option A: just fix the `And::new([])` being terrible - option B: option A, *and also*, `SolverRegionConstraintStorage` stores an `Option` that is lazily init on first access, to prevent the perf hit from the `or` case - option C: option A, *and also*, use some kind of `SmallVec` something or other to make the `or` case be zero-alloc. I have not profiled this due to it being an invasive change and effort, this might not actually fix the perf. This PR is out option A to see if it actually works with the full perf machinery with PGO and whatnot instead of just on my machine (I am very inexperienced with perf testing!). It might be the case that full PGO blah blah *is* actually sufficiently smart to optimize `And::new([])`, and the actual perf diff is due to the `or` case (which I'm calling the 30%ish impact, might be actually 100%), in which case this PR should produce a no-op perf diff. r? @BoxyUwU
2 parents 018018e + a8f5602 commit c3a3312

2 files changed

Lines changed: 16 additions & 15 deletions

File tree

compiler/rustc_infer/src/infer/solver_region_constraints.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,23 @@ pub type SolverRegionConstraint<'tcx> =
66
rustc_type_ir::region_constraint::RegionConstraint<TyCtxt<'tcx>, Span>;
77

88
#[derive(Clone, Debug)]
9-
pub(crate) struct SolverRegionConstraintStorage<'tcx>(SolverRegionConstraint<'tcx>);
9+
pub(crate) struct SolverRegionConstraintStorage<'tcx>(Option<SolverRegionConstraint<'tcx>>);
1010

1111
impl<'tcx> SolverRegionConstraintStorage<'tcx> {
1212
pub(crate) fn new() -> Self {
13-
Self(SolverRegionConstraint::new_true())
13+
Self(None)
1414
}
1515

1616
pub(crate) fn get_constraint(&self) -> SolverRegionConstraint<'tcx> {
17-
self.0.clone()
17+
match &self.0 {
18+
Some(v) => v.clone(),
19+
None => SolverRegionConstraint::new_true(),
20+
}
1821
}
1922

2023
#[instrument(level = "debug", skip(self))]
2124
pub(crate) fn overwrite(&mut self, constraint: SolverRegionConstraint<'tcx>) {
22-
self.0 = constraint;
25+
self.0 = Some(constraint);
2326
}
2427
}
2528

compiler/rustc_type_ir/src/region_constraint.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ impl<I: Interner> Or<I> {
240240
}
241241
impl<I: Interner, S: Clone + std::hash::Hash + std::fmt::Debug + Eq> Or<I, S> {
242242
pub fn new_true() -> Self {
243-
Self(Box::new([And::new([])]))
243+
Self(Box::new([And::new_true()]))
244244
}
245245

246246
pub fn is_true(&self) -> bool {
@@ -323,6 +323,10 @@ impl<I: Interner> And<I> {
323323
}
324324
}
325325
impl<I: Interner, S: Clone + std::hash::Hash + std::fmt::Debug + Eq> And<I, S> {
326+
pub fn new_true() -> Self {
327+
Self(Box::new([]))
328+
}
329+
326330
pub fn new(i: impl IntoIterator<Item = LeafRegionConstraint<I, S>>) -> Self {
327331
let mut seen = IndexSet::new();
328332
And(i
@@ -406,7 +410,7 @@ impl<I: Interner, S: Clone + std::fmt::Debug + Eq + std::hash::Hash> RegionConst
406410
}));
407411

408412
Self {
409-
and_constraint: if or_constraint.is_false() { And::new([]) } else { and_constraint },
413+
and_constraint: if or_constraint.is_false() { And::new_true() } else { and_constraint },
410414
or_constraint,
411415
}
412416
}
@@ -422,7 +426,7 @@ impl<I: Interner, S: Clone + std::fmt::Debug + Eq + std::hash::Hash> RegionConst
422426
let or_constraint = Or::build_and(a.or_constraint, b.or_constraint);
423427

424428
Self {
425-
and_constraint: if or_constraint.is_false() { And::new([]) } else { and_constraint },
429+
and_constraint: if or_constraint.is_false() { And::new_true() } else { and_constraint },
426430
or_constraint,
427431
}
428432
}
@@ -432,15 +436,15 @@ impl<I: Interner, S: Clone + std::fmt::Debug + Eq + std::hash::Hash> RegionConst
432436
}
433437

434438
pub fn new_true() -> Self {
435-
Self { and_constraint: And::new([]), or_constraint: Or::new_true() }
439+
Self { and_constraint: And::new_true(), or_constraint: Or::new_true() }
436440
}
437441

438442
pub fn is_true(&self) -> bool {
439443
self.and_constraint.0.is_empty() && self.or_constraint.is_true()
440444
}
441445

442446
pub fn new_false() -> Self {
443-
Self { and_constraint: And::new([]), or_constraint: Or::new_false() }
447+
Self { and_constraint: And::new_true(), or_constraint: Or::new_false() }
444448
}
445449

446450
pub fn is_false(&self) -> bool {
@@ -477,12 +481,6 @@ impl<I: Interner, S: Clone + std::fmt::Debug + Eq + std::hash::Hash> RegionConst
477481
}
478482
}
479483

480-
impl<I: Interner> Default for RegionConstraint<I> {
481-
fn default() -> Self {
482-
Self::new_true()
483-
}
484-
}
485-
486484
impl<I: Interner, S: Clone + std::fmt::Debug> LeafRegionConstraint<I, S> {
487485
pub fn is_ambig(&self) -> bool {
488486
matches!(self, Self::Ambiguity(_))

0 commit comments

Comments
 (0)