cache param env canonicalization

[aliemjay]

Canonicalize ParamEnv only once and store it. Then whenever we try to canonicalize ParamEnvAnd<'tcx, T> we only have to canonicalize T and then merge the results.

Prelimiary results show ~3-4% savings in diesel and serde benchmarks.

Best to review commits individually. Some commits have a short description.

Initial implementation had a soundness bug (#117749 (comment)) due to cache invalidation:

• When canonicalizing Ty<'?0> we first try to resolve region variables in the current InferCtxt which may have a constraint ?0 == 'static. This means that we register Ty<'?0> => Canonical<Ty<'static>> in the cache, which is obviously incorrect in another inference context.
• This is fixed by not doing region resolution when canonicalizing the query input (vs. response), which is the only place where ParamEnv is used, and then in a later commit we statically guard against any form of inference variable resolution of the cached canonical ParamEnv's.

r? @ghost

[aliemjay]

@bors try @rust-timer queue

[bors]

☀️ Try build successful - checks-actions
Build commit: 557c12e (557c12e39628232629cbe904f927f16144010a47)

[bors]

☀️ Try build successful - checks-actions
Build commit: 557c12e (557c12e39628232629cbe904f927f16144010a47)

[rust-timer]

Finished benchmarking commit (557c12e): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.3%, 0.3%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.9%	[-3.3%, -0.4%]	17
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.7%	[-3.3%, 0.3%]	18

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.7%	[0.6%, 0.8%]	2
Regressions ❌ (secondary)	4.7%	[4.3%, 5.2%]	2
Improvements ✅ (primary)	-1.2%	[-4.2%, -0.4%]	11
Improvements ✅ (secondary)	-2.0%	[-2.0%, -2.0%]	1
All ❌✅ (primary)	-1.0%	[-4.2%, 0.8%]	13

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.5%, 0.5%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-1.3%, -0.5%]	6
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.7%	[-1.3%, 0.5%]	7

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 661.067s -> 661.537s (0.07%)
Artifact size: 308.78 MiB -> 308.75 MiB (-0.01%)

[aliemjay]

another approach with a global cache
@bors try @rust-timer queue

[aliemjay]

The job x86_64-gnu-llvm-16 failed! Check out the build log: (web) (plain)

MCVE:

struct VarZeroVec<'a, F>(&'a (), F);

trait Yokeable<'a> {
    type Output;
}

impl<'a> Yokeable<'a> for () {
    type Output = VarZeroVec<'a, ()>;
}

fn transform_mut<'a, F>(f: F)
where
    F: FnOnce(VarZeroVec<'a, ()>),
{
    f(absurd::<<() as Yokeable<'a>>::Output>())
}

fn absurd<T>() -> T{ todo!() }

[aliemjay]

@bors try @rust-timer queue

[bors]

☀️ Try build successful - checks-actions
Build commit: fd72347 (fd72347c63ef586b13bce5e2c07d442a0b5ec7ef)

[lcnr]

r? @lcnr

will look at this soon (hopefully)

[lcnr]

some nits, then r=me, afaict CI was an unrelated issue

[lcnr]

This does not spark joy 😁 Does anything break if you change this to CanonicalizeFreeRegions? I want to get away from canonicalizing static completely. iirc the last time I tried after #102472 there was still some failure in project or sth 🤔 would prefer for you (or me) to first try to also canonicalize 'static again to check why it matters.

[aliemjay]

Borrowck error in tests/ui/trivial-bounds/trivial-bounds-object.rs

ICE in tests/ui/nll/issue-61311-normalize.rs and tests/ui/nll/issue-61320-normalize.rs. MCVE:

pub trait HasProj {
    type Proj;
}

impl<T: ?Sized> HasProj for T {
    type Proj = ();
}

fn test()
where
    dyn Send + 'static: HasProj,
    <dyn Send + 'static as HasProj>::Proj: Clone,
{
    // ICE: cannot prove `<dyn Send + 'static as HasProj>::Proj: Clone`
    let _ = test;
}

[aliemjay]

cc #118965. It is also the root cause behind the ICE.

[aliemjay]

some nits, then r=me, afaict CI was an unrelated issue

I turns out it is related. Rustdoc incorrectly uses ParamEnv's with inference variables in them. This causes an ICE during canonicalization in one of the unwraps in the commit "make infcx optional in canonicalizer". This was fixed by the last commit by using an empty param env. I assume it does not need a re-approval.

@bors r=lcnr rollup=never

[bors]

📌 Commit aa36c35 has been approved by lcnr

It is now in the queue for this repository.

[bors]

⌛ Testing commit aa36c35 with merge d23e1a6...

[bors]

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing d23e1a6 to master...

[rust-timer]

Finished benchmarking commit (d23e1a6): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.0%	[-4.8%, -0.2%]	32
Improvements ✅ (secondary)	-1.3%	[-1.4%, -1.3%]	2
All ❌✅ (primary)	-2.0%	[-4.8%, -0.2%]	32

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.9%	[0.7%, 1.2%]	2
Regressions ❌ (secondary)	3.5%	[2.5%, 4.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.9%	[0.7%, 1.2%]	2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.8%	[-3.0%, -0.7%]	16
Improvements ✅ (secondary)	-1.5%	[-1.5%, -1.5%]	1
All ❌✅ (primary)	-1.8%	[-3.0%, -0.7%]	16

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 673.665s -> 670.811s (-0.42%)
Artifact size: 312.46 MiB -> 312.42 MiB (-0.01%)