add cache to `AmbiguityCausesVisitor` #135618

lcnr · 2025-01-17T09:03:37Z

fixes #135457, alternative to #135524.

cc https://rust-lang.zulipchat.com/#narrow/channel/364551-t-types.2Ftrait-system-refactor/topic/new-solver.20hang.20.23135457

r? @compiler-errors

lcnr · 2025-01-17T09:03:46Z

@bors try @rust-timer queue

add cache to `AmbiguityCausesVisitor` fixes rust-lang#135457, alternative to rust-lang#135524. cc https://rust-lang.zulipchat.com/#narrow/channel/364551-t-types.2Ftrait-system-refactor/topic/new-solver.20hang.20.23135457 r? `@compiler-errors`

bors · 2025-01-17T09:04:58Z

⌛ Trying commit 94bf8f0 with merge 1a372d6...

bors · 2025-01-17T10:54:31Z

☀️ Try build successful - checks-actions
Build commit: 1a372d6 (1a372d66de385a25aaacfb711a4d1d32af5acd55)

rust-timer · 2025-01-17T12:11:32Z

Finished benchmarking commit (1a372d6): comparison URL.

Overall result: no relevant changes - 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 benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.5%, secondary 2.5%)

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)	2.5%	[1.6%, 3.4%]	2
Regressions ❌ (secondary)	4.9%	[1.7%, 8.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.3%	[-2.3%, -2.3%]	1
All ❌✅ (primary)	2.5%	[1.6%, 3.4%]	2

Cycles

Results (primary -1.6%, secondary 2.8%)

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)	2.8%	[2.8%, 2.8%]	1
Improvements ✅ (primary)	-1.6%	[-2.3%, -1.2%]	3
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-1.6%	[-2.3%, -1.2%]	3

Binary size

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

Bootstrap: 764.62s -> 763.451s (-0.15%)
Artifact size: 326.02 MiB -> 326.07 MiB (0.02%)

compiler-errors · 2025-01-17T16:32:13Z

@bors r+

bors · 2025-01-17T16:32:15Z

📌 Commit 0910173 has been approved by compiler-errors

It is now in the queue for this repository.

Kobzol · 2025-01-17T16:34:47Z

Should we perhaps add the test as a secondary benchmark in rustc-perf instead? If something regresses from seconds to minutes, it will hardly be visible in CI. Last year, the whole CI took like 2 hours longer because of some similar blow-up and we only found out after a couple of days, IIRC.

CC @lqd if you have opinions on the topic :)

lqd · 2025-01-17T16:53:35Z

"Instead", or in addition to this one?

"Instead of in this PR", I guess not really: since we also need an in-tree test for the Compilation taking unexpected long after 1.84 #135457 issue itself.
"In addition to", why not. It should be easier to see it regress on rustc-perf, yeah. As to whether it's needed, I'm not really sure, did we add secondary benchmarks for the other new solver blowups?

Kobzol · 2025-01-17T16:55:30Z

Yeah sorry, I meant "in addition to". I think that last time we added typenum as a benchmark for the new solver, but that's not exactly a stress test :)

bors · 2025-01-18T00:06:30Z

⌛ Testing commit 0910173 with merge 8e59cf9...

bors · 2025-01-18T02:52:18Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 8e59cf9 to master...

rust-timer · 2025-01-18T04:10:13Z

Finished benchmarking commit (8e59cf9): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.3%, 0.3%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 2.5%)

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)	2.5%	[1.5%, 3.4%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.5%	[1.5%, 3.4%]	2

Cycles

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

Binary size

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

Bootstrap: 763.854s -> 765.445s (0.21%)
Artifact size: 326.09 MiB -> 326.00 MiB (-0.03%)

apiraino · 2025-01-23T15:18:54Z

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle. Backport labels handled by them.

Stable backport approved as well. Since a dot release seems likely, this is a good candidate for inclusion.

@rustbot label +beta-accepted +stable-accepted

[beta] backports - Always force non-trimming of path in `unreachable_patterns` lint rust-lang#135310 - Add Profile Override for Non-Git Sources rust-lang#135433 - resolve symlinks of LLVM tool binaries before copying them rust-lang#135585 - add cache to `AmbiguityCausesVisitor` rust-lang#135618 - When LLVM's location discriminator value limit is exceeded, emit locations with dummy spans instead of dropping them entirely rust-lang#135643 - Temporarily bring back `Rvalue::Len` rust-lang#135709 - make it possible to use ci-rustc on tarball sources rust-lang#135722 - Remove test panic from File::open rust-lang#135837 - Only assert the `Parser` size on specific arches rust-lang#135855 r? cuviper

[beta] backports - Always force non-trimming of path in `unreachable_patterns` lint rust-lang#135310 - Add Profile Override for Non-Git Sources rust-lang#135433 - resolve symlinks of LLVM tool binaries before copying them rust-lang#135585 - add cache to `AmbiguityCausesVisitor` rust-lang#135618 - When LLVM's location discriminator value limit is exceeded, emit locations with dummy spans instead of dropping them entirely rust-lang#135643 - Temporarily bring back `Rvalue::Len` rust-lang#135709 - make it possible to use ci-rustc on tarball sources rust-lang#135722 - Remove test panic from File::open rust-lang#135837 - Only assert the `Parser` size on specific arches rust-lang#135855 - [beta] TRPL: more backward-compatible Edition changes rust-lang#135843 r? cuviper

[beta] backports - Always force non-trimming of path in `unreachable_patterns` lint rust-lang#135310 - Add Profile Override for Non-Git Sources rust-lang#135433 - resolve symlinks of LLVM tool binaries before copying them rust-lang#135585 - add cache to `AmbiguityCausesVisitor` rust-lang#135618 - When LLVM's location discriminator value limit is exceeded, emit locations with dummy spans instead of dropping them entirely rust-lang#135643 - make it possible to use ci-rustc on tarball sources rust-lang#135722 - Remove test panic from File::open rust-lang#135837 - Only assert the `Parser` size on specific arches rust-lang#135855 - [beta] TRPL: more backward-compatible Edition changes rust-lang#135843 r? cuviper

[stable] Prepare Rust 1.84.1 point release - [Fix ICE 132920 in duplicate-crate diagnostics.](rust-lang#133304) - [Fix errors for overlapping impls in incremental rebuilds.](rust-lang#133828) - [Fix slow compilation related to the next-generation trait solver.](rust-lang#135618) - [Fix debuginfo when LLVM's location discriminator value limit is exceeded.](rust-lang#135643) - Fixes for building Rust from source: - [Only try to distribute `llvm-objcopy` if llvm tools are enabled.](rust-lang#134240) - [Add Profile Override for Non-Git Sources.](rust-lang#135433) - [Resolve symlinks of LLVM tool binaries before copying them.](rust-lang#135585) - [Make it possible to use ci-rustc on tarball sources.](rust-lang#135722) cc `@rust-lang/release` r? ghost

lcnr added 2 commits January 15, 2025 09:58

avoid running the overlap check twice

ebbcfd4

add cache to AmbiguityCausesVisitor

94bf8f0

rustbot assigned compiler-errors Jan 17, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 17, 2025

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 17, 2025

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 17, 2025

add test

0910173

rustbot added the WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) label Jan 17, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 17, 2025

lcnr added beta-nominated Nominated for backporting to the compiler in the beta channel. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Jan 17, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 18, 2025

bors merged commit 8e59cf9 into rust-lang:master Jan 18, 2025
7 checks passed

rustbot added this to the 1.86.0 milestone Jan 18, 2025

lcnr deleted the coherence-unknown branch January 20, 2025 09:32

apiraino mentioned this pull request Jan 23, 2025

don't check for overlap if it's allowed regardless #135524

Closed

rustbot added beta-accepted Accepted for backporting to the compiler in the beta channel. stable-accepted Accepted for backporting to the compiler in the stable channel. labels Jan 23, 2025

cuviper mentioned this pull request Jan 24, 2025

[beta] backports #136017

Merged

cuviper modified the milestones: 1.86.0, 1.85.0 Jan 24, 2025

cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jan 24, 2025

cuviper mentioned this pull request Jan 27, 2025

[stable] Prepare Rust 1.84.1 point release #136158

Merged

cuviper modified the milestones: 1.85.0, 1.84.1 Jan 27, 2025

cuviper removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Jan 27, 2025

lcnr restored the coherence-unknown branch January 30, 2025 15:59

lcnr deleted the coherence-unknown branch January 31, 2025 14:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add cache to `AmbiguityCausesVisitor` #135618

add cache to `AmbiguityCausesVisitor` #135618

lcnr commented Jan 17, 2025 •

edited

Loading

lcnr commented Jan 17, 2025

This comment has been minimized.

bors commented Jan 17, 2025

bors commented Jan 17, 2025

This comment has been minimized.

rust-timer commented Jan 17, 2025

compiler-errors commented Jan 17, 2025

bors commented Jan 17, 2025

Kobzol commented Jan 17, 2025 •

edited

Loading

lqd commented Jan 17, 2025 •

edited

Loading

Kobzol commented Jan 17, 2025

bors commented Jan 18, 2025

bors commented Jan 18, 2025

rust-timer commented Jan 18, 2025

apiraino commented Jan 23, 2025 •

edited

Loading

add cache to AmbiguityCausesVisitor #135618

add cache to AmbiguityCausesVisitor #135618

Conversation

lcnr commented Jan 17, 2025 • edited Loading

lcnr commented Jan 17, 2025

This comment has been minimized.

bors commented Jan 17, 2025

bors commented Jan 17, 2025

This comment has been minimized.

rust-timer commented Jan 17, 2025

Overall result: no relevant changes - no action needed

compiler-errors commented Jan 17, 2025

bors commented Jan 17, 2025

Kobzol commented Jan 17, 2025 • edited Loading

lqd commented Jan 17, 2025 • edited Loading

Kobzol commented Jan 17, 2025

bors commented Jan 18, 2025

bors commented Jan 18, 2025

rust-timer commented Jan 18, 2025

Overall result: ❌ regressions - no action needed

apiraino commented Jan 23, 2025 • edited Loading

add cache to `AmbiguityCausesVisitor` #135618

add cache to `AmbiguityCausesVisitor` #135618

lcnr commented Jan 17, 2025 •

edited

Loading

Kobzol commented Jan 17, 2025 •

edited

Loading

lqd commented Jan 17, 2025 •

edited

Loading

apiraino commented Jan 23, 2025 •

edited

Loading