Fast-path some relations via strict equality#104598

compiler-errors · 2022-11-19T06:04:14Z

Some relations can take advantage of strict equality (that is, the == operator), which is far cheaper to check in some cases like substs and existential predicate lists.

This also fixes #104583.

r? @ghost

compiler-errors · 2022-11-19T06:04:20Z

@bors try @rust-timer queue

bors · 2022-11-19T06:04:28Z

⌛ Trying commit 3a5bf11 with merge bf7cfa7141d188772c184387e27cc0505b3dcc5c...

bors · 2022-11-19T08:27:09Z

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

rust-timer · 2022-11-19T11:35:37Z

Finished benchmarking commit (bf7cfa7141d188772c184387e27cc0505b3dcc5c): 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-review -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.3%]	5
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.3%	[-0.4%, -0.3%]	5

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)	1.2%	[1.2%, 1.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.0%	[-5.0%, -3.0%]	2
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)	3.3%	[3.3%, 3.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

matthiaskrgr · 2022-11-19T14:38:46Z

compiler/rustc_hir_analysis/src/check/dropck.rs


+    fn fast_equate_combine(&self) -> bool {
+        true
+    }
+


should these be marked #[inline]?

perhaps that would help... 🤔

compiler-errors · 2022-11-19T18:42:50Z

I'll try one more perf run, but unclear that this is worth the complexity...

@bors try @rust-timer queue

bors · 2022-11-19T18:42:59Z

⌛ Trying commit d1ed286 with merge aff588effced4b8915a1406ff5b4271169c1181a...

bors · 2022-11-19T21:00:12Z

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

rust-timer · 2022-11-19T22:20:07Z

Finished benchmarking commit (aff588effced4b8915a1406ff5b4271169c1181a): 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-review -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)	-1.3%	[-1.3%, -1.3%]	2
All ❌✅ (primary)	-	-	0

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.1%	[1.1%, 1.1%]	1
Regressions ❌ (secondary)	3.2%	[3.2%, 3.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.1%	[1.1%, 1.1%]	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)	5.8%	[5.4%, 6.2%]	2
Improvements ✅ (primary)	-1.5%	[-2.1%, -0.7%]	17
Improvements ✅ (secondary)	-3.0%	[-4.1%, -2.2%]	18
All ❌✅ (primary)	-1.5%	[-2.1%, -0.7%]	17

ptrca · 2022-11-20T00:05:33Z

compiler/rustc_middle/src/ty/relate.rs


    /// Returns whether or not structural equality can be used to fast-path this relation
-    fn fast_equate_combine(&self) -> bool;
+    fn fast_equate(&self) -> bool;


Annotate inline this declaration instead?

#[inline] means nothing on trait methods without a body.

jackh726 · 2022-11-21T18:06:51Z

Is this confirmed to fix #104583? I don't think the perf is enough of a win to justify to added complexity otherwise. If we want to land this, I think every one of the trues need to have a comment explaining why the can be true.

Also, can just make a default method with false, and only add an impl method for true.

compiler-errors · 2022-11-26T05:33:11Z

Is this confirmed to fix #104583?

Yes, though I can probably reformulate this to be simpler, given that we don't have a perf win in general. Let me close this and go back to the drawing board.

…, r=oli-obk Fast-path some binder relations A simpler approach than rust-lang#104598 Fixes rust-lang#104583 r? types

Fast-path some relations via strict equality

3a5bf11

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 Nov 19, 2022

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 Nov 19, 2022

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 Nov 19, 2022

matthiaskrgr reviewed Nov 19, 2022

View reviewed changes

rename, add inline

d1ed286

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 Nov 19, 2022

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 Nov 19, 2022

ptrca reviewed Nov 20, 2022

View reviewed changes

compiler-errors closed this Nov 26, 2022

compiler-errors mentioned this pull request Dec 6, 2022

Fast-path some binder relations #105350

Merged

bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 13, 2022

Auto merge of rust-lang#105350 - compiler-errors:faster-binder-relate…

109cccb

…, r=oli-obk Fast-path some binder relations A simpler approach than rust-lang#104598 Fixes rust-lang#104583 r? types

compiler-errors deleted the fast-path-relate branch August 11, 2023 20:01

Uh oh!

Conversation

compiler-errors commented Nov 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

compiler-errors commented Nov 19, 2022

Uh oh!

This comment has been minimized.

bors commented Nov 19, 2022

Uh oh!

bors commented Nov 19, 2022

Uh oh!

This comment has been minimized.

rust-timer commented Nov 19, 2022

Overall result: ✅ improvements - no action needed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Nov 19, 2022

Uh oh!

This comment has been minimized.

bors commented Nov 19, 2022

Uh oh!

bors commented Nov 19, 2022

Uh oh!

This comment has been minimized.

rust-timer commented Nov 19, 2022

Overall result: ✅ improvements - no action needed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jackh726 commented Nov 21, 2022

Uh oh!

compiler-errors commented Nov 26, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

compiler-errors commented Nov 19, 2022 •

edited

Loading