Avoid doing redundant work in `receiver_is_valid` by compiler-errors · Pull Request #127473 · rust-lang/rust

compiler-errors · 2024-07-08T03:01:39Z

Avoids a redundant eq call to restore perf.

compiler-errors · 2024-07-08T03:01:48Z

…, r=<try> Revert "Rework receiver_is_valid" This reverts commit 465e7d5. Testing perf: rust-lang#127172 (comment) r? `@ghost`

bors · 2024-07-08T03:02:58Z

⌛ Trying commit 355ef6b with merge 1f9fc8f...

compiler-errors · 2024-07-08T03:02:21Z

compiler/rustc_hir_analysis/src/check/wfcheck.rs

    }

+    // The first type is `receiver_ty`, which we know its not equal to `self_ty`; skip it.
+    autoderef.next();


This autoderef may have been load-bearing, since we're doing >1 equate per method even though we tested the receiver was equal above.

compiler-errors · 2024-07-08T03:03:25Z

compiler/rustc_hir_analysis/src/check/wfcheck.rs

-            // Register the bound, in case it has any region side-effects.
-            wfcx.register_bound(
-                cause.clone(),
-                wfcx.param_env,
-                potential_self_ty,
-                receiver_trait_def_id,
-            );


This may have been perf negative, since we now have trait goals to process... If this is the problem, then we probably need to accept this for soundness reasons if we ever want to stabilize receiver lol

so this means we only check T: Receiver ignoring regions? 🤔

As we were previously. Let me try adding it back and seeing what happens still...

bors · 2024-07-08T04:44:32Z

☀️ Try build successful - checks-actions
Build commit: 1f9fc8f (1f9fc8fd464c879807a0b98c316c67fc3442ac9f)

rust-timer · 2024-07-08T05:58:55Z

Finished benchmarking commit (1f9fc8f): comparison URL.

Overall result: ❌✅ regressions and improvements - BENCHMARK(S) FAILED

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:

rustc
❗ ❗ ❗ ❗ ❗

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.1%	[0.1%, 0.1%]	2
Improvements ✅ (primary)	-0.3%	[-0.5%, -0.2%]	18
Improvements ✅ (secondary)	-0.7%	[-0.9%, -0.7%]	7
All ❌✅ (primary)	-0.3%	[-0.5%, -0.2%]	18

Max RSS (memory usage)

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

Cycles

Results (primary 2.9%, secondary 3.1%)

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

Binary size

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

Bootstrap: missing data
Artifact size: 328.36 MiB -> 328.37 MiB (0.00%)

compiler-errors · 2024-07-08T14:54:14Z

OK let me try my theories

compiler-errors · 2024-07-08T14:58:12Z

@bors try @rust-timer queue

…, r=<try> Revert "Rework receiver_is_valid" This reverts commit 465e7d5. Testing perf: rust-lang#127172 (comment) r? `@ghost`

bors · 2024-07-08T14:59:23Z

⌛ Trying commit ae938b0 with merge 087de49...

bors · 2024-07-08T16:43:01Z

☀️ Try build successful - checks-actions
Build commit: 087de49 (087de49937a84f60f5875f854c8a12dfe2a22317)

rust-timer · 2024-07-08T18:35:00Z

Finished benchmarking commit (087de49): 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
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	3
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

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: 701.456s -> 702s (0.08%)
Artifact size: 328.71 MiB -> 328.59 MiB (-0.04%)

compiler-errors · 2024-07-08T19:51:43Z

@bors try @rust-timer queue

…, r=<try> Revert "Rework receiver_is_valid" This reverts commit 465e7d5. Testing perf: rust-lang#127172 (comment) r? `@ghost`

bors · 2024-07-08T19:52:53Z

⌛ Trying commit 865b9ab with merge 6c0f300...

bors · 2024-07-08T21:34:32Z

☀️ Try build successful - checks-actions
Build commit: 6c0f300 (6c0f300ad1704533f4e7ce733745eebe37964c25)

rust-timer · 2024-07-09T01:34:07Z

Finished benchmarking commit (6c0f300): 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.4%, -0.2%]	14
Improvements ✅ (secondary)	-0.7%	[-0.8%, -0.6%]	7
All ❌✅ (primary)	-0.3%	[-0.4%, -0.2%]	14

Max RSS (memory usage)

Results (primary 1.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)	5.5%	[5.5%, 5.5%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.4%	[-2.4%, -2.4%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.5%	[-2.4%, 5.5%]	2

Cycles

Results (secondary -2.4%)

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.4%	[-2.9%, -2.1%]	4
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 700.067s -> 702.579s (0.36%)
Artifact size: 328.73 MiB -> 328.74 MiB (0.00%)

compiler/rustc_hir_analysis/src/check/wfcheck.rs

compiler-errors · 2024-07-16T15:55:01Z

@bors try @rust-timer queue

bors · 2024-07-16T16:00:58Z

⌛ Trying commit c2630bc with merge 3b4d3cf...

…, r=<try> Avoid doing redundant work in `receiver_is_valid` Avoids a redundant `eq` call to restore perf.

bors · 2024-07-16T17:49:07Z

☀️ Try build successful - checks-actions
Build commit: 3b4d3cf (3b4d3cffd6e69b175ab80906bb0e32e1c048f7ee)

rust-timer · 2024-07-16T19:48:37Z

Finished benchmarking commit (3b4d3cf): comparison URL.

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

Max RSS (memory usage)

Results (primary 2.3%)

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

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: 700.14s -> 700.55s (0.06%)
Artifact size: 328.63 MiB -> 328.64 MiB (0.00%)

compiler-errors · 2024-07-16T20:11:55Z

Ok yeah, it really is confirming the trait goal which is the problem. I don't think we should do this -- actually having lifetime outputs from our goals is important for WF :>

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 Jul 8, 2024

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 Jul 8, 2024

compiler-errors commented Jul 8, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jul 8, 2024

w

ae938b0

compiler-errors force-pushed the perf-revert-receiver branch from 355ef6b to ae938b0 Compare July 8, 2024 14:58

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 Jul 8, 2024

klensy mentioned this pull request Jul 8, 2024

fix alert display when build fails rust-lang/rustc-perf#1941

Merged

This comment has been minimized.

Sign in to view

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Jul 8, 2024

w

865b9ab

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 Jul 8, 2024

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 Jul 9, 2024

Kobzol mentioned this pull request Jul 9, 2024

Make can_eq process obligations (almost) everywhere #127172

Merged

compiler-errors changed the title ~~Revert "Rework receiver_is_valid"~~ Avoid doing redundant work in receiver_is_valid Jul 15, 2024

compiler-errors assigned lcnr Jul 15, 2024

compiler-errors marked this pull request as ready for review July 15, 2024 22:19

compiler-errors commented Jul 16, 2024

View reviewed changes

compiler/rustc_hir_analysis/src/check/wfcheck.rs Show resolved Hide resolved

Add it back

c2630bc

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 Jul 16, 2024

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 16, 2024

Auto merge of rust-lang#127473 - compiler-errors:perf-revert-receiver…

3b4d3cf

…, r=<try> Avoid doing redundant work in `receiver_is_valid` Avoids a redundant `eq` call to restore perf.

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 Jul 16, 2024

compiler-errors closed this Jul 16, 2024

Uh oh!

Conversation

compiler-errors commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

compiler-errors commented Jul 8, 2024

Uh oh!

This comment has been minimized.

bors commented Jul 8, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Jul 8, 2024

Uh oh!

This comment has been minimized.

rust-timer commented Jul 8, 2024

Overall result: ❌✅ regressions and improvements - BENCHMARK(S) FAILED

Uh oh!

compiler-errors commented Jul 8, 2024

Uh oh!

compiler-errors commented Jul 8, 2024

Uh oh!

This comment has been minimized.

bors commented Jul 8, 2024

Uh oh!

bors commented Jul 8, 2024

Uh oh!

This comment has been minimized.

rust-timer commented Jul 8, 2024

Overall result: ✅ improvements - no action needed

Uh oh!

compiler-errors commented Jul 8, 2024

Uh oh!

This comment has been minimized.

bors commented Jul 8, 2024

Uh oh!

bors commented Jul 8, 2024

Uh oh!

This comment has been minimized.

rust-timer commented Jul 9, 2024

Overall result: ✅ improvements - no action needed

Uh oh!

Uh oh!

compiler-errors commented Jul 16, 2024

Uh oh!

This comment has been minimized.

bors commented Jul 16, 2024

Uh oh!

bors commented Jul 16, 2024

Uh oh!

This comment has been minimized.

rust-timer commented Jul 16, 2024

Overall result: ❌✅ regressions and improvements - no action needed

Uh oh!

compiler-errors commented Jul 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

compiler-errors commented Jul 8, 2024 •

edited

Loading