Add `constness` to `TraitDef` by fee1-dead · Pull Request #127200 · rust-lang/rust

fee1-dead · 2024-07-01T08:38:23Z

Second attempt at fixing the regression @ #120639 (comment)

r? project-const-traits

rustbot · 2024-07-01T08:38:32Z

HIR ty lowering was modified

cc @fmease

fee1-dead · 2024-07-01T08:38:47Z

@bors try @rust-timer queue

bors · 2024-07-01T08:39:57Z

⌛ Trying commit ee2d52a with merge 6cb3b57...

…ait, r=<try> Add `constness` to `TraitDef` Second attempt at fixing the regression @ rust-lang#120639 (comment) r? project-const-traits

bors · 2024-07-01T08:51:51Z

💔 Test failed - checks-actions

fee1-dead · 2024-07-01T08:56:59Z

@bors try @rust-timer queue

…ait, r=<try> Add `constness` to `TraitDef` Second attempt at fixing the regression @ rust-lang#120639 (comment) r? project-const-traits

bors · 2024-07-01T08:58:11Z

⌛ Trying commit 93b42fc with merge 576fbf8...

bors · 2024-07-01T10:39:08Z

☀️ Try build successful - checks-actions
Build commit: 576fbf8 (576fbf8589d1c38cf150f37c8e69b276a6ec23c3)

rust-timer · 2024-07-01T11:54:48Z

Finished benchmarking commit (576fbf8): comparison URL.

Overall result: ❌✅ regressions and improvements - 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

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.4%	[0.4%, 0.4%]	1
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.2%]	3
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	1
All ❌✅ (primary)	-0.3%	[-0.3%, -0.2%]	3

Max RSS (memory usage)

Results (primary -6.7%, secondary -2.7%)

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

Cycles

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

Binary size

Results (secondary 0.0%)

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

Bootstrap: 699.484s -> 695.977s (-0.50%)
Artifact size: 326.66 MiB -> 326.72 MiB (0.02%)

compiler-errors

You should be able to set EncodeCrossCrate::No for const_trait, though I don't think it'll improve perf :/

fee1-dead · 2024-07-03T15:38:27Z

You should be able to set EncodeCrossCrate::No for const_trait, though I don't think it'll improve perf :/

done

@rustbot ready

fee1-dead · 2024-07-08T06:49:33Z

FWIW, I think this is a perf improvement. Looking at the details of the single perf regression seems to show more instructions being spent on loading incremental data. That might be because we added constness to TraitDef. But cycles doesn't have any significant changes so I think this should be fine.

compiler-errors

One question, r=me otherwise

compiler-errors · 2024-07-09T04:21:16Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

                    path.segments[..path.segments.len() - 2].iter(),
                    GenericsArgsErrExtend::None,
                );
-                // HACK: until we support `<Type as ~const Trait>`, assume all of them are.


Why was this removed? Do we not need this anymore?

since we don't have the constness param on traits, we don't need <T as ~const> anymore

compiler-errors · 2024-07-09T04:22:00Z

@bors rollup=never

fee1-dead · 2024-07-09T05:41:47Z

@bors r=compiler-errors

bors · 2024-07-09T05:41:49Z

📌 Commit 46af987 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2024-07-09T06:51:38Z

⌛ Testing commit 46af987 with merge 5be2ec7...

bors · 2024-07-09T09:06:34Z

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

rust-timer · 2024-07-09T11:51:01Z

Finished benchmarking commit (5be2ec7): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -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.4%	[-0.4%, -0.4%]	1
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

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

Cycles

Results (secondary -2.7%)

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

Binary size

Results (secondary 0.0%)

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

Bootstrap: 703.367s -> 702.485s (-0.13%)
Artifact size: 328.80 MiB -> 328.89 MiB (0.03%)

rustbot assigned fmease Jul 1, 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 Jul 1, 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 1, 2024

This comment has been minimized.

Sign in to view

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 1, 2024

fee1-dead force-pushed the trait_def_const_trait branch from ee2d52a to 93b42fc Compare July 1, 2024 08:55

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 1, 2024

compiler-errors reviewed Jul 1, 2024

View reviewed changes

Add constness to TraitDef

46af987

fee1-dead force-pushed the trait_def_const_trait branch from 93b42fc to 46af987 Compare July 3, 2024 15:37

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 3, 2024

compiler-errors approved these changes Jul 9, 2024

View reviewed changes

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

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 9, 2024

bors merged commit 5be2ec7 into rust-lang:master Jul 9, 2024

rustbot added this to the 1.81.0 milestone Jul 9, 2024

rustbot removed the perf-regression Performance regression. label Jul 9, 2024

fee1-dead deleted the trait_def_const_trait branch July 10, 2024 06:41

fee1-dead added the PG-const-traits Project group: Const traits label Aug 9, 2024

Uh oh!

Conversation

fee1-dead commented Jul 1, 2024

Uh oh!

rustbot commented Jul 1, 2024

Uh oh!

fee1-dead commented Jul 1, 2024

Uh oh!

This comment has been minimized.

bors commented Jul 1, 2024

Uh oh!

This comment has been minimized.

This comment has been minimized.

bors commented Jul 1, 2024

Uh oh!

fee1-dead commented Jul 1, 2024

Uh oh!

This comment has been minimized.

bors commented Jul 1, 2024

Uh oh!

bors commented Jul 1, 2024

Uh oh!

This comment has been minimized.

rust-timer commented Jul 1, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

fee1-dead commented Jul 3, 2024

Uh oh!

fee1-dead commented Jul 8, 2024

Uh oh!

compiler-errors left a comment

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!

compiler-errors commented Jul 9, 2024

Uh oh!

fee1-dead commented Jul 9, 2024

Uh oh!

bors commented Jul 9, 2024

Uh oh!

bors commented Jul 9, 2024

Uh oh!

bors commented Jul 9, 2024

Uh oh!

rust-timer commented Jul 9, 2024

Overall result: ✅ improvements - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants