anonymize all bound vars, not just regions #99730

lcnr · 2022-07-25T18:25:29Z

fixes #98702

r? types

lcnr · 2022-07-25T18:26:40Z

@bors try @rust-timer queue

rust-timer · 2022-07-25T18:26:42Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-07-25T18:26:48Z

⌛ Trying commit 8f7f5b8a38a1cb52504142937e0cd6834e5d29cc with merge 56beffeda7c3ae12b116d2fdb10c6e2ac0431631...

bors · 2022-07-25T19:56:01Z

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

rust-timer · 2022-07-25T19:56:03Z

Queued 56beffeda7c3ae12b116d2fdb10c6e2ac0431631 with parent dc2d232, future comparison URL.

rust-timer · 2022-07-25T22:28:00Z

Finished benchmarking commit (56beffeda7c3ae12b116d2fdb10c6e2ac0431631): comparison url.

Instruction count

Primary benchmarks: 😿 relevant regressions found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	0.4%	0.4%	3
Regressions 😿 (secondary)	1.7%	2.6%	9
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	0.4%	0.4%	3

Max RSS (memory usage)

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

Cycles

Results

Primary benchmarks: 🎉 relevant improvement found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	2.6%	3.2%	6
Improvements 🎉 (primary)	-2.1%	-2.1%	1
Improvements 🎉 (secondary)	-3.4%	-3.6%	2
All 😿🎉 (primary)	-2.1%	-2.1%	1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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-review -S-waiting-on-perf +perf-regression

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

jackh726 · 2022-07-25T22:53:13Z

Want to read through this again, but overall LGTM I think

oli-obk · 2022-07-27T11:11:31Z

Lgtm, too

r? @jackh726

lcnr · 2022-07-28T14:56:07Z

@bors try @rust-timer queue

rust-timer · 2022-07-28T14:56:09Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-07-28T14:56:15Z

⌛ Trying commit 8dffa1f672c66961ece63f1a8a8659a2675cfdaa with merge f6c9701f48e24692d3403ab5d31d9ec8838a5c18...

bors · 2022-07-28T16:46:36Z

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

rust-timer · 2022-07-28T16:46:38Z

Queued f6c9701f48e24692d3403ab5d31d9ec8838a5c18 with parent 05e678c, future comparison URL.

rust-timer · 2022-07-28T19:01:57Z

Finished benchmarking commit (f6c9701f48e24692d3403ab5d31d9ec8838a5c18): comparison url.

Instruction count

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	0.7%	1.5%	8
Improvements 🎉 (primary)	-0.4%	-0.6%	35
Improvements 🎉 (secondary)	-0.5%	-1.2%	21
All 😿🎉 (primary)	-0.4%	-0.6%	35

Max RSS (memory usage)

Results

Primary benchmarks: 😿 relevant regression found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	2.3%	2.3%	1
Regressions 😿 (secondary)	3.0%	5.0%	8
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	2.3%	2.3%	1

Cycles

Results

Primary benchmarks: 😿 relevant regressions found
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	4.9%	7.9%	2
Regressions 😿 (secondary)	2.6%	3.0%	9
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	4.9%	7.9%	2

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

jackh726

one thought, otherwise r=me

jackh726 · 2022-07-28T20:51:01Z

compiler/rustc_middle/src/ty/fold.rs

+                    if debruijn == ty::INNERMOST {
+                        region
+                    } else {
+                        self.tcx.mk_region(ty::ReLateBound(debruijn, br))
+                    }


Isn't there a mk_or_reuse_region`?

not that i know of, we previously used mk_or_reuse_ty during type folding but afaict that has been removed in favor of a manual check as well.

it's called reuse_or_mk_region instead 😁

lcnr · 2022-07-29T06:31:12Z

@bors r=jackh726 rollup=never

bors · 2022-07-29T06:31:14Z

📌 Commit 8dffa1f672c66961ece63f1a8a8659a2675cfdaa has been approved by jackh726

It is now in the queue for this repository.

lcnr · 2022-07-29T06:31:43Z

@bors r-

lcnr · 2022-07-29T06:49:03Z

want to try only reusing regions without the change from dyn to type param, doing that in a separate pr

lcnr · 2022-07-29T09:55:01Z

see #99894, using generics instead of trait objects is a major part of the perf improvement

@bors r=jackh726 rollup=never

bors · 2022-07-29T09:55:03Z

📌 Commit 1436fa9 has been approved by jackh726

It is now in the queue for this repository.

bors · 2022-07-29T21:38:38Z

⌛ Testing commit 1436fa9 with merge 211637d...

bors · 2022-07-30T00:26:16Z

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing 211637d to master...

rust-timer · 2022-07-30T02:16:56Z

Finished benchmarking commit (211637d): comparison url.

Instruction count

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	N/A	N/A	0
Improvements 🎉 (primary)	-0.4%	-0.6%	56
Improvements 🎉 (secondary)	-0.4%	-0.9%	36
All 😿🎉 (primary)	-0.4%	-0.6%	56

Max RSS (memory usage)

Results

Primary benchmarks: 🎉 relevant improvement found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	2.7%	3.2%	5
Improvements 🎉 (primary)	-1.6%	-1.6%	1
Improvements 🎉 (secondary)	-1.3%	-1.3%	1
All 😿🎉 (primary)	-1.6%	-1.6%	1

Cycles

Results

Primary benchmarks: mixed results
Secondary benchmarks: 😿 relevant regressions found

	mean¹	max	count²
Regressions 😿 (primary)	2.8%	2.8%	1
Regressions 😿 (secondary)	2.8%	3.5%	10
Improvements 🎉 (primary)	-2.8%	-2.8%	1
Improvements 🎉 (secondary)	N/A	N/A	0
All 😿🎉 (primary)	-0.0%	-2.8%	2

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

Mark-Simulacrum · 2022-07-31T00:10:28Z

This PR was a pretty large regression in bootstrap compile times (bottom of https://perf.rust-lang.org/compare.html?start=3924dac7bb29bc8eb348059c901e8f912399c857&end=211637d0802a1c17d41b414e091e9a8691b26068&stat=instructions:u), 2% overall and 5+% across a number of rustc_ crates. Might be worth looking at the diff here to see if we can reduce some extra monomorphization that was presumably introduced.

jackh726 · 2022-07-31T03:26:27Z

I think we likely need to revert back to using dyn Fns here. @lcnr

I think this was tried in #99894, which shows that it's not "free", but small enough impact that it's probably an okay for bootstrap compile times.

rust-log-analyzer · 2022-07-31T16:53:32Z

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

  CACHE_DOMAIN: ci-caches.rust-lang.org
  DEPLOY: 1
  IMAGE: dist-x86_64-linux
##[endgroup]
error: PRs changing the `nightly` channel should be sent to the `master` branch!
##[error]Process completed with exit code 1.

rust-highfive assigned oli-obk Jul 25, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 25, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 25, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 25, 2022

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

rust-highfive assigned jackh726 and unassigned oli-obk Jul 27, 2022

lcnr added 2 commits July 28, 2022 16:13

BoundVarReplacer: trait object instead of 3 fns

fd59d05

anonymize all bound vars, not just regions

c3fce8e

lcnr force-pushed the bound-vars-anon branch 2 times, most recently from ca1f10f to 8dffa1f Compare July 28, 2022 14:55

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 28, 2022

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jul 28, 2022

jackh726 approved these changes Jul 28, 2022

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 29, 2022

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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 29, 2022

optimize bound vars replacement :3

1436fa9

lcnr force-pushed the bound-vars-anon branch from 8dffa1f to 1436fa9 Compare July 29, 2022 06:46

lcnr mentioned this pull request Jul 29, 2022

[perf] anonymize all bound vars, not just regions #99894

Closed

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 29, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 30, 2022

bors merged commit 211637d into rust-lang:master Jul 30, 2022

rustbot added this to the 1.64.0 milestone Jul 30, 2022

lcnr deleted the bound-vars-anon branch July 30, 2022 17:39

lcnr mentioned this pull request Aug 1, 2022

super_relate_consts: stop being generic #100016

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

anonymize all bound vars, not just regions #99730

anonymize all bound vars, not just regions #99730

lcnr commented Jul 25, 2022 •

edited

Loading

lcnr commented Jul 25, 2022

rust-timer commented Jul 25, 2022

bors commented Jul 25, 2022

bors commented Jul 25, 2022

rust-timer commented Jul 25, 2022

rust-timer commented Jul 25, 2022

jackh726 commented Jul 25, 2022

oli-obk commented Jul 27, 2022

lcnr commented Jul 28, 2022

rust-timer commented Jul 28, 2022

bors commented Jul 28, 2022

bors commented Jul 28, 2022

rust-timer commented Jul 28, 2022

rust-timer commented Jul 28, 2022

jackh726 left a comment

jackh726 Jul 28, 2022

lcnr Jul 29, 2022

lcnr Jul 29, 2022

lcnr commented Jul 29, 2022

bors commented Jul 29, 2022

lcnr commented Jul 29, 2022

lcnr commented Jul 29, 2022

lcnr commented Jul 29, 2022

bors commented Jul 29, 2022

bors commented Jul 29, 2022

bors commented Jul 30, 2022

rust-timer commented Jul 30, 2022

Mark-Simulacrum commented Jul 31, 2022

jackh726 commented Jul 31, 2022

rust-log-analyzer commented Jul 31, 2022

anonymize all bound vars, not just regions #99730

anonymize all bound vars, not just regions #99730

Conversation

lcnr commented Jul 25, 2022 • edited Loading

lcnr commented Jul 25, 2022

rust-timer commented Jul 25, 2022

bors commented Jul 25, 2022

bors commented Jul 25, 2022

rust-timer commented Jul 25, 2022

rust-timer commented Jul 25, 2022

Footnotes

jackh726 commented Jul 25, 2022

oli-obk commented Jul 27, 2022

lcnr commented Jul 28, 2022

rust-timer commented Jul 28, 2022

bors commented Jul 28, 2022

bors commented Jul 28, 2022

rust-timer commented Jul 28, 2022

rust-timer commented Jul 28, 2022

Footnotes

jackh726 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lcnr commented Jul 29, 2022

bors commented Jul 29, 2022

lcnr commented Jul 29, 2022

lcnr commented Jul 29, 2022

lcnr commented Jul 29, 2022

bors commented Jul 29, 2022

bors commented Jul 29, 2022

bors commented Jul 30, 2022

rust-timer commented Jul 30, 2022

Footnotes

Mark-Simulacrum commented Jul 31, 2022

jackh726 commented Jul 31, 2022

rust-log-analyzer commented Jul 31, 2022

lcnr commented Jul 25, 2022 •

edited

Loading