[perf] Make fulfill in method probe less bad #124303

compiler-errors · 2024-04-23T18:25:46Z

r? @lcnr

compiler-errors · 2024-04-23T18:25:52Z

@bors try @rust-timer queue

…<try> [perf] Make fulfill in method probe less bad r? `@lcnr`

bors · 2024-04-23T18:27:04Z

⌛ Trying commit cafa457 with merge b0dd851...

jackh726 · 2024-04-23T18:57:45Z

compiler/rustc_hir_typeck/src/method/probe.rs

-                    ));
+                    )
+                    .collect();
+                    if let Some(obligation) = predicates.iter().find(|obligation| {


Could this be rewritten as if !self.infcx.next_trait_solver() && let Some(obligation) = predicates.iter().find(...)

bors · 2024-04-23T20:06:23Z

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

rust-timer · 2024-04-23T21:21:20Z

Finished benchmarking commit (b0dd851): comparison URL.

Overall result: ❌ regressions - 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.2%	[0.2%, 0.2%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.2%, 0.2%]	1

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)	3.7%	[3.3%, 4.0%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.7%	[3.3%, 4.0%]	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)	-	-	0
Improvements ✅ (secondary)	-2.2%	[-2.3%, -2.0%]	2
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 675.427s -> 672.261s (-0.47%)
Artifact size: 315.55 MiB -> 316.22 MiB (0.21%)

compiler-errors · 2024-04-24T00:31:13Z

sadness

compiler-errors · 2024-04-24T15:36:33Z

@bors try @rust-timer queue

bors · 2024-04-24T15:37:44Z

⌛ Trying commit 3ffdd7f with merge 6b3da17...

…<try> [perf] Make fulfill in method probe less bad r? `@lcnr`

RalfJung · 2024-04-24T17:10:54Z

@bors r-
just to make sure bors does not merge this...

RalfJung · 2024-04-24T17:11:50Z

@bors retry

bors · 2024-04-24T17:24:15Z

⌛ Trying commit 3ffdd7f with merge 6b1c814...

…<try> [perf] Make fulfill in method probe less bad r? `@lcnr`

bors · 2024-04-24T19:02:08Z

☀️ Try build successful - checks-actions
Build commit: 6b1c814 (6b1c8148bb8d72a077217b1ead0bc424bdbaa2fb)

rust-timer · 2024-04-24T20:17:41Z

Finished benchmarking commit (6b1c814): 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

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)	3.7%	[3.7%, 3.7%]	1
Improvements ✅ (primary)	-2.2%	[-2.2%, -2.2%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.2%	[-2.2%, -2.2%]	1

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)	-	-	0
Improvements ✅ (secondary)	-2.9%	[-3.1%, -2.6%]	4
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 674.062s -> 671.689s (-0.35%)
Artifact size: 316.12 MiB -> 316.15 MiB (0.01%)

compiler-errors · 2024-04-24T20:55:33Z

@bors try @rust-timer queue

compiler-errors · 2024-04-24T20:56:30Z

compiler/rustc_hir_typeck/src/method/probe.rs

+                                EvaluationResult::EvaluatedToOk => {
+                                    // No side-effects, no need to register obligations.
+                                }
+                                EvaluationResult::EvaluatedToOkModuloRegions


TODO: We could also skip registering the obligations if they're OkModuloRegions, since we already check for leak check failures in the eval call.

bors · 2024-04-24T20:56:43Z

⌛ Trying commit b4427cd with merge aeb7743...

…<try> [perf] Make fulfill in method probe less bad r? `@lcnr`

bors · 2024-04-24T22:30:33Z

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

rust-timer · 2024-04-25T00:00:55Z

Finished benchmarking commit (aeb7743): 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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.3%	[-0.3%, -0.3%]	1

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
Regressions ❌ (secondary)	4.7%	[3.9%, 5.5%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

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

Binary size

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

Bootstrap: 672.117s -> 675.012s (0.43%)
Artifact size: 316.14 MiB -> 315.46 MiB (-0.21%)

compiler-errors · 2024-04-25T00:05:26Z

@bors try @rust-timer queue

…<try> [perf] Make fulfill in method probe less bad r? `@lcnr`

bors · 2024-04-25T00:06:37Z

⌛ Trying commit 6117987 with merge 5b27a6e...

bors · 2024-04-25T01:39:21Z

☀️ Try build successful - checks-actions
Build commit: 5b27a6e (5b27a6eaf1fed2d7eaad364e35a7f44b177a89da)

rust-timer · 2024-04-25T03:02:04Z

Finished benchmarking commit (5b27a6e): 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
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.3%	[-0.3%, -0.3%]	1

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)	3.1%	[2.7%, 3.6%]	2
Regressions ❌ (secondary)	4.0%	[4.0%, 4.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.1%	[2.7%, 3.6%]	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)	-	-	0
Improvements ✅ (secondary)	-2.2%	[-2.4%, -2.1%]	3
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 674.533s -> 673.567s (-0.14%)
Artifact size: 316.16 MiB -> 315.48 MiB (-0.22%)

compiler-errors · 2024-04-25T15:53:08Z

Ok -- after many attempts to fix this, I'm tempted to say that this perf regression is not possible to fix.

rustbot assigned lcnr Apr 23, 2024

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 Apr 23, 2024