Assert that global cache values are stable on insertion

[compiler-errors]

Let's make sure not to overwrite cache values with new values in, e.g., the evaluation cache and selection cache. I haven't found this being triggered in practice, but if we ever did, that'd be incredibly interesting to study.

There was a FIXME that says that we should be using HashMapExt::insert_same, but it turns out that we can't (cc @Zoxc in #50507), because it turns out that these cache entries are only equal modulo dep node index. But we don't need to use that particular method to protect against collisions in these caches, since we can just ignore the dep node. So instead of using that, let's just check that the values are the same.

[rustbot]

r? @WaffleLapkin

(rustbot has picked a reviewer for you, use r? to override)

[Zoxc]

This only partially addresses the FIXME, given that the DepNodeIndex can still differ. The relevant question is why the same value ends up having different dependencies? If one of them are missing dependencies, that can lead to incremental bugs.

[compiler-errors]

Ah, @SparrowLii's #100987 seems to suggest a reason for the instability of the DepNodeIndex part of the cache value?

I'm mostly interested in the cached EvaluationResult here staying stable. I'm happy to add the FIXME back though because of your concern, or at least clarify the FIXME though.

[compiler-errors]

Lol, oh, didn't read down-thread enough to see you commented on that. Oh well. I'll update the FIXME anyways.

[WaffleLapkin]

This looks good to me. Do we need a perf run? Adding an assertion to the cache seems potentially damaging it.

[compiler-errors]

@bors try @rust-timer queue

[bors]

⌛ Trying commit 6b9862d with merge 6fedcc8c38319f932c1e095b4e13a5cb1268253c...

[compiler-errors]

@rust-timer queue 6fedcc8c38319f932c1e095b4e13a5cb1268253c

[compiler-errors]

@bors try

[bors]

⌛ Trying commit 6b9862d with merge dc41b88ee80166179217d22575571a4b01efdc00...

[compiler-errors]

whoops wrong rust-timer command, maybe my try build from last night did succeed

@rust-timer build 6fedcc8c38319f932c1e095b4e13a5cb1268253c

[rust-timer]

Finished benchmarking commit (6fedcc8c38319f932c1e095b4e13a5cb1268253c): comparison URL.

Overall result: ❌ regressions - 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.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

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

Warning ⚠: The following benchmark(s) failed to build:

• diesel-1.4.8

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)	1.7%	[0.7%, 2.9%]	4
Regressions ❌ (secondary)	4.9%	[0.2%, 8.1%]	8
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.7%	[0.7%, 2.9%]	4

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)	1.8%	[1.8%, 1.8%]	1
Regressions ❌ (secondary)	2.3%	[1.6%, 2.9%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-3.5%, -3.5%]	1
All ❌✅ (primary)	1.8%	[1.8%, 1.8%]	1

Cycles

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

[compiler-errors]

Warning warning: The following benchmark(s) failed to build:
diesel-1.4.8

this sucks lol

[bors]

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

[Zoxc]

Why was this closed? The failure seems like an existing bug to me?

[compiler-errors]

That's a cache conflict in the InferCtxt-local cache used by evaluation.

It's probably not a bug worth solving, since it's likely only going to happen on overflow anyways, when a goal is later re-evaluated without hitting overflow because the depth is lower or something.

[Zoxc]

That does seem like something that could lead to incremental compilation bugs.